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.
Review URL: http://breakpad.appspot.com/300001

git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@826 4c0a9323-5329-0410-9bdc-e9ce6186880e

2 files changed, 32 insertions, 27 deletions

diff --git a/src/client/mac/handler/minidump_generator.cc b/src/client/mac/handler/minidump_generator.cc
index 4f388e6f..a7680c21 100644
--- a/src/client/mac/handler/minidump_generator.cc
+++ b/src/client/mac/handler/minidump_generator.cc
@@ -255,37 +255,45 @@ size_t MinidumpGenerator::CalculateStackSize(mach_vm_address_t start_addr) {
   kern_return_t result =
     mach_vm_region_recurse(crashing_task_, &stack_region_base,
                            &stack_region_size, &nesting_level,
-                           region_info,
-                           &info_count);
+                           region_info, &info_count);
 
-  if (start_addr < stack_region_base) {
-    // probably stack corruption, since mach_vm_region had to go
+  if (result != KERN_SUCCESS || start_addr < stack_region_base) {
+    // Failure or stack corruption, since mach_vm_region had to go
     // higher in the process address space to find a valid region.
     return 0;
   }
 
-  if (((cpu_type_ & CPU_ARCH_ABI64) &&
-       (stack_region_base + stack_region_size) == TOP_OF_THREAD0_STACK_64BIT) ||
-      (!(cpu_type_ & CPU_ARCH_ABI64) &&
-       (stack_region_base + stack_region_size) == TOP_OF_THREAD0_STACK_32BIT)) {
-    // The stack for thread 0 needs to extend all the way to
-    // 0xc0000000 on 32 bit and 00007fff5fc00000 on 64bit.  HOWEVER,
-    // for many processes, the stack is first created in one page
-    // below this, and is then later extended to a much larger size by
-    // creating a new VM region immediately below the initial page.
-
-    // You can see this for yourself by running vmmap on a "hello,
-    // world" program
-
-    // Because of the above, we'll add 4k to include the original
-    // stack frame page.
-    // This method of finding the stack region needs to be done in
-    // a better way; the breakpad issue 247 is tracking this.
-    stack_region_size += 0x1000;
+  unsigned int tag = submap_info.user_tag;
+
+  // If the user tag is VM_MEMORY_STACK, look for more readable regions with
+  // the same tag placed immediately above the computed stack region. Under
+  // some circumstances, the stack for thread 0 winds up broken up into
+  // multiple distinct abutting regions. This can happen for several reasons,
+  // including user code that calls setrlimit(RLIMIT_STACK, ...) or changes
+  // the access on stack pages by calling mprotect.
+  if (tag == VM_MEMORY_STACK) {
+    while (true) {
+      mach_vm_address_t next_region_base = stack_region_base +
+                                           stack_region_size;
+      mach_vm_address_t proposed_next_region_base = next_region_base;
+      mach_vm_size_t next_region_size;
+      nesting_level = 0;
+      mach_msg_type_number_t info_count = VM_REGION_SUBMAP_INFO_COUNT_64;
+      result = mach_vm_region_recurse(crashing_task_, &next_region_base,
+                                      &next_region_size, &nesting_level,
+                                      region_info, &info_count);
+      if (result != KERN_SUCCESS ||
+          next_region_base != proposed_next_region_base ||
+          submap_info.user_tag != tag ||
+          submap_info.protection & VM_PROT_READ == 0) {
+        break;
+      }
+
+      stack_region_size += next_region_size;
+    }
   }
 
-  return result == KERN_SUCCESS ?
-    stack_region_base + stack_region_size - start_addr : 0;
+  return stack_region_base + stack_region_size - start_addr;
 }
 
 bool MinidumpGenerator::WriteStackFromStartAddress(
diff --git a/src/client/mac/handler/minidump_generator.h b/src/client/mac/handler/minidump_generator.h
index 55c4c8a3..6f9fe8f7 100644
--- a/src/client/mac/handler/minidump_generator.h
+++ b/src/client/mac/handler/minidump_generator.h
@@ -51,9 +51,6 @@ namespace google_breakpad {
 
 using std::string;
 
-const u_int64_t TOP_OF_THREAD0_STACK_64BIT = 0x00007fff5fbff000LL;
-const u_int32_t TOP_OF_THREAD0_STACK_32BIT = 0xbffff000;
-
 // Use the REGISTER_FROM_THREADSTATE to access a register name from the
 // breakpad_thread_state_t structure.
 #if __DARWIN_UNIX03 || TARGET_CPU_X86_64 || TARGET_CPU_PPC64