Issue 181: Add version info for Mac OS X modules. Found by iterating over load commands until I found LC_ID_DYLIB. Also modified crash_report to generate version number. Also added suspend/resume capability to exception handler, necessary because exception handling can behave strangely across fork() calls. Also added fix for filtering out functions with no line number information, and for filtering out some multiple inheritance glue the compiler generates.

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

5 files changed, 160 insertions, 94 deletions

diff --git a/src/client/mac/handler/dynamic_images.cc b/src/client/mac/handler/dynamic_images.cc
index 7bbcd566..ac1cca40 100644
--- a/src/client/mac/handler/dynamic_images.cc
+++ b/src/client/mac/handler/dynamic_images.cc
@@ -187,7 +187,7 @@ void* ReadTaskMemory(task_port_t target_task,
 
 //==============================================================================
 // Initializes vmaddr_, vmsize_, and slide_
-void DynamicImage::CalculateMemoryInfo() {
+void DynamicImage::CalculateMemoryAndVersionInfo() {
   breakpad_mach_header *header = GetMachHeader();
 
   // unless we can process the header, ensure that calls to
@@ -195,7 +195,11 @@ void DynamicImage::CalculateMemoryInfo() {
   vmaddr_ = 0;
   vmsize_ = 0;
   slide_ = 0;
+  version_ = 0;
 
+  bool foundTextSection = false;
+  bool foundDylibIDCommand = false;
+  
 #if __LP64__
   if(header->magic != MH_MAGIC_64) {
     return;
@@ -206,30 +210,48 @@ void DynamicImage::CalculateMemoryInfo() {
   }
 #endif
 
-  const struct load_command *cmd =
-    reinterpret_cast<const struct load_command *>(header + 1);
-
-  for (unsigned int i = 0; cmd && (i < header->ncmds); ++i) {
 #ifdef __LP64__
-    if (cmd->cmd == LC_SEGMENT_64) {
+  const uint32_t segmentLoadCommand = LC_SEGMENT_64;
 #else
-    if (cmd->cmd == LC_SEGMENT) {
+  const uint32_t segmentLoadCommand = LC_SEGMENT;
 #endif
-      const breakpad_mach_segment_command *seg =
-        reinterpret_cast<const breakpad_mach_segment_command *>(cmd);
 
-      if (!strcmp(seg->segname, "__TEXT")) {
-        vmaddr_ = seg->vmaddr;
-        vmsize_ = seg->vmsize;
-        slide_ = 0;
+  const struct load_command *cmd =
+    reinterpret_cast<const struct load_command *>(header + 1);
 
-        if (seg->fileoff == 0  &&  seg->filesize != 0) {
-          slide_ = (uintptr_t)GetLoadAddress() - (uintptr_t)seg->vmaddr;
+  for (unsigned int i = 0; cmd && (i < header->ncmds); ++i) {
+    if (!foundTextSection) {
+      if (cmd->cmd == segmentLoadCommand) {
+        const breakpad_mach_segment_command *seg =
+            reinterpret_cast<const breakpad_mach_segment_command *>(cmd);
+
+        if (!strcmp(seg->segname, "__TEXT")) {
+          vmaddr_ = seg->vmaddr;
+          vmsize_ = seg->vmsize;
+          slide_ = 0;
+
+          if (seg->fileoff == 0  &&  seg->filesize != 0) {
+            slide_ = (uintptr_t)GetLoadAddress() - (uintptr_t)seg->vmaddr;
+          }
+          foundTextSection = true;
         }
-        return;
       }
     }
 
+    if (!foundDylibIDCommand) {
+      if (cmd->cmd == LC_ID_DYLIB) {
+        const struct dylib_command *dc =
+            reinterpret_cast<const struct dylib_command *>(cmd);
+
+        version_ = dc->dylib.current_version;
+        foundDylibIDCommand = true;
+      }
+    }
+
+    if (foundDylibIDCommand && foundTextSection) {
+      return;
+    }
+
     cmd = reinterpret_cast<const struct load_command *>
       (reinterpret_cast<const char *>(cmd) + cmd->cmdsize);
   }
diff --git a/src/client/mac/handler/dynamic_images.h b/src/client/mac/handler/dynamic_images.h
index c9acf70a..85ba8cf9 100644
--- a/src/client/mac/handler/dynamic_images.h
+++ b/src/client/mac/handler/dynamic_images.h
@@ -114,7 +114,7 @@ class DynamicImage {
       file_mod_date_(image_mod_date),
       task_(task) {
     InitializeFilePath(inFilePath);
-    CalculateMemoryInfo();
+    CalculateMemoryAndVersionInfo();
   }
 
   ~DynamicImage() {
@@ -150,6 +150,7 @@ class DynamicImage {
   // Task owning this loaded image
   mach_port_t GetTask() {return task_;}
 
+  uint32_t GetVersion() {return version_;}
   // For sorting
   bool operator<(const DynamicImage &inInfo) {
     return GetLoadAddress() < inInfo.GetLoadAddress();
@@ -176,29 +177,7 @@ class DynamicImage {
   }
 
   // Initializes vmaddr_, vmsize_, and slide_
-  void CalculateMemoryInfo();
-
-#if 0   // currently not needed
-  // Copy constructor: we don't want this to be invoked,
-  // but here's the code in case we need to make it public some day.
-  DynamicImage(DynamicImage &inInfo)
-    : load_address_(inInfo.load_address_),
-      vmaddr_(inInfo.vmaddr_),
-      vmsize_(inInfo.vmsize_),
-      slide_(inInfo.slide_),
-      file_mod_date_(inInfo.file_mod_date_),
-      task_(inInfo.task_) {
-    // copy file path string
-    InitializeFilePath(inInfo.GetFilePath());
-
-    // copy mach_header and load commands
-    void *headerBuffer = malloc(inInfo.header_size_);
-    header_ = reinterpret_cast<breakpad_mach_header*>(headerBuffer);
-
-    memcpy(header_, inInfo.header_, inInfo.header_size_);
-    header_size_ = inInfo.header_size_;
-  }
-#endif
+  void CalculateMemoryAndVersionInfo();
 
   breakpad_mach_header    *header_;        // our local copy of the header
   int                     header_size_;    // mach_header plus load commands
@@ -206,7 +185,7 @@ class DynamicImage {
   mach_vm_address_t       vmaddr_;
   mach_vm_size_t          vmsize_;
   ptrdiff_t               slide_;
-
+  uint32_t                version_;        // Dylib version
   char                    *file_path_;     // path dyld used to load the image
   uintptr_t               file_mod_date_;  // time_t of image file
 
diff --git a/src/client/mac/handler/exception_handler.cc b/src/client/mac/handler/exception_handler.cc
index 41ffb194..b91cdd49 100644
--- a/src/client/mac/handler/exception_handler.cc
+++ b/src/client/mac/handler/exception_handler.cc
@@ -83,7 +83,7 @@ struct ExceptionReplyMessage {
 
 // Only catch these three exceptions.  The other ones are nebulously defined
 // and may result in treating a non-fatal exception as fatal.
-exception_mask_t s_exception_mask = EXC_MASK_BAD_ACCESS | 
+exception_mask_t s_exception_mask = EXC_MASK_BAD_ACCESS |
 EXC_MASK_BAD_INSTRUCTION | EXC_MASK_ARITHMETIC | EXC_MASK_BREAKPOINT;
 
 extern "C"
@@ -137,7 +137,7 @@ extern "C"
                                    mach_msg_type_number_t thread_state_count,
                                    thread_state_t thread_state,
                                    mach_msg_type_number_t *thread_state_count);
-  
+
   kern_return_t breakpad_exception_raise_state(mach_port_t exception_port,
                                                exception_type_t exception,
                                                const exception_data_t code,
@@ -148,7 +148,7 @@ extern "C"
                                                thread_state_t new_state,
                                                mach_msg_type_number_t *new_stateCnt
                                                );
-  
+
   kern_return_t breakpad_exception_raise_state_identity(mach_port_t exception_port,
                                                         mach_port_t thread,
                                                         mach_port_t task,
@@ -161,12 +161,12 @@ extern "C"
                                                         thread_state_t new_state,
                                                         mach_msg_type_number_t *new_stateCnt
                                                         );
-  
+
   kern_return_t breakpad_exception_raise(mach_port_t port, mach_port_t failed_thread,
                                          mach_port_t task,
                                          exception_type_t exception,
                                          exception_data_t code,
-                                         mach_msg_type_number_t code_count);  
+                                         mach_msg_type_number_t code_count);
 }
 
 
@@ -221,7 +221,7 @@ ExceptionHandler::ExceptionHandler(const string &dump_path,
       callback_context_(callback_context),
       directCallback_(NULL),
       handler_thread_(NULL),
-      handler_port_(0),
+      handler_port_(MACH_PORT_NULL),
       previous_(NULL),
       installed_exception_handler_(false),
       is_in_teardown_(false),
@@ -244,7 +244,7 @@ ExceptionHandler::ExceptionHandler(DirectCallback callback,
       callback_context_(callback_context),
       directCallback_(callback),
       handler_thread_(NULL),
-      handler_port_(0),
+      handler_port_(MACH_PORT_NULL),
       previous_(NULL),
       installed_exception_handler_(false),
       is_in_teardown_(false),
@@ -276,7 +276,7 @@ bool ExceptionHandler::WriteMinidump() {
     // the mutex when completed
     pthread_mutex_lock(&minidump_write_mutex_);
   }
-  
+
   use_minidump_write_mutex_ = false;
   UpdateNextID();
   return last_minidump_write_result_;
@@ -327,14 +327,14 @@ bool ExceptionHandler::WriteMinidumpWithException(int exception_type,
       // If the user callback returned true and we're handling an exception
       // (rather than just writing out the file), then we should exit without
       // forwarding the exception to the next handler.
-      if (callback_(dump_path_c_, next_minidump_id_c_, callback_context_, 
+      if (callback_(dump_path_c_, next_minidump_id_c_, callback_context_,
                     result)) {
         if (exception_type && exception_code)
           _exit(exception_type);
       }
     }
   }
-  
+
   return result;
 }
 
@@ -343,20 +343,20 @@ kern_return_t ForwardException(mach_port_t task, mach_port_t failed_thread,
                                exception_data_t code,
                                mach_msg_type_number_t code_count) {
   // At this time, we should have called Uninstall() on the exception handler
-  // so that the current exception ports are the ones that we should be 
+  // so that the current exception ports are the ones that we should be
   // forwarding to.
   ExceptionParameters current;
-  
+
   current.count = EXC_TYPES_COUNT;
   mach_port_t current_task = mach_task_self();
-  kern_return_t result = task_get_exception_ports(current_task, 
+  kern_return_t result = task_get_exception_ports(current_task,
                                                   s_exception_mask,
                                                   current.masks,
                                                   &current.count,
                                                   current.ports,
                                                   current.behaviors,
                                                   current.flavors);
-  
+
   // Find the first exception handler that matches the exception
   unsigned int found;
   for (found = 0; found < current.count; ++found) {
@@ -446,7 +446,7 @@ void *ExceptionHandler::WaitForMessage(void *exception_handler_class) {
                                     sizeof(receive), self->handler_port_,
                                     MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
 
-    
+
     if (result == KERN_SUCCESS) {
       // Uninstall our handler so that we don't get in a loop if the process of
       // writing out a minidump causes an exception.  However, if the exception
@@ -489,12 +489,12 @@ void *ExceptionHandler::WaitForMessage(void *exception_handler_class) {
 
         // When forking a child process with the exception handler installed,
         // if the child crashes, it will send the exception back to the parent
-        // process.  The check for task == self_task() ensures that only 
-        // exceptions that occur in the parent process are caught and 
+        // process.  The check for task == self_task() ensures that only
+        // exceptions that occur in the parent process are caught and
         // processed.
         if (receive.task.name == mach_task_self()) {
           self->SuspendThreads();
-          
+
 #if USE_PROTECTED_ALLOCATIONS
         if(gBreakpadAllocator)
           gBreakpadAllocator->Unprotect();
@@ -503,7 +503,7 @@ void *ExceptionHandler::WaitForMessage(void *exception_handler_class) {
           // Generate the minidump with the exception data.
           self->WriteMinidumpWithException(receive.exception, receive.code[0],
                                            receive.thread.name);
-        
+
           self->UninstallHandler(true);
 
 #if USE_PROTECTED_ALLOCATIONS
@@ -511,7 +511,7 @@ void *ExceptionHandler::WaitForMessage(void *exception_handler_class) {
           gBreakpadAllocator->Protect();
 #endif
 
-          // Pass along the exception to the server, which will setup the 
+          // Pass along the exception to the server, which will setup the
           // message and call catch_exception_raise() and put the KERN_SUCCESS
           // into the reply.
           ExceptionReplyMessage reply;
@@ -523,7 +523,7 @@ void *ExceptionHandler::WaitForMessage(void *exception_handler_class) {
                             reply.header.msgh_size, 0, MACH_PORT_NULL,
                             MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
         } else {
-          // An exception occurred in a child process 
+          // An exception occurred in a child process
         }
       }
     }
@@ -536,11 +536,11 @@ bool ExceptionHandler::InstallHandler() {
   try {
 #if USE_PROTECTED_ALLOCATIONS
     previous_ = new (gBreakpadAllocator->Allocate(sizeof(ExceptionParameters)) )
-      ExceptionParameters();    
+      ExceptionParameters();
 #else
     previous_ = new ExceptionParameters();
 #endif
-  
+
   }
   catch (std::bad_alloc) {
     return false;
@@ -549,14 +549,14 @@ bool ExceptionHandler::InstallHandler() {
   // Save the current exception ports so that we can forward to them
   previous_->count = EXC_TYPES_COUNT;
   mach_port_t current_task = mach_task_self();
-  kern_return_t result = task_get_exception_ports(current_task, 
+  kern_return_t result = task_get_exception_ports(current_task,
                                                   s_exception_mask,
                                                   previous_->masks,
                                                   &previous_->count,
                                                   previous_->ports,
                                                   previous_->behaviors,
                                                   previous_->flavors);
-  
+
   // Setup the exception ports on this task
   if (result == KERN_SUCCESS)
     result = task_set_exception_ports(current_task, s_exception_mask,
@@ -570,10 +570,10 @@ bool ExceptionHandler::InstallHandler() {
 
 bool ExceptionHandler::UninstallHandler(bool in_exception) {
   kern_return_t result = KERN_SUCCESS;
-  
+
   if (installed_exception_handler_) {
     mach_port_t current_task = mach_task_self();
-    
+
     // Restore the previous ports
     for (unsigned int i = 0; i < previous_->count; ++i) {
        result = task_set_exception_ports(current_task, previous_->masks[i],
@@ -583,23 +583,53 @@ bool ExceptionHandler::UninstallHandler(bool in_exception) {
       if (result != KERN_SUCCESS)
         return false;
     }
-    
+
     // this delete should NOT happen if an exception just occurred!
     if (!in_exception) {
 #if USE_PROTECTED_ALLOCATIONS
       previous_->~ExceptionParameters();
 #else
-      delete previous_; 
+      delete previous_;
 #endif
     }
-    
+
     previous_ = NULL;
     installed_exception_handler_ = false;
   }
-  
+
   return result == KERN_SUCCESS;
 }
 
+bool ExceptionHandler::SuspendExceptionHandling() {
+  if (!installed_exception_handler_) {
+    return false;
+  }
+
+  return UninstallHandler(false);
+}
+
+
+bool ExceptionHandler::ResumeExceptionHandling() {
+
+  if (installed_exception_handler_) {
+    return false;
+  }
+
+  // This conditional means that Setup() has never been
+  // called, but since it's called from the constructor
+  // we should never hit this.
+  assert(handler_port_);
+  if (handler_port_ == MACH_PORT_NULL) {
+    return false;
+  }
+
+  return InstallHandler();
+}
+
+bool ExceptionHandler::ExceptionHandlerIsSuspended() {
+  return handler_port_ != MACH_PORT_NULL && !installed_exception_handler_;
+}
+
 bool ExceptionHandler::Setup(bool install_handler) {
   if (pthread_mutex_init(&minidump_write_mutex_, NULL))
     return false;
@@ -623,7 +653,7 @@ bool ExceptionHandler::Setup(bool install_handler) {
     pthread_attr_t attr;
     pthread_attr_init(&attr);
     pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
-    int thread_create_result = pthread_create(&handler_thread_, &attr, 
+    int thread_create_result = pthread_create(&handler_thread_, &attr,
                                               &WaitForMessage, this);
     pthread_attr_destroy(&attr);
     result = thread_create_result ? KERN_FAILURE : KERN_SUCCESS;
@@ -638,7 +668,7 @@ bool ExceptionHandler::Teardown() {
 
   if (!UninstallHandler(false))
     return false;
-  
+
   // Send an empty message so that the handler_thread exits
   if (SendEmptyMachMessage()) {
     mach_port_t current_task = mach_task_self();
@@ -648,7 +678,7 @@ bool ExceptionHandler::Teardown() {
   } else {
     return false;
   }
-  
+
   handler_thread_ = NULL;
   handler_port_ = NULL;
   pthread_mutex_destroy(&minidump_write_mutex_);
@@ -693,7 +723,7 @@ bool ExceptionHandler::SuspendThreads() {
         return false;
     }
   }
-  
+
   return true;
 }
 
@@ -711,7 +741,7 @@ bool ExceptionHandler::ResumeThreads() {
         return false;
     }
   }
-  
+
   return true;
 }
 
diff --git a/src/client/mac/handler/exception_handler.h b/src/client/mac/handler/exception_handler.h
index 8d7755a6..1245c5e6 100644
--- a/src/client/mac/handler/exception_handler.h
+++ b/src/client/mac/handler/exception_handler.h
@@ -58,14 +58,14 @@ class ExceptionHandler {
   // will immediately report the exception as unhandled without writing a
   // minidump, allowing another handler the opportunity to handle it.
   typedef bool (*FilterCallback)(void *context);
-  
+
   // A callback function to run after the minidump has been written.
   // |minidump_id| is a unique id for the dump, so the minidump
   // file is <dump_dir>/<minidump_id>.dmp.
-  // |context| is the value passed into the constructor. 
+  // |context| is the value passed into the constructor.
   // |succeeded| indicates whether a minidump file was successfully written.
   // Return true if the exception was fully handled and breakpad should exit.
-  // Return false to allow any other exception handlers to process the 
+  // Return false to allow any other exception handlers to process the
   // exception.
   typedef bool (*MinidumpCallback)(const char *dump_dir,
                                    const char *minidump_id,
@@ -85,7 +85,7 @@ class ExceptionHandler {
   // If install_handler is true, then a minidump will be written whenever
   // an unhandled exception occurs.  If it is false, minidumps will only
   // be written when WriteMinidump is called.
-  ExceptionHandler(const string &dump_path, 
+  ExceptionHandler(const string &dump_path,
                    FilterCallback filter, MinidumpCallback callback,
                    void *callback_context, bool install_handler);
 
@@ -104,7 +104,7 @@ class ExceptionHandler {
     dump_path_c_ = dump_path_.c_str();
     UpdateNextID();  // Necessary to put dump_path_ in next_minidump_path_.
   }
-  
+
   // Writes a minidump immediately.  This can be used to capture the
   // execution state independently of a crash.  Returns true on success.
   bool WriteMinidump();
@@ -114,17 +114,32 @@ class ExceptionHandler {
   static bool WriteMinidump(const string &dump_path, MinidumpCallback callback,
                             void *callback_context);
 
+  // Temporarily stop this class from receiving exceptions
+  // Returns true if exception handling was successfully suspended
+  // It's an error to call this function if exception handling is
+  // not installed(we return false)
+  bool SuspendExceptionHandling();
+
+  // Resume this class from receiving exceptions
+  // Returns true if exception handling was successfully resumed
+  // It's an error to call this function if exception handling is
+  // already installed
+  bool ResumeExceptionHandling();
+
+  // Tell caller whether we're setup to handle exceptions or not
+  bool ExceptionHandlerIsSuspended();
+
  private:
   // Install the mach exception handler
   bool InstallHandler();
 
   // Uninstall the mach exception handler (if any)
   bool UninstallHandler(bool in_exception);
-      
+
   // Setup the handler thread, and if |install_handler| is true, install the
   // mach exception port handler
   bool Setup(bool install_handler);
-    
+
   // Uninstall the mach exception handler (if any) and terminate the helper
   // thread
   bool Teardown();
@@ -149,20 +164,20 @@ class ExceptionHandler {
   // path of the next minidump to be written in next_minidump_path_.
   void UpdateNextID();
 
-  // These functions will suspend/resume all threads except for the 
+  // 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_;
-  
+
   // The basename of the next minidump w/o extension
   string next_minidump_id_;
-  
+
   // The full path to the next minidump to be written, including extension
   string next_minidump_path_;
-  
+
   // Pointers to the UTF-8 versions of above
   const char *dump_path_c_;
   const char *next_minidump_id_c_;
@@ -191,18 +206,18 @@ class ExceptionHandler {
 
   // True, if we've installed the exception handler
   bool installed_exception_handler_;
-  
+
   // True, if we're in the process of uninstalling the exception handler and
   // the thread.
   bool is_in_teardown_;
-  
+
   // Save the last result of the last minidump
   bool last_minidump_write_result_;
-  
-  // A mutex for use when writing out a minidump that was requested on a 
+
+  // A mutex for use when writing out a minidump that was requested on a
   // thread other than the exception handler.
   pthread_mutex_t minidump_write_mutex_;
-  
+
   // True, if we're using the mutext to indicate when mindump writing occurs
   bool use_minidump_write_mutex_;
 };
diff --git a/src/client/mac/handler/minidump_generator.cc b/src/client/mac/handler/minidump_generator.cc
index 75a24b58..e15f062d 100644
--- a/src/client/mac/handler/minidump_generator.cc
+++ b/src/client/mac/handler/minidump_generator.cc
@@ -680,6 +680,26 @@ bool MinidumpGenerator::WriteModuleStream(unsigned int index,
     module->size_of_image = image->GetVMSize();
     module->module_name_rva = string_location.rva;
 
+    // We'll skip the executable module, because they don't have
+    // LC_ID_DYLIB load commands, and the crash processing server gets
+    // version information from the Plist file, anyway.
+    if (index != (uint32_t)FindExecutableModule()) {
+      module->version_info.signature = MD_VSFIXEDFILEINFO_SIGNATURE;
+      module->version_info.struct_version |= MD_VSFIXEDFILEINFO_VERSION;
+      // Convert MAC dylib version format, which is a 32 bit number, to the
+      // format used by minidump.  The mac format is <16 bits>.<8 bits>.<8 bits>
+      // so it fits nicely into the windows version with some massaging
+      // The mapping is:
+      //    1) upper 16 bits of MAC version go to lower 16 bits of product HI
+      //    2) Next most significant 8 bits go to upper 16 bits of product LO
+      //    3) Least significant 8 bits go to lower 16 bits of product LO
+      uint32_t modVersion = image->GetVersion();
+      module->version_info.file_version_hi = 0;
+      module->version_info.file_version_hi = modVersion >> 16;
+      module->version_info.file_version_lo |= (modVersion & 0xff00)  << 8;
+      module->version_info.file_version_lo |= (modVersion & 0xff);
+    }
+    
     if (!WriteCVRecord(module, cpu_type, name)) {
       return false;
     }