Bug 1286802 - Part 5: Implement ExceptionHandler::set_include_context_heap() for inprocess crash generation. r?ted draft
authorCervantes Yu <cyu@mozilla.com>
Fri, 24 Mar 2017 19:56:56 +0800
changeset 580338 52795bf3ebf3d6fa230247dad4de8b3fb361b8f6
parent 580337 726c57c2a35b6a3d1efc0ee7196de4ae259e2b8d
child 580339 b6c23fbfda5d6f91c7cf8ac1d3744a4301bb4013
push id59519
push usercyu@mozilla.com
push dateThu, 18 May 2017 11:09:20 +0000
reviewersted
bugs1286802
milestone55.0a1
Bug 1286802 - Part 5: Implement ExceptionHandler::set_include_context_heap() for inprocess crash generation. r?ted MozReview-Commit-ID: AOrGlMjnNjE
toolkit/crashreporter/breakpad-client/windows/handler/exception_handler.cc
--- a/toolkit/crashreporter/breakpad-client/windows/handler/exception_handler.cc
+++ b/toolkit/crashreporter/breakpad-client/windows/handler/exception_handler.cc
@@ -913,60 +913,28 @@ bool ExceptionHandler::WriteMinidumpWith
       if (assertion) {
         int index = user_streams.UserStreamCount;
         user_stream_array[index].Type = MD_ASSERTION_INFO_STREAM;
         user_stream_array[index].BufferSize = sizeof(MDRawAssertionInfo);
         user_stream_array[index].Buffer = assertion;
         ++user_streams.UserStreamCount;
       }
 
-      // Older versions of DbgHelp.dll don't correctly put the memory around
-      // the faulting instruction pointer into the minidump. This
-      // callback will ensure that it gets included.
-      if (exinfo) {
-        // Find a memory region of 256 bytes centered on the
-        // faulting instruction pointer.
-        const ULONG64 instruction_pointer =
-#if defined(_M_IX86)
-          exinfo->ContextRecord->Eip;
-#elif defined(_M_AMD64)
-        exinfo->ContextRecord->Rip;
-#else
-#error Unsupported platform
-#endif
-
-        MEMORY_BASIC_INFORMATION info;
-        if (VirtualQueryEx(process,
-                           reinterpret_cast<LPCVOID>(instruction_pointer),
-                           &info,
-                           sizeof(MEMORY_BASIC_INFORMATION)) != 0 &&
-            info.State == MEM_COMMIT) {
-          // Attempt to get 128 bytes before and after the instruction
-          // pointer, but settle for whatever's available up to the
-          // boundaries of the memory region.
-          const ULONG64 kIPMemorySize = 256;
-          ULONG64 base =
-            (std::max)(reinterpret_cast<ULONG64>(info.BaseAddress),
-                       instruction_pointer - (kIPMemorySize / 2));
-          ULONG64 end_of_range =
-            (std::min)(instruction_pointer + (kIPMemorySize / 2),
-                       reinterpret_cast<ULONG64>(info.BaseAddress)
-                       + info.RegionSize);
-          ULONG size = static_cast<ULONG>(end_of_range - base);
-
-          AppMemory& elt = app_memory_info_.front();
-          elt.ptr = base;
-          elt.length = size;
-        }
-      }
-
       MinidumpCallbackContext context;
       context.iter = app_memory_info_.begin();
       context.end = app_memory_info_.end();
 
+      if (exinfo) {
+        IncludeAppMemoryFromExceptionContext(process,
+                                             requesting_thread_id,
+                                             app_memory_info_,
+                                             exinfo->ContextRecord,
+                                             !include_context_heap_);
+      }
+
       // Skip the reserved element if there was no instruction memory
       if (context.iter->ptr == 0) {
         context.iter++;
       }
 
       MINIDUMP_CALLBACK_INFORMATION callback;
       callback.CallbackRoutine = MinidumpWriteDumpCallback;
       callback.CallbackParam = reinterpret_cast<void*>(&context);
@@ -1026,12 +994,30 @@ void ExceptionHandler::UnregisterAppMemo
   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::set_include_context_heap(bool enabled) {
+  if (enabled && !include_context_heap_) {
+    // Preallocate AppMemory instances for exception context if necessary.
+    auto predicate = [] (const AppMemory& appMemory) -> bool {
+      return appMemory.preallocated;
+    };
+
+    int preallocatedCount =
+      std::count_if(app_memory_info_.begin(), app_memory_info_.end(), predicate);
+
+    for (size_t i = 0; i < kExceptionAppMemoryRegions - preallocatedCount; i++) {
+      AppMemory app_memory;
+      app_memory.ptr = reinterpret_cast<ULONG64>(nullptr);
+      app_memory.length = 0;
+      app_memory.preallocated = true;
+      app_memory_info_.push_back(app_memory);
+    }
+  }
+
   include_context_heap_ = enabled;
 }
 
 }  // namespace google_breakpad