Write a window of memory around the instruction pointer from the crashing thread to the minidump on Linux.

R=nealsid at http://breakpad.appspot.com/194001/show

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

2 files changed, 214 insertions, 2 deletions

diff --git a/src/client/linux/handler/exception_handler_unittest.cc b/src/client/linux/handler/exception_handler_unittest.cc
index 731207a8..3fea1159 100644
--- a/src/client/linux/handler/exception_handler_unittest.cc
+++ b/src/client/linux/handler/exception_handler_unittest.cc
@@ -32,6 +32,7 @@
 #include <stdint.h>
 #include <unistd.h>
 #include <signal.h>
+#include <sys/mman.h>
 #include <sys/poll.h>
 #include <sys/socket.h>
 #include <sys/uio.h>
@@ -42,6 +43,7 @@
 #include "common/linux/eintr_wrapper.h"
 #include "common/linux/linux_libc_support.h"
 #include "third_party/lss/linux_syscall_support.h"
+#include "google_breakpad/processor/minidump.h"
 
 using namespace google_breakpad;
 
@@ -126,6 +128,127 @@ TEST(ExceptionHandlerTest, ChildCrash) {
   ASSERT_EQ(stat(minidump_filename.c_str(), &st), 0);
   ASSERT_GT(st.st_size, 0u);
   unlink(minidump_filename.c_str());
+}
+
+TEST(ExceptionHandlerTest, InstructionPointerMemory) {
+  int fds[2];
+  ASSERT_NE(pipe(fds), -1);
+
+  // These are defined here so the parent can use them to check the
+  // data from the minidump afterwards.
+  const u_int32_t kMemorySize = 256;  // bytes
+  const int kOffset = kMemorySize / 2;
+  // This crashes with SIGILL on x86/x86-64/arm.
+  const unsigned char instructions[] = { 0xff, 0xff, 0xff, 0xff };
+
+  const pid_t child = fork();
+  if (child == 0) {
+    close(fds[0]);
+    ExceptionHandler handler("/tmp", NULL, DoneCallback, (void*) fds[1],
+                             true);
+    // Get some executable memory.
+    char* memory =
+      reinterpret_cast<char*>(mmap(NULL,
+                                   kMemorySize,
+                                   PROT_READ | PROT_WRITE | PROT_EXEC,
+                                   MAP_PRIVATE | MAP_ANON,
+                                   -1,
+                                   0));
+    if (!memory)
+      exit(0);
+
+    // Write some instructions that will crash. Put them in the middle
+    // of the block of memory, because the minidump should contain 128
+    // bytes on either side of the instruction pointer.
+    memcpy(memory + kOffset, instructions, sizeof(instructions));
+    
+    // Now execute the instructions, which should crash.
+    typedef void (*void_function)(void);
+    void_function memory_function =
+      reinterpret_cast<void_function>(memory + kOffset);
+    memory_function();
+  }
+  close(fds[1]);
+
+  int status;
+  ASSERT_NE(HANDLE_EINTR(waitpid(child, &status, 0)), -1);
+  ASSERT_TRUE(WIFSIGNALED(status));
+  ASSERT_EQ(WTERMSIG(status), SIGILL);
+
+  struct pollfd pfd;
+  memset(&pfd, 0, sizeof(pfd));
+  pfd.fd = fds[0];
+  pfd.events = POLLIN | POLLERR;
+
+  const int r = HANDLE_EINTR(poll(&pfd, 1, 0));
+  ASSERT_EQ(r, 1);
+  ASSERT_TRUE(pfd.revents & POLLIN);
+
+  uint32_t len;
+  ASSERT_EQ(read(fds[0], &len, sizeof(len)), (ssize_t)sizeof(len));
+  ASSERT_LT(len, (uint32_t)2048);
+  char* filename = reinterpret_cast<char*>(malloc(len + 1));
+  ASSERT_EQ(read(fds[0], filename, len), len);
+  filename[len] = 0;
+  close(fds[0]);
+
+  const std::string minidump_filename = std::string("/tmp/") + filename +
+                                        ".dmp";
+
+  struct stat st;
+  ASSERT_EQ(stat(minidump_filename.c_str(), &st), 0);
+  ASSERT_GT(st.st_size, 0u);
+
+  // Read the minidump. Locate the exception record and the
+  // memory list, and then ensure that there is a memory region
+  // in the memory list that covers the instruction pointer from
+  // the exception record.
+  Minidump minidump(minidump_filename);
+  ASSERT_TRUE(minidump.Read());
+
+  MinidumpException* exception = minidump.GetException();
+  MinidumpMemoryList* memory_list = minidump.GetMemoryList();
+  ASSERT_TRUE(exception);
+  ASSERT_TRUE(memory_list);
+  ASSERT_LT(0, memory_list->region_count());
+
+  MinidumpContext* context = exception->GetContext();
+  ASSERT_TRUE(context);
+
+  u_int64_t instruction_pointer;
+  switch (context->GetContextCPU()) {
+  case MD_CONTEXT_X86:
+    instruction_pointer = context->GetContextX86()->eip;
+    break;
+  case MD_CONTEXT_AMD64:
+    instruction_pointer = context->GetContextAMD64()->rip;
+    break;
+  case MD_CONTEXT_ARM:
+    instruction_pointer = context->GetContextARM()->iregs[15];
+    break;
+  default:
+    FAIL() << "Unknown context CPU: " << context->GetContextCPU();
+    break;
+  }
+
+  MinidumpMemoryRegion* region =
+    memory_list->GetMemoryRegionForAddress(instruction_pointer);
+  ASSERT_TRUE(region);
+
+  EXPECT_EQ(kMemorySize, region->GetSize());
+  const u_int8_t* bytes = region->GetMemory();
+  ASSERT_TRUE(bytes);
+
+  u_int8_t prefix_bytes[kOffset];
+  u_int8_t suffix_bytes[kMemorySize - kOffset - sizeof(instructions)];
+  memset(prefix_bytes, 0, sizeof(prefix_bytes));
+  memset(suffix_bytes, 0, sizeof(suffix_bytes));
+  EXPECT_TRUE(memcmp(bytes, prefix_bytes, sizeof(prefix_bytes)) == 0);
+  EXPECT_TRUE(memcmp(bytes + kOffset, instructions, sizeof(instructions)) == 0);
+  EXPECT_TRUE(memcmp(bytes + kOffset + sizeof(instructions),
+                     suffix_bytes, sizeof(suffix_bytes)) == 0);
+
+  unlink(minidump_filename.c_str());
   free(filename);
 }
 
diff --git a/src/client/linux/minidump_writer/minidump_writer.cc b/src/client/linux/minidump_writer/minidump_writer.cc
index 260ce934..6b55ade0 100644
--- a/src/client/linux/minidump_writer/minidump_writer.cc
+++ b/src/client/linux/minidump_writer/minidump_writer.cc
@@ -46,6 +46,8 @@
 #include "client/linux/minidump_writer/minidump_writer.h"
 #include "client/minidump_file_writer-inl.h"
 
+#include <algorithm>
+
 #include <errno.h>
 #include <fcntl.h>
 #include <link.h>
@@ -367,7 +369,8 @@ class MinidumpWriter {
         float_state_(NULL),
 #endif
         crashing_tid_(context->tid),
-        dumper_(crashing_pid) {
+        dumper_(crashing_pid),
+        memory_blocks_(dumper_.allocator()) {
   }
 
   bool Init() {
@@ -400,7 +403,9 @@ class MinidumpWriter {
 
     // A minidump file contains a number of tagged streams. This is the number
     // of stream which we write.
-    const unsigned kNumWriters = 11 + !!r_debug;
+    unsigned kNumWriters = 12;
+    if (r_debug)
+      ++kNumWriters;
 
     TypedMDRVA<MDRawHeader> header(&minidump_writer_);
     TypedMDRVA<MDRawDirectory> dir(&minidump_writer_);
@@ -427,6 +432,10 @@ class MinidumpWriter {
       return false;
     dir.CopyIndex(dir_index++, &dirent);
 
+    if (!WriteMemoryListStream(&dirent))
+      return false;
+    dir.CopyIndex(dir_index++, &dirent);
+
     if (!WriteExceptionStream(&dirent))
       return false;
     dir.CopyIndex(dir_index++, &dirent);
@@ -635,6 +644,50 @@ class MinidumpWriter {
         memory.Copy(stack_copy, stack_len);
         thread.stack.start_of_memory_range = (uintptr_t) (stack);
         thread.stack.memory = memory.location();
+        memory_blocks_.push_back(thread.stack);
+
+        // Copy 256 bytes around crashing instruction pointer to minidump.
+        const size_t kIPMemorySize = 256;
+        u_int64_t ip = GetInstructionPointer();
+        // Bound it to the upper and lower bounds of the memory map
+        // it's contained within. If it's not in mapped memory,
+        // don't bother trying to write it.
+        bool ip_is_mapped = false;
+        MDMemoryDescriptor ip_memory_d;
+        for (unsigned i = 0; i < dumper_.mappings().size(); ++i) {
+          const MappingInfo& mapping = *dumper_.mappings()[i];
+          if (ip >= mapping.start_addr &&
+              ip < mapping.start_addr + mapping.size) {
+            ip_is_mapped = true;
+            // Try to get 128 bytes before and after the IP, but
+            // settle for whatever's available.
+            ip_memory_d.start_of_memory_range =
+              std::min(mapping.start_addr,
+                       uintptr_t(ip - (kIPMemorySize / 2)));
+            ip_memory_d.memory.data_size =
+              std::min(ptrdiff_t(kIPMemorySize),
+                       ptrdiff_t(mapping.start_addr + mapping.size
+                                 - ip_memory_d.start_of_memory_range));
+            break;
+          }
+        }
+
+        if (ip_is_mapped) {
+          UntypedMDRVA ip_memory(&minidump_writer_);
+          if (!ip_memory.Allocate(ip_memory_d.memory.data_size))
+            return false;
+          uint8_t* memory_copy =
+            (uint8_t*) dumper_.allocator()->Alloc(ip_memory_d.memory.data_size);
+          dumper_.CopyFromProcess(
+            memory_copy,
+            thread.thread_id,
+            reinterpret_cast<void*>(ip_memory_d.start_of_memory_range),
+            ip_memory_d.memory.data_size);
+          ip_memory.Copy(memory_copy, ip_memory_d.memory.data_size);
+          ip_memory_d.memory = ip_memory.location();
+          memory_blocks_.push_back(ip_memory_d);
+        }
+
         TypedMDRVA<RawContextCPU> cpu(&minidump_writer_);
         if (!cpu.Allocate())
           return false;
@@ -657,6 +710,8 @@ class MinidumpWriter {
         memory.Copy(stack_copy, info.stack_len);
         thread.stack.start_of_memory_range = (uintptr_t)(info.stack);
         thread.stack.memory = memory.location();
+        memory_blocks_.push_back(thread.stack);
+
         TypedMDRVA<RawContextCPU> cpu(&minidump_writer_);
         if (!cpu.Allocate())
           return false;
@@ -757,6 +812,24 @@ class MinidumpWriter {
     return true;
   }
 
+  bool WriteMemoryListStream(MDRawDirectory* dirent) {
+    TypedMDRVA<uint32_t> list(&minidump_writer_);
+    if (!list.AllocateObjectAndArray(memory_blocks_.size(),
+                                     sizeof(MDMemoryDescriptor)))
+      return false;
+
+    dirent->stream_type = MD_MEMORY_LIST_STREAM;
+    dirent->location = list.location();
+
+    *list.get() = memory_blocks_.size();
+
+    for (size_t i = 0; i < memory_blocks_.size(); ++i) {
+      list.CopyIndexAfterObject(i, &memory_blocks_[i],
+                                sizeof(MDMemoryDescriptor));
+    }
+    return true;
+  }
+
   bool WriteExceptionStream(MDRawDirectory* dirent) {
     TypedMDRVA<MDRawExceptionStream> exc(&minidump_writer_);
     if (!exc.Allocate())
@@ -872,14 +945,26 @@ class MinidumpWriter {
   uintptr_t GetStackPointer() {
     return ucontext_->uc_mcontext.gregs[REG_ESP];
   }
+
+  uintptr_t GetInstructionPointer() {
+    return ucontext_->uc_mcontext.gregs[REG_EIP];
+  }
 #elif defined(__x86_64)
   uintptr_t GetStackPointer() {
     return ucontext_->uc_mcontext.gregs[REG_RSP];
   }
+
+  uintptr_t GetInstructionPointer() {
+    return ucontext_->uc_mcontext.gregs[REG_RIP];
+  }
 #elif defined(__ARM_EABI__)
   uintptr_t GetStackPointer() {
     return ucontext_->uc_mcontext.arm_sp;
   }
+
+  uintptr_t GetInstructionPointer() {
+    return ucontext_->uc_mcontext.arm_ip;
+  }
 #else
 #error "This code has not been ported to your platform yet."
 #endif
@@ -1129,6 +1214,10 @@ popline:
   LinuxDumper dumper_;
   MinidumpFileWriter minidump_writer_;
   MDLocationDescriptor crashing_thread_context_;
+  // Blocks of memory written to the dump. These are all currently
+  // written while writing the thread list stream, but saved here
+  // so a memory list stream can be written afterwards.
+  wasteful_vector<MDMemoryDescriptor> memory_blocks_;
 };
 
 bool WriteMinidump(const char* filename, pid_t crashing_process,