fixes for issue 128: reviewed by Waylonis

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

3 files changed, 63 insertions, 4 deletions

diff --git a/src/client/mac/handler/exception_handler.cc b/src/client/mac/handler/exception_handler.cc
index 0fdd18f7..e540df23 100644
--- a/src/client/mac/handler/exception_handler.cc
+++ b/src/client/mac/handler/exception_handler.cc
@@ -337,13 +337,22 @@ void *ExceptionHandler::WaitForMessage(void *exception_handler_class) {
       // If the actual exception code is zero, then we're calling this handler
       // in a way that indicates that we want to either exit this thread or
       // generate a minidump
+      //
+      // While reporting, all threads (except this one) must be suspended
+      // to avoid misleading stacks.  If appropriate they will be resumed
+      // afterwards.
       if (!receive.exception) {
         if (self->is_in_teardown_)
           return NULL;
 
+        self->SuspendThreads();
+
         // Write out the dump and save the result for later retrieval
         self->last_minidump_write_result_ =
           self->WriteMinidumpWithException(0, 0, 0);
+
+        self->ResumeThreads();
+
         if (self->use_minidump_write_mutex_)
           pthread_mutex_unlock(&self->minidump_write_mutex_);
       } else {
@@ -353,6 +362,7 @@ void *ExceptionHandler::WaitForMessage(void *exception_handler_class) {
         // exceptions that occur in the parent process are caught and 
         // processed.
         if (receive.task.name == mach_task_self()) {
+          self->SuspendThreads();
           
           // Generate the minidump with the exception data.
           self->WriteMinidumpWithException(receive.exception, receive.code[0],
@@ -512,4 +522,40 @@ void ExceptionHandler::UpdateNextID() {
   next_minidump_id_c_ = next_minidump_id_.c_str();
 }
 
+bool ExceptionHandler::SuspendThreads() {
+  thread_act_port_array_t   threads_for_task;
+  mach_msg_type_number_t    thread_count;
+
+  if (task_threads(mach_task_self(), &threads_for_task, &thread_count))
+    return false;
+
+  // suspend all of the threads except for this one
+  for (unsigned int i = 0; i < thread_count; ++i) {
+    if (threads_for_task[i] != mach_thread_self()) {
+      if (thread_suspend(threads_for_task[i]))
+        return false;
+    }
+  }
+  
+  return true;
+}
+
+bool ExceptionHandler::ResumeThreads() {
+  thread_act_port_array_t   threads_for_task;
+  mach_msg_type_number_t    thread_count;
+
+  if (task_threads(mach_task_self(), &threads_for_task, &thread_count))
+    return false;
+
+  // resume all of the threads except for this one
+  for (unsigned int i = 0; i < thread_count; ++i) {
+    if (threads_for_task[i] != mach_thread_self()) {
+      if (thread_resume(threads_for_task[i]))
+        return false;
+    }
+  }
+  
+  return true;
+}
+
 }  // namespace google_breakpad
diff --git a/src/client/mac/handler/exception_handler.h b/src/client/mac/handler/exception_handler.h
index dcc61cb7..11babb73 100644
--- a/src/client/mac/handler/exception_handler.h
+++ b/src/client/mac/handler/exception_handler.h
@@ -133,6 +133,11 @@ class ExceptionHandler {
   // Generates a new ID and stores it in next_minidump_id_, and stores the
   // path of the next minidump to be written in next_minidump_path_.
   void UpdateNextID();
+
+  // These functions will suspend/resume all threads except for the 
+  // reporting thread
+  bool SuspendThreads();
+  bool ResumeThreads();
   
   // The destination directory for the minidump
   string dump_path_;
diff --git a/src/client/mac/handler/minidump_generator.cc b/src/client/mac/handler/minidump_generator.cc
index 26a6adad..5cddf88d 100644
--- a/src/client/mac/handler/minidump_generator.cc
+++ b/src/client/mac/handler/minidump_generator.cc
@@ -184,9 +184,6 @@ bool MinidumpGenerator::Write(const char *path) {
   return result;
 }
 
-// TODO(waylonis): This routine works most of the time.  However, if a process
-// fork()s, it might cause the stack so that the current top of stack will
-// exist on a single page.
 static size_t CalculateStackSize(vm_address_t start_addr) {
   vm_address_t stack_region_base = start_addr;
   vm_size_t stack_region_size;
@@ -199,6 +196,17 @@ static size_t CalculateStackSize(vm_address_t start_addr) {
                       reinterpret_cast<vm_region_recurse_info_t>(&submap_info),
                       &info_count);
 
+  if ((stack_region_base + stack_region_size) == 0xbffff000) {
+    // The stack for thread 0 needs to extend all the way to 0xc0000000
+    // For many processes the stack is first created in one page
+    // from 0xbffff000 - 0xc0000000 and is then later extended to
+    // a much larger size by creating a new VM region immediately below
+    // the initial page
+
+    // include the original stack frame page (0xbffff000 - 0xc0000000)
+    stack_region_size += 0x1000;  
+  }
+
   return result == KERN_SUCCESS ? 
     stack_region_base + stack_region_size - start_addr : 0;
 }
@@ -224,7 +232,6 @@ bool MinidumpGenerator::WriteStackFromStartAddress(
   return result;
 }
 
-
 #if TARGET_CPU_PPC
 bool MinidumpGenerator::WriteStack(thread_state_data_t state,
                                    MDMemoryDescriptor *stack_location) {
@@ -393,6 +400,7 @@ bool MinidumpGenerator::WriteThreadListStream(
 
   MDRawThread thread;
   int thread_idx = 0;
+
   for (unsigned int i = 0; i < thread_count; ++i) {
     memset(&thread, 0, sizeof(MDRawThread));