Use a MinidumpCallback to force minidumps on Windows to include memory around the faulting instruction pointer. Older versions of DbgHelp don't seem to do this correctly (on Windows XP, for example)

R=mark at http://breakpad.appspot.com/259001

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

2 files changed, 100 insertions, 1 deletions

diff --git a/src/client/windows/handler/exception_handler.cc b/src/client/windows/handler/exception_handler.cc
index f02544cd..9c0593b4 100644
--- a/src/client/windows/handler/exception_handler.cc
+++ b/src/client/windows/handler/exception_handler.cc
@@ -28,6 +28,8 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <ObjBase.h>
+
+#include <algorithm>
 #include <cassert>
 #include <cstdio>
 
@@ -44,6 +46,13 @@ namespace google_breakpad {
 static const int kWaitForHandlerThreadMs = 60000;
 static const int kExceptionHandlerThreadInitialStackSize = 64 * 1024;
 
+// This is passed as the context to the MinidumpWriteDump callback.
+typedef struct {
+  ULONG64 memory_base;
+  ULONG memory_size;
+  bool finished;
+} MinidumpCallbackContext;
+
 vector<ExceptionHandler*>* ExceptionHandler::handler_stack_ = NULL;
 LONG ExceptionHandler::handler_stack_index_ = 0;
 CRITICAL_SECTION ExceptionHandler::handler_stack_critical_section_;
@@ -757,6 +766,50 @@ bool ExceptionHandler::WriteMinidumpWithException(
           ++user_streams.UserStreamCount;
         }
 
+        MINIDUMP_CALLBACK_INFORMATION callback;
+        MinidumpCallbackContext context;
+        MINIDUMP_CALLBACK_INFORMATION* callback_pointer = NULL;
+        // 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 (VirtualQuery(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;
+            context.memory_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);
+            context.memory_size =
+                static_cast<ULONG>(end_of_range - context.memory_base);
+
+            context.finished = false;
+            callback.CallbackRoutine = MinidumpWriteDumpCallback;
+            callback.CallbackParam = reinterpret_cast<void*>(&context);
+            callback_pointer = &callback;
+          }
+        }
+
         // The explicit comparison to TRUE avoids a warning (C4800).
         success = (minidump_write_dump_(GetCurrentProcess(),
                                         GetCurrentProcessId(),
@@ -764,7 +817,7 @@ bool ExceptionHandler::WriteMinidumpWithException(
                                         dump_type_,
                                         exinfo ? &except_info : NULL,
                                         &user_streams,
-                                        NULL) == TRUE);
+                                        callback_pointer) == TRUE);
 
         CloseHandle(dump_file);
       }
@@ -783,6 +836,45 @@ bool ExceptionHandler::WriteMinidumpWithException(
   return success;
 }
 
+// static
+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);
+    if (callback_context->finished)
+      return FALSE;
+
+    // Include the specified memory region.
+    callback_output->MemoryBase = callback_context->memory_base;
+    callback_output->MemorySize = callback_context->memory_size;
+    callback_context->finished = true;
+    return TRUE;
+  }
+    
+    // Include all modules.
+  case IncludeModuleCallback:
+  case ModuleCallback:
+    return TRUE;
+
+    // Include all threads.
+  case IncludeThreadCallback:
+  case ThreadCallback:
+    return TRUE;
+
+    // Stop receiving cancel callbacks.
+  case CancelCallback:
+    callback_output->CheckCancel = FALSE;
+    callback_output->Cancel = FALSE;
+    return TRUE;
+  }
+  // Ignore other callback types.
+  return FALSE;
+}
+
 void ExceptionHandler::UpdateNextID() {
   assert(uuid_create_);
   UUID id = {0};
diff --git a/src/client/windows/handler/exception_handler.h b/src/client/windows/handler/exception_handler.h
index 2cacdc38..2c2e7b76 100644
--- a/src/client/windows/handler/exception_handler.h
+++ b/src/client/windows/handler/exception_handler.h
@@ -277,6 +277,13 @@ class ExceptionHandler {
                                   EXCEPTION_POINTERS* exinfo,
                                   MDRawAssertionInfo* assertion);
 
+  // This function is used as a callback when calling MinidumpWriteDump,
+  // in order to add additional memory regions to the dump.
+  static BOOL CALLBACK MinidumpWriteDumpCallback(
+      PVOID context,
+      const PMINIDUMP_CALLBACK_INPUT callback_input,
+      PMINIDUMP_CALLBACK_OUTPUT callback_output);
+
   // 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();