Bug 1035892 - Handle 64-bit addresses for EXC_BAD_ACCESS exceptions on Mac r=froydnj
authorGabriele Svelto <gsvelto@mozilla.com>
Thu, 15 Aug 2019 19:44:15 +0000
changeset 488341 2b1ffc8004d5e8160bc5bb1109c9f67a003048ef
parent 488340 056d9515483cd593dc1a931b4ade7ba7b143b540
child 488342 69224eff4d072682565cc98fcce41f0f4c6be8eb
push id36440
push userncsoregi@mozilla.com
push dateFri, 16 Aug 2019 03:57:48 +0000
treeherdermozilla-central@a58b7dc85887 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1035892
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1035892 - Handle 64-bit addresses for EXC_BAD_ACCESS exceptions on Mac r=froydnj Differential Revision: https://phabricator.services.mozilla.com/D24356
toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.cc
toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.h
toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.h
toolkit/crashreporter/breakpad-client/mac/handler/exception_handler.cc
toolkit/crashreporter/breakpad-client/mac/handler/exception_handler.h
toolkit/crashreporter/breakpad-client/mac/handler/minidump_generator.h
--- a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.cc
+++ b/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.cc
@@ -34,17 +34,17 @@
 
 #include "mozilla/recordreplay/ChildIPC.h"
 
 namespace google_breakpad {
 
 bool CrashGenerationClient::RequestDumpForException(
     int exception_type,
     int exception_code,
-    int exception_subcode,
+    int64_t exception_subcode,
     mach_port_t crashing_thread) {
   // The server will send a message to this port indicating that it
   // has finished its work.
   ReceivePort acknowledge_port;
 
   MachSendMessage message(kDumpRequestMessage);
   message.AddDescriptor(mach_task_self());            // this task
   message.AddDescriptor(crashing_thread);             // crashing thread
--- a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.h
+++ b/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_client.h
@@ -40,17 +40,17 @@ class CrashGenerationClient {
     : sender_(mach_port_name) {
   }
 
   // Request the crash server to generate a dump.
   //
   // Return true if the dump was successful; false otherwise.
   bool RequestDumpForException(int exception_type,
 			       int exception_code,
-			       int exception_subcode,
+			       int64_t exception_subcode,
 			       mach_port_t crashing_thread);
 
   bool RequestDump() {
     return RequestDumpForException(0, 0, 0, MACH_PORT_NULL);
   }
 
  private:
   MachPortSender sender_;
--- a/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.h
+++ b/toolkit/crashreporter/breakpad-client/mac/crash_generation/crash_generation_server.h
@@ -46,17 +46,17 @@ enum {
   kAcknowledgementMessage = 2,
   kQuitMessage            = 3
 };
 
 // Exception details sent by the client when requesting a dump.
 struct ExceptionInfo {
   int32_t exception_type;
   int32_t exception_code;
-  int32_t exception_subcode;
+  int64_t exception_subcode;
   int32_t child_pid;
 };
 
 class CrashGenerationServer {
  public:
   // WARNING: callbacks may be invoked on a different thread
   // than that which creates the CrashGenerationServer.  They must
   // be thread safe.
--- a/toolkit/crashreporter/breakpad-client/mac/handler/exception_handler.cc
+++ b/toolkit/crashreporter/breakpad-client/mac/handler/exception_handler.cc
@@ -84,148 +84,205 @@ static union {
 #endif  // USE_PROTECTED_ALLOCATIONS
   google_breakpad::ExceptionHandler *handler;
 } gProtectedData;
 
 using std::map;
 
 // These structures and techniques are illustrated in
 // Mac OS X Internals, Amit Singh, ch 9.7
+#pragma pack(push, 4)
 struct ExceptionMessage {
   mach_msg_header_t           header;
   mach_msg_body_t             body;
   mach_msg_port_descriptor_t  thread;
   mach_msg_port_descriptor_t  task;
   NDR_record_t                ndr;
   exception_type_t            exception;
   mach_msg_type_number_t      code_count;
-  integer_t                   code[EXCEPTION_CODE_MAX];
+  mach_exception_data_type_t  code[EXCEPTION_CODE_MAX];
   char                        padding[512];
 };
+#pragma pack(pop)
 
 struct ExceptionParameters {
   ExceptionParameters() : count(0) {}
   mach_msg_type_number_t count;
   exception_mask_t masks[EXC_TYPES_COUNT];
   mach_port_t ports[EXC_TYPES_COUNT];
   exception_behavior_t behaviors[EXC_TYPES_COUNT];
   thread_state_flavor_t flavors[EXC_TYPES_COUNT];
 };
 
+#pragma pack(push, 4)
 struct ExceptionReplyMessage {
   mach_msg_header_t  header;
   NDR_record_t       ndr;
   kern_return_t      return_code;
 };
+#pragma pack(pop)
 
 // 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 |
 EXC_MASK_BAD_INSTRUCTION | EXC_MASK_ARITHMETIC | EXC_MASK_BREAKPOINT;
 
-#if !TARGET_OS_IPHONE
-extern "C" {
-  // Forward declarations for functions that need "C" style compilation
-  boolean_t exc_server(mach_msg_header_t* request,
-                       mach_msg_header_t* reply);
-
-  // This symbol must be visible to dlsym() - see
-  // https://bugs.chromium.org/p/google-breakpad/issues/detail?id=345 for details.
-  kern_return_t catch_exception_raise(mach_port_t target_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)
-      __attribute__((visibility("default")));
-}
-#endif
-
 kern_return_t ForwardException(mach_port_t task,
                                mach_port_t failed_thread,
                                exception_type_t exception,
-                               exception_data_t code,
+                               mach_exception_data_t code,
                                mach_msg_type_number_t code_count);
 
-#if TARGET_OS_IPHONE
-// Implementation is based on the implementation generated by mig.
-boolean_t breakpad_exc_server(mach_msg_header_t* InHeadP,
-                              mach_msg_header_t* OutHeadP) {
+// The contents of mach_exc_server() and mach_exception_raise() are derived
+// from /usr/include/mach/mach_exc.defs, as follows:
+//
+// 1) Run 'mig mach_exc.defs' which creates the following files:
+//    mach_exc.h
+//    mach_excServer.c
+//    mach_excUser.c
+// 2) The relevant code for mach_exc_server() comes from the following methods
+//    in mach_excServer.c:
+//      mach_exc_server()
+//      _Xmach_exception_raise()
+// 3) The relevant code for mach_exception_raise() comes from the following
+//    method in mach_excUser.c:
+//      mach_exception_raise()
+boolean_t mach_exc_server(mach_msg_header_t* InHeadP,
+                          mach_msg_header_t* OutHeadP)
+{
   OutHeadP->msgh_bits =
-      MACH_MSGH_BITS(MACH_MSGH_BITS_REMOTE(InHeadP->msgh_bits), 0);
+    MACH_MSGH_BITS(MACH_MSGH_BITS_REMOTE(InHeadP->msgh_bits), 0);
   OutHeadP->msgh_remote_port = InHeadP->msgh_remote_port;
-  /* Minimal size: routine() will update it if different */
+  /* Minimal size: mach_exception_raise() will update it if different */
   OutHeadP->msgh_size = (mach_msg_size_t)sizeof(mig_reply_error_t);
   OutHeadP->msgh_local_port = MACH_PORT_NULL;
   OutHeadP->msgh_id = InHeadP->msgh_id + 100;
+  OutHeadP->msgh_reserved = 0;
 
-  if (InHeadP->msgh_id != 2401) {
+  if (InHeadP->msgh_id != 2405) {
     ((mig_reply_error_t*)OutHeadP)->NDR = NDR_record;
     ((mig_reply_error_t*)OutHeadP)->RetCode = MIG_BAD_ID;
     return FALSE;
   }
 
-#ifdef  __MigPackStructs
-#pragma pack(4)
-#endif
+#pragma pack(push, 4)
   typedef struct {
     mach_msg_header_t Head;
     /* start of the kernel processed data */
     mach_msg_body_t msgh_body;
     mach_msg_port_descriptor_t thread;
     mach_msg_port_descriptor_t task;
     /* end of the kernel processed data */
     NDR_record_t NDR;
     exception_type_t exception;
     mach_msg_type_number_t codeCnt;
-    integer_t code[2];
+    int64_t code[2];
     mach_msg_trailer_t trailer;
   } Request;
 
   typedef struct {
     mach_msg_header_t Head;
     NDR_record_t NDR;
     kern_return_t RetCode;
   } Reply;
-#ifdef  __MigPackStructs
-#pragma pack()
-#endif
+#pragma pack(pop)
 
   Request* In0P = (Request*)InHeadP;
   Reply* OutP = (Reply*)OutHeadP;
 
   if (In0P->task.name != mach_task_self()) {
     return FALSE;
   }
   OutP->RetCode = ForwardException(In0P->task.name,
                                    In0P->thread.name,
                                    In0P->exception,
                                    In0P->code,
                                    In0P->codeCnt);
   OutP->NDR = NDR_record;
   return TRUE;
 }
-#else
-boolean_t breakpad_exc_server(mach_msg_header_t* request,
-                              mach_msg_header_t* reply) {
-  return exc_server(request, reply);
-}
+
+kern_return_t mach_exception_raise(mach_port_t exception_port,
+                                   mach_port_t thread,
+                                   mach_port_t task,
+                                   exception_type_t exception,
+                                   mach_exception_data_t code,
+                                   mach_msg_type_number_t codeCnt)
+{
+#pragma pack(push, 4)
+  typedef struct {
+    mach_msg_header_t Head;
+    /* start of the kernel processed data */
+    mach_msg_body_t msgh_body;
+    mach_msg_port_descriptor_t thread;
+    mach_msg_port_descriptor_t task;
+    /* end of the kernel processed data */
+    NDR_record_t NDR;
+    exception_type_t exception;
+    mach_msg_type_number_t codeCnt;
+    int64_t code[2];
+  } Request;
+
+  typedef struct {
+    mach_msg_header_t Head;
+    NDR_record_t NDR;
+    kern_return_t RetCode;
+    mach_msg_trailer_t trailer;
+  } Reply;
+#pragma pack(pop)
+
+  Request In;
+  Request *InP = &In;
+
+  mach_msg_return_t msg_result;
+  unsigned int msgh_size;
 
-// Callback from exc_server()
-kern_return_t catch_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) {
-  if (task != mach_task_self()) {
-    return KERN_FAILURE;
+  InP->msgh_body.msgh_descriptor_count = 2;
+  InP->thread.name = thread;
+  InP->thread.disposition = 19;
+  InP->thread.type = MACH_MSG_PORT_DESCRIPTOR;
+  InP->task.name = task;
+  InP->task.disposition = 19;
+  InP->task.type = MACH_MSG_PORT_DESCRIPTOR;
+
+  InP->NDR = NDR_record;
+
+  InP->exception = exception;
+
+  if (codeCnt > 2) {
+    { return MIG_ARRAY_TOO_LARGE; }
   }
-  return ForwardException(task, failed_thread, exception, code, code_count);
+  (void)memcpy((char *) InP->code, (const char *) code, 8 * codeCnt);
+
+  InP->codeCnt = codeCnt;
+
+  msgh_size = (mach_msg_size_t)(sizeof(Request) - 16) + ((8 * codeCnt));
+  InP->Head.msgh_bits = MACH_MSGH_BITS_COMPLEX |
+    MACH_MSGH_BITS(19, MACH_MSG_TYPE_MAKE_SEND_ONCE);
+  /* msgh_size passed as argument */
+  InP->Head.msgh_remote_port = exception_port;
+  InP->Head.msgh_local_port = mig_get_reply_port();
+  InP->Head.msgh_id = 2405;
+  InP->Head.msgh_reserved = 0;
+
+  msg_result = mach_msg(&InP->Head,
+                        MACH_SEND_MSG|MACH_RCV_MSG|MACH_MSG_OPTION_NONE,
+                        msgh_size,
+                        (mach_msg_size_t)sizeof(Reply),
+                        InP->Head.msgh_local_port,
+                        MACH_MSG_TIMEOUT_NONE,
+                        MACH_PORT_NULL);
+
+  if (msg_result != MACH_MSG_SUCCESS) {
+    fprintf(stderr, "** mach_msg() error forwarding exception!!\n");
+    return msg_result;
+  }
+
+  return KERN_SUCCESS;
 }
-#endif
 
 ExceptionHandler::ExceptionHandler(const string &dump_path,
                                    FilterCallback filter,
                                    MinidumpCallback callback,
                                    void* callback_context,
                                    bool install_handler,
                                    const char* port_name)
     : dump_path_(),
@@ -346,17 +403,17 @@ bool ExceptionHandler::WriteMinidumpForC
                     callback_context, nullptr, result);
   }
   return result;
 }
 
 bool ExceptionHandler::WriteMinidumpWithException(
     int exception_type,
     int exception_code,
-    int exception_subcode,
+    int64_t exception_subcode,
     breakpad_ucontext_t* task_context,
     mach_port_t thread_name,
     bool exit_after_write,
     bool report_current_thread) {
   bool result = false;
 
 #if TARGET_OS_IPHONE
   // _exit() should never be called on iOS.
@@ -425,17 +482,17 @@ bool ExceptionHandler::WriteMinidumpWith
     }
   }
 
   return result;
 }
 
 kern_return_t ForwardException(mach_port_t task, mach_port_t failed_thread,
                                exception_type_t exception,
-                               exception_data_t code,
+                               mach_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
   // forwarding to.
   ExceptionParameters current;
 
   current.count = EXC_TYPES_COUNT;
   mach_port_t current_task = mach_task_self();
@@ -460,22 +517,20 @@ kern_return_t ForwardException(mach_port
     fprintf(stderr, "** No previous ports for forwarding!! \n");
     exit(KERN_FAILURE);
   }
 
   mach_port_t target_port = current.ports[found];
   exception_behavior_t target_behavior = current.behaviors[found];
 
   kern_return_t result;
-  // TODO: Handle the case where |target_behavior| has MACH_EXCEPTION_CODES
-  // set. https://bugs.chromium.org/p/google-breakpad/issues/detail?id=551
-  switch (target_behavior) {
+  switch (target_behavior & ~MACH_EXCEPTION_CODES) {
     case EXCEPTION_DEFAULT:
-      result = exception_raise(target_port, failed_thread, task, exception,
-                               code, code_count);
+      result = mach_exception_raise(target_port, failed_thread, task, exception,
+                                    code, code_count);
       break;
     default:
       fprintf(stderr, "** Unknown exception behavior: %d\n", target_behavior);
       result = KERN_FAILURE;
       break;
   }
 
   return result;
@@ -555,29 +610,26 @@ void* ExceptionHandler::WaitForMessage(v
 
         if (self->use_minidump_write_mutex_)
           pthread_mutex_unlock(&self->minidump_write_mutex_);
       } else {
         // 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
-        // processed.  If the exception was not caused by this task, we
-        // still need to call into the exception server and have it return
-        // KERN_FAILURE (see catch_exception_raise) in order for the kernel
-        // to move onto the host exception handler for the child task
+        // processed.
         if (receive.task.name == mach_task_self()) {
           self->SuspendThreads();
 
 #if USE_PROTECTED_ALLOCATIONS
           if (gBreakpadAllocator)
             gBreakpadAllocator->Unprotect();
 #endif
 
-          int subcode = 0;
+          mach_exception_data_type_t subcode = 0;
           if (receive.exception == EXC_BAD_ACCESS && receive.code_count > 1)
             subcode = receive.code[1];
 
           // Generate the minidump with the exception data.
           self->WriteMinidumpWithException(receive.exception, receive.code[0],
                                            subcode, NULL, receive.thread.name,
                                            true, false);
 
@@ -590,27 +642,20 @@ void* ExceptionHandler::WaitForMessage(v
 #endif
 
           self->UninstallHandler(true);
 
 #if USE_PROTECTED_ALLOCATIONS
           if (gBreakpadAllocator)
             gBreakpadAllocator->Protect();
 #endif
-          // It's not safe to call exc_server with threads suspended.
-          // exc_server can trigger dlsym(3) calls which deadlock if
-          // another thread is paused while in dlopen(3).
-          self->ResumeThreads();
         }
 
-        // Pass along the exception to the server, which will setup the
-        // message and call catch_exception_raise() and put the return
-        // code into the reply.
         ExceptionReplyMessage reply;
-        if (!breakpad_exc_server(&receive.header, &reply.header))
+        if (!mach_exc_server(&receive.header, &reply.header))
           exit(1);
 
         // Send a reply and exit
         mach_msg(&(reply.header), MACH_SEND_MSG,
                  reply.header.msgh_size, 0, MACH_PORT_NULL,
                  MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
       }
     }
@@ -702,20 +747,23 @@ bool ExceptionHandler::InstallHandler() 
   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
+  // Setup the exception ports on this task.  Such documentation as exists for
+  // task_set_exception_port() and friends can be found in the source code for
+  // xnu.  Apple's implementation is available at https://opensource.apple.com/.
   if (result == KERN_SUCCESS)
     result = task_set_exception_ports(current_task, s_exception_mask,
-                                      handler_port_, EXCEPTION_DEFAULT,
+                                      handler_port_,
+                                      EXCEPTION_DEFAULT | MACH_EXCEPTION_CODES,
                                       THREAD_STATE_NONE);
 
   installed_exception_handler_ = (result == KERN_SUCCESS);
 
   return installed_exception_handler_;
 }
 
 bool ExceptionHandler::UninstallHandler(bool in_exception) {
--- a/toolkit/crashreporter/breakpad-client/mac/handler/exception_handler.h
+++ b/toolkit/crashreporter/breakpad-client/mac/handler/exception_handler.h
@@ -99,17 +99,17 @@ class ExceptionHandler {
                                    bool succeeded);
 
   // A callback function which will be called directly if an exception occurs.
   // This bypasses the minidump file writing and simply gives the client
   // the exception information.
   typedef bool (*DirectCallback)( void *context,
                                   int exception_type,
                                   int exception_code,
-                                  int exception_subcode,
+                                  int64_t exception_subcode,
                                   mach_port_t thread_name);
 
   // Creates a new ExceptionHandler instance to handle writing minidumps.
   // Minidump files will be written to dump_path, and the optional callback
   // is called after writing the dump file, as described above.
   // 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.
@@ -198,17 +198,17 @@ class ExceptionHandler {
   // success, false otherwise.
   bool SendMessageToHandlerThread(HandlerThreadMessage message_id);
 
   // All minidump writing goes through this one routine.
   // |task_context| can be NULL. If not, it will be used to retrieve the
   // context of the current thread, instead of using |thread_get_state|.
   bool WriteMinidumpWithException(int exception_type,
                                   int exception_code,
-                                  int exception_subcode,
+                                  int64_t exception_subcode,
                                   breakpad_ucontext_t *task_context,
                                   mach_port_t thread_name,
                                   bool exit_after_write,
                                   bool report_current_thread);
 
   // When installed, this static function will be call from a newly created
   // pthread with |this| as the argument
   static void *WaitForMessage(void *exception_handler_class);
--- a/toolkit/crashreporter/breakpad-client/mac/handler/minidump_generator.h
+++ b/toolkit/crashreporter/breakpad-client/mac/handler/minidump_generator.h
@@ -92,17 +92,17 @@ class MinidumpGenerator {
   static string UniqueNameInDirectory(const string &dir, string *unique_name);
 
   // Write out the minidump into |path|
   // All of the components of |path| must exist and be writable
   // Return true if successful, false otherwise
   bool Write(const char *path);
 
   // Specify some exception information, if applicable
-  void SetExceptionInformation(int type, int code, int subcode,
+  void SetExceptionInformation(int type, int code, int64_t subcode,
                                mach_port_t thread_name) {
     exception_type_ = type;
     exception_code_ = code;
     exception_subcode_ = subcode;
     exception_thread_ = thread_name;
   }
 
   // Specify the task context. If |task_context| is not NULL, it will be used
@@ -195,17 +195,17 @@ class MinidumpGenerator {
  protected:
   // Use this writer to put the data to disk
   MinidumpFileWriter writer_;
 
  private:
   // Exception information
   int exception_type_;
   int exception_code_;
-  int exception_subcode_;
+  int64_t exception_subcode_;
   mach_port_t exception_thread_;
   mach_port_t crashing_task_;
   mach_port_t handler_thread_;
 
   // CPU type of the task being dumped.
   cpu_type_t cpu_type_;
 
   // System information