Fix CalculateStackSize to behave properly when the main thread's stack is split up into blah blah

Fix CalculateStackSize to behave properly when the main thread's stack is
split up into multiple regions.

An older workaround relyied on known fixed stack locations and only filled in
the initial page of the stack if it was in a distinct region. The new approach
looks upwards for additional regions that appear to be part of the same stack.

With PIE on Lion, the stack no longer begins at a fixed address, so the older
workaround became ineffective.

BUG=247, chromium:94107
TEST=Stacks should run through to _main/start and then stop when examining
     Chrome on Lion with PIE and "slid" stacks.

[Mark Mentovai]

What’s happening here is that the dumper isn’t writing the last page of the
stack on Lion. Over on the processor side, the stackwalker finds that it’s asked
to recover some registers that aren’t present in its view of memory, so it
begins scanning the stack looking for likely return addresses. Anything that it
finds that smells like a pointer to code is a candidate for inclusion in this
stackwalk. (This is an old old old stackwalker hack that I had to put in because
the Windows native tools do something like this.)

This is why _ChromeMain/_main/start are often missing from Lion stacks, and why
the walk seems to put in other random junk in their place.

This is why we only see the problem for the main thread’s stack.

When we switched to clang and turned off the STACK CFI lines, we triggered the
“other random junk” problem, because instead of seeing that a walk according to
CFI data was fruitless, the stackwalker fell back to doing the traditional
recover-by-frame-pointer walk, which has the silly scan as its own fallback.

[Ted Mielczarek]

I think you can probably write a unit test for this if you care to, the existing
minidump_generator_unittest setup already imports the processor libs, and the
existing tests write minidumps and then read them back in to verify some info:
http://code.google.com/p/google-breakpad/source/browse/trunk/src/client/mac/t...

[shess]

LGTM, but this is deeper than anything I really understand.

http://breakpad.appspot.com/300001/diff/2001/1003
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/300001/diff/2001/1003#newcode249
Line 249: region_info =
reinterpret_cast<vm_region_recurse_info_t>(&submap_info);
This threw me at first, as the new place you're using it is so far away.  I
couldn't see how the break-out test was testing anything new!

http://breakpad.appspot.com/300001/diff/2001/1003#newcode288
Line 288: submap_info.protection & VM_PROT_READ == 0) {
I'm assuming there's a guard page or something after which is not accessible -
would it be reasonable to have .protection == .protection from the initial call
to mach_vm_region_recurse()?

[shess]

http://breakpad.appspot.com/300001/diff/2001/1003
File src/client/mac/handler/minidump_generator.cc (right):

http://breakpad.appspot.com/300001/diff/2001/1003#newcode288
Line 288: submap_info.protection & VM_PROT_READ == 0) {
On 2011/08/25 19:13:53, shess wrote:
> I'm assuming there's a guard page or something after which is not accessible -
> would it be reasonable to have .protection == .protection from the initial
call
> to mach_vm_region_recurse()?

Hmm, you patch adding the mprotect() point perhaps addresses this comment.