Bug 1286802 - Part 2: Add support for registering app memory under exception context. r?ted draft
authorCervantes Yu <cyu@mozilla.com>
Mon, 26 Sep 2016 17:59:29 +0800
changeset 421468 0b0dcd17c403b457ee81dcc47a399056744fc287
parent 421467 fce88044f0a6fe86a6a50e88f00e38bd560b156e
child 421469 ceee31aa86b384abba46a5d14c9ccc291ed22257
push id31517
push usercyu@mozilla.com
push dateThu, 06 Oct 2016 07:48:12 +0000
reviewersted
bugs1286802
milestone52.0a1
Bug 1286802 - Part 2: Add support for registering app memory under exception context. r?ted MozReview-Commit-ID: KTBsuMCOzQY
toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.cc
toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.h
toolkit/crashreporter/google-breakpad/src/client/linux/minidump_writer/minidump_writer.cc
toolkit/crashreporter/google-breakpad/src/client/linux/minidump_writer/minidump_writer.h
toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.cc
toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.h
toolkit/crashreporter/nsExceptionHandler.cpp
--- a/toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.cc
+++ b/toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.cc
@@ -420,17 +420,17 @@ int ExceptionHandler::ThreadEntry(void *
 
   return thread_arg->handler->DoDump(thread_arg->pid, thread_arg->context,
                                      thread_arg->context_size) == false;
 }
 
 // This function runs in a compromised context: see the top of the file.
 // Runs on the crashing thread.
 bool ExceptionHandler::HandleSignal(int sig, siginfo_t* info, void* uc) {
-  if (filter_ && !filter_(callback_context_))
+  if (filter_ && !filter_(callback_context_, uc))
     return false;
 
   // Allow ourselves to be dumped if the signal is trusted.
   bool signal_trusted = info->si_code > 0;
   bool signal_pid_trusted = info->si_code == SI_USER ||
       info->si_code == SI_TKILL;
   if (signal_trusted || (signal_pid_trusted && info->si_pid == getpid())) {
     sys_prctl(PR_SET_DUMPABLE, 1, 0, 0, 0);
@@ -742,38 +742,64 @@ void ExceptionHandler::AddMappingInfo(co
   info.name[sizeof(info.name) - 1] = '\0';
 
   MappingEntry mapping;
   mapping.first = info;
   memcpy(mapping.second, identifier, sizeof(MDGUID));
   mapping_list_.push_back(mapping);
 }
 
-void ExceptionHandler::RegisterAppMemory(void* ptr, size_t length) {
+void ExceptionHandler::RegisterAppMemory(void* ptr,
+                                         size_t length,
+                                         bool exception_safe) {
   AppMemoryList::iterator iter =
     std::find(app_memory_list_.begin(), app_memory_list_.end(), ptr);
   if (iter != app_memory_list_.end()) {
     // Don't allow registering the same pointer twice.
     return;
   }
 
+  // Handle exception_safe request: no memory allocation allowed.
+  if (exception_safe) {
+    AppMemoryList::iterator iter =
+      std::find(app_memory_list_.begin(), app_memory_list_.end(), nullptr);
+    if (iter == app_memory_list_.end() || !iter->exception_safe) {
+      return;
+    }
+
+    iter->ptr = ptr;
+    iter->length = length;
+    return;
+  }
+
   AppMemory app_memory;
   app_memory.ptr = ptr;
   app_memory.length = length;
+  app_memory.exception_safe = exception_safe;
   app_memory_list_.push_back(app_memory);
 }
 
 void ExceptionHandler::UnregisterAppMemory(void* ptr) {
   AppMemoryList::iterator iter =
     std::find(app_memory_list_.begin(), app_memory_list_.end(), ptr);
   if (iter != app_memory_list_.end()) {
     app_memory_list_.erase(iter);
   }
 }
 
+void ExceptionHandler::ReserveExceptionSafeAppMemory(size_t n) {
+  AppMemory app_memory;
+  for (size_t i = 0; i < n; i++) {
+    app_memory.ptr = nullptr;
+    app_memory.length = 0;
+    app_memory.exception_safe = true;
+    app_memory_list_.push_back(app_memory);
+  }
+}
+
 // static
 bool ExceptionHandler::WriteMinidumpForChild(pid_t child,
                                              pid_t child_blamed_thread,
                                              const string& dump_path,
                                              MinidumpCallback callback,
                                              void* callback_context) {
   // This function is not run in a compromised context.
   MinidumpDescriptor descriptor(dump_path);
--- a/toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.h
+++ b/toolkit/crashreporter/google-breakpad/src/client/linux/handler/exception_handler.h
@@ -77,17 +77,17 @@ class ExceptionHandler {
   // processing of an exception.  A FilterCallback is called before writing
   // a minidump.  |context| is the parameter supplied by the user as
   // callback_context when the handler was created.
   //
   // If a FilterCallback returns true, Breakpad will continue processing,
   // attempting to write a minidump.  If a FilterCallback returns false,
   // Breakpad  will immediately report the exception as unhandled without
   // writing a minidump, allowing another handler the opportunity to handle it.
-  typedef bool (*FilterCallback)(void *context);
+  typedef bool (*FilterCallback)(void *context, void *uc);
 
   // A callback function to run after the minidump has been written.
   // |descriptor| contains the file descriptor or file path containing the
   // minidump. |context| is the parameter supplied by the user as
   // callback_context when the handler was created.  |succeeded| indicates
   // whether a minidump file was successfully written.
   //
   // If an exception occurred and the callback returns true, Breakpad will
@@ -211,21 +211,28 @@ class ExceptionHandler {
   void AddMappingInfo(const string& name,
                       const uint8_t identifier[sizeof(MDGUID)],
                       uintptr_t start_address,
                       size_t mapping_size,
                       size_t file_offset);
 
   // Register a block of memory of length bytes starting at address ptr
   // to be copied to the minidump when a crash happens.
-  void RegisterAppMemory(void* ptr, size_t length);
+  // Use exception_safe = true under exception context, but it doesn't guarantee
+  // to succeed if the items added in ReserveExceptionSafeAppMemory() have all
+  // been used.
+  void RegisterAppMemory(void* ptr, size_t length, bool exception_safe = false);
 
   // Unregister a block of memory that was registered with RegisterAppMemory.
   void UnregisterAppMemory(void* ptr);
 
+  // Call ReserveExceptionSafeAppMemory(n) cases n items in app_memory_list_
+  // to be allocated for exception-safe RegisterAppMemory() calls.
+  void ReserveExceptionSafeAppMemory(size_t n);
+
   // Force signal handling for the specified signal.
   bool SimulateSignalDelivery(int sig);
 
   // Report a crash signal from an SA_SIGINFO signal handler.
   bool HandleSignal(int sig, siginfo_t* info, void* uc);
 
  private:
   // Save the old signal handlers and install new ones.
--- a/toolkit/crashreporter/google-breakpad/src/client/linux/minidump_writer/minidump_writer.cc
+++ b/toolkit/crashreporter/google-breakpad/src/client/linux/minidump_writer/minidump_writer.cc
@@ -425,16 +425,19 @@ class MinidumpWriter {
     return true;
   }
 
   // Write application-provided memory regions.
   bool WriteAppMemory() {
     for (AppMemoryList::const_iterator iter = app_memory_list_.begin();
          iter != app_memory_list_.end();
          ++iter) {
+      if (iter->ptr == NULL && iter->exception_safe) {
+        continue;
+      }
       uint8_t* data_copy =
         reinterpret_cast<uint8_t*>(dumper_->allocator()->Alloc(iter->length));
       dumper_->CopyFromProcess(data_copy, GetCrashThread(), iter->ptr,
                                iter->length);
 
       UntypedMDRVA memory(&minidump_writer_);
       if (!memory.Allocate(iter->length)) {
         return false;
--- a/toolkit/crashreporter/google-breakpad/src/client/linux/minidump_writer/minidump_writer.h
+++ b/toolkit/crashreporter/google-breakpad/src/client/linux/minidump_writer/minidump_writer.h
@@ -51,16 +51,17 @@ typedef struct fpsimd_context fpstate_t;
 typedef struct _libc_fpstate fpstate_t;
 #endif
 
 // These entries store a list of memory regions that the client wants included
 // in the minidump.
 struct AppMemory {
   void* ptr;
   size_t length;
+  bool exception_safe;
 
   bool operator==(const struct AppMemory& other) const {
     return ptr == other.ptr;
   }
 
   bool operator==(const void* other) const {
     return ptr == other;
   }
--- a/toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.cc
+++ b/toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.cc
@@ -869,16 +869,22 @@ bool ExceptionHandler::WriteMinidumpWith
 BOOL CALLBACK ExceptionHandler::MinidumpWriteDumpCallback(
     PVOID context,
     const PMINIDUMP_CALLBACK_INPUT callback_input,
     PMINIDUMP_CALLBACK_OUTPUT callback_output) {
   switch (callback_input->CallbackType) {
   case MemoryCallback: {
     MinidumpCallbackContext* callback_context =
         reinterpret_cast<MinidumpCallbackContext*>(context);
+    // Skip the unused exception_safe slots
+    while (callback_context->iter->ptr == NULL &&
+           callback_context->iter->exception_safe) {
+      callback_context->iter++;
+    }
+
     if (callback_context->iter == callback_context->end)
       return FALSE;
 
     // Include the specified memory region.
     callback_output->MemoryBase = callback_context->iter->ptr;
     callback_output->MemorySize = callback_context->iter->length;
     callback_context->iter++;
     return TRUE;
@@ -1045,31 +1051,56 @@ void ExceptionHandler::UpdateNextID() {
 
   // remove when VC++7.1 is no longer supported
   minidump_path[MAX_PATH - 1] = L'\0';
 
   next_minidump_path_ = minidump_path;
   next_minidump_path_c_ = next_minidump_path_.c_str();
 }
 
-void ExceptionHandler::RegisterAppMemory(void* ptr, size_t length) {
+void ExceptionHandler::RegisterAppMemory(void* ptr, size_t length,
+                                         bool exception_safe) {
   AppMemoryList::iterator iter =
     std::find(app_memory_info_.begin(), app_memory_info_.end(), ptr);
   if (iter != app_memory_info_.end()) {
     // Don't allow registering the same pointer twice.
     return;
   }
 
+  // Handle exception_safe request: no memory allocation allowed.
+  if (exception_safe) {
+    AppMemoryList::iterator iter =
+      std::find(app_memory_info_.begin(), app_memory_info_.end(), nullptr);
+    if (iter == app_memory_info_.end() || !iter->exception_safe) {
+      return;
+    }
+
+    iter->ptr = reinterpret_cast<ULONG64>(ptr);
+    iter->length = static_cast<ULONG>(length);
+    return;
+  }
+
   AppMemory app_memory;
   app_memory.ptr = reinterpret_cast<ULONG64>(ptr);
   app_memory.length = static_cast<ULONG>(length);
+  app_memory.exception_safe = exception_safe;
   app_memory_info_.push_back(app_memory);
 }
 
 void ExceptionHandler::UnregisterAppMemory(void* ptr) {
   AppMemoryList::iterator iter =
     std::find(app_memory_info_.begin(), app_memory_info_.end(), ptr);
   if (iter != app_memory_info_.end()) {
     app_memory_info_.erase(iter);
   }
 }
 
+void ExceptionHandler::ReserveExceptionSafeAppMemory(size_t n) {
+  AppMemory app_memory;
+  for (size_t i = 0; i < n; i++) {
+    app_memory.ptr = reinterpret_cast<ULONG64>(nullptr);
+    app_memory.length = static_cast<ULONG>(0);
+    app_memory.exception_safe = true;
+    app_memory_info_.push_back(app_memory);
+  }
+}
+
 }  // namespace google_breakpad
--- a/toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.h
+++ b/toolkit/crashreporter/google-breakpad/src/client/windows/handler/exception_handler.h
@@ -79,16 +79,17 @@ namespace google_breakpad {
 using std::vector;
 using std::wstring;
 
 // These entries store a list of memory regions that the client wants included
 // in the minidump.
 struct AppMemory {
   ULONG64 ptr;
   ULONG length;
+  bool exception_safe;
 
   bool operator==(const struct AppMemory& other) const {
     return ptr == other.ptr;
   }
 
   bool operator==(const void* other) const {
     return ptr == reinterpret_cast<ULONG64>(other);
   }
@@ -274,19 +275,26 @@ class ExceptionHandler {
     consume_invalid_handle_exceptions_ = consume_invalid_handle_exceptions;
   }
 
   // Returns whether out-of-process dump generation is used or not.
   bool IsOutOfProcess() const { return crash_generation_client_.get() != NULL; }
 
   // Calling RegisterAppMemory(p, len) causes len bytes starting
   // at address p to be copied to the minidump when a crash happens.
-  void RegisterAppMemory(void* ptr, size_t length);
+  // Use exception_safe = true under exception context, but it doesn't guarantee
+  // to succeed if the items added in ReserveExceptionSafeAppMemory() have all
+  // been used.
+  void RegisterAppMemory(void* ptr, size_t length, bool exception_safe = false);
   void UnregisterAppMemory(void* ptr);
 
+  // Call ReserveExceptionSafeAppMemory(n) cases n items in app_memory_list_
+  // to be allocated for exception-safe RegisterAppMemory() calls.
+  void ReserveExceptionSafeAppMemory(size_t n);
+
  private:
   friend class AutoExceptionHandler;
 
   // Initializes the instance with given values.
   void Initialize(const wstring& dump_path,
                   FilterCallback filter,
                   MinidumpCallback callback,
                   void* callback_context,
--- a/toolkit/crashreporter/nsExceptionHandler.cpp
+++ b/toolkit/crashreporter/nsExceptionHandler.cpp
@@ -1466,26 +1466,38 @@ static bool ShouldReport()
   if (envvar && *envvar) {
     return false;
   }
 
   return true;
 }
 
 static bool
-Filter(void* context)
+Filter(void* context
+#ifdef XP_LINUX
+       , void* uc
+#endif
+       )
 {
   mozilla::IOInterposer::Disable();
   return true;
 }
 
 static bool
-ChildFilter(void* context)
+ChildFilter(void* context
+#ifdef XP_LINUX
+       , void* uc
+#endif
+            )
 {
-  bool result = Filter(context);
+  bool result = Filter(context
+#ifdef XP_LINUX
+                       , uc
+#endif
+                       );
   if (result) {
     PrepareChildExceptionTimeAnnotations();
   }
   return result;
 }
 
 nsresult SetExceptionHandler(nsIFile* aXREDirectory,
                              bool force/*=false*/)