Remove the API to pass known memory mappings to MinidumpWriter and ExceptionHandler

src/client/linux/minidump_writer/minidump_writer_unittest.cc

Index: src/client/linux/minidump_writer/minidump_writer_unittest.cc
diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest.cc b/src/client/linux/minidump_writer/minidump_writer_unittest.cc
index 5de2705b6cb4fba1e61c9dd5079eeba861193679..2ca7569b8936b717c643f0e90b51e015d556bc61 100644
--- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc
+++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc
@@ -177,218 +177,6 @@ TEST(MinidumpWriterTest, ModulesIncludeLinuxGate) {
 }
 #endif
 
-// Test that mapping info can be specified when writing a minidump,
-// and that it ends up in the module list of the minidump.
-TEST(MinidumpWriterTest, MappingInfo) {
-  int fds[2];
-  ASSERT_NE(-1, pipe(fds));
-
-  // These are defined here so the parent can use them to check the
-  // data from the minidump afterwards.
-  const uint32_t memory_size = sysconf(_SC_PAGESIZE);
-  const char* kMemoryName = "a fake module";
-  const uint8_t kModuleGUID[sizeof(MDGUID)] = {
-    0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
-    0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF
-  };
-  char module_identifier_buffer[kGUIDStringSize];
-  FileID::ConvertIdentifierToString(kModuleGUID,
-                                    module_identifier_buffer,
-                                    sizeof(module_identifier_buffer));
-  string module_identifier(module_identifier_buffer);
-  // Strip out dashes
-  size_t pos;
-  while ((pos = module_identifier.find('-')) != string::npos) {
-    module_identifier.erase(pos, 1);
-  }
-  // And append a zero, because module IDs include an "age" field
-  // which is always zero on Linux.
-  module_identifier += "0";
-
-  // Get some memory.
-  char* memory =
-    reinterpret_cast<char*>(mmap(NULL,
-                                 memory_size,
-                                 PROT_READ | PROT_WRITE,
-                                 MAP_PRIVATE | MAP_ANON,
-                                 -1,
-                                 0));
-  const uintptr_t kMemoryAddress = reinterpret_cast<uintptr_t>(memory);
-  ASSERT_TRUE(memory);
-
-  const pid_t child = fork();
-  if (child == 0) {
-    close(fds[1]);
-    char b;
-    IGNORE_RET(HANDLE_EINTR(read(fds[0], &b, sizeof(b))));
-    close(fds[0]);
-    syscall(__NR_exit);
-  }
-  close(fds[0]);
-
-  ExceptionHandler::CrashContext context;
-  memset(&context, 0, sizeof(context));
-  ASSERT_EQ(0, getcontext(&context.context));
-  context.tid = child;
-
-  AutoTempDir temp_dir;
-  string templ = temp_dir.path() + kMDWriterUnitTestFileName;
-
-  // Add information about the mapped memory.
-  MappingInfo info;
-  info.start_addr = kMemoryAddress;
-  info.size = memory_size;
-  info.offset = 0;
-  strcpy(info.name, kMemoryName);
-
-  MappingList mappings;
-  AppMemoryList memory_list;
-  MappingEntry mapping;
-  mapping.first = info;
-  memcpy(mapping.second, kModuleGUID, sizeof(MDGUID));
-  mappings.push_back(mapping);
-  ASSERT_TRUE(WriteMinidump(templ.c_str(), child, &context, sizeof(context),
-                            mappings, memory_list));
-
-  // Read the minidump. Load the module list, and ensure that
-  // the mmap'ed |memory| is listed with the given module name
-  // and debug ID.
-  Minidump minidump(templ);
-  ASSERT_TRUE(minidump.Read());
-
-  MinidumpModuleList* module_list = minidump.GetModuleList();
-  ASSERT_TRUE(module_list);
-  const MinidumpModule* module =
-    module_list->GetModuleForAddress(kMemoryAddress);
-  ASSERT_TRUE(module);
-
-  EXPECT_EQ(kMemoryAddress, module->base_address());
-  EXPECT_EQ(memory_size, module->size());
-  EXPECT_EQ(kMemoryName, module->code_file());
-  EXPECT_EQ(module_identifier, module->debug_identifier());
-
-  uint32_t len;
-  // These streams are expected to be there
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_THREAD_LIST_STREAM, &len));
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_MEMORY_LIST_STREAM, &len));
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_EXCEPTION_STREAM, &len));
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_SYSTEM_INFO_STREAM, &len));
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_CPU_INFO, &len));
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_PROC_STATUS, &len));
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_CMD_LINE, &len));
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_ENVIRON, &len));
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_AUXV, &len));
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_MAPS, &len));
-  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_DSO_DEBUG, &len));
-
-  close(fds[1]);
-}
-
-// Test that mapping info can be specified, and that it overrides
-// existing mappings that are wholly contained within the specified
-// range.
-TEST(MinidumpWriterTest, MappingInfoContained) {
-  int fds[2];
-  ASSERT_NE(-1, pipe(fds));
-
-  // These are defined here so the parent can use them to check the
-  // data from the minidump afterwards.
-  const int32_t memory_size = sysconf(_SC_PAGESIZE);
-  const char* kMemoryName = "a fake module";
-  const uint8_t kModuleGUID[sizeof(MDGUID)] = {
-    0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77,
-    0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF
-  };
-  char module_identifier_buffer[kGUIDStringSize];
-  FileID::ConvertIdentifierToString(kModuleGUID,
-                                    module_identifier_buffer,
-                                    sizeof(module_identifier_buffer));
-  string module_identifier(module_identifier_buffer);
-  // Strip out dashes
-  size_t pos;
-  while ((pos = module_identifier.find('-')) != string::npos) {
-    module_identifier.erase(pos, 1);
-  }
-  // And append a zero, because module IDs include an "age" field
-  // which is always zero on Linux.
-  module_identifier += "0";
-
-  // mmap a file
-  AutoTempDir temp_dir;
-  string tempfile = temp_dir.path() + "/minidump-writer-unittest-temp";
-  int fd = open(tempfile.c_str(), O_RDWR | O_CREAT, 0);
-  ASSERT_NE(-1, fd);
-  unlink(tempfile.c_str());
-  // fill with zeros
-  google_breakpad::scoped_array<char> buffer(new char[memory_size]);
-  memset(buffer.get(), 0, memory_size);
-  ASSERT_EQ(memory_size, write(fd, buffer.get(), memory_size));
-  lseek(fd, 0, SEEK_SET);
-
-  char* memory =
-    reinterpret_cast<char*>(mmap(NULL,
-                                 memory_size,
-                                 PROT_READ | PROT_WRITE,
-                                 MAP_PRIVATE,
-                                 fd,
-                                 0));
-  const uintptr_t kMemoryAddress = reinterpret_cast<uintptr_t>(memory);
-  ASSERT_TRUE(memory);
-  close(fd);
-
-  const pid_t child = fork();
-  if (child == 0) {
-    close(fds[1]);
-    char b;
-    IGNORE_RET(HANDLE_EINTR(read(fds[0], &b, sizeof(b))));
-    close(fds[0]);
-    syscall(__NR_exit);
-  }
-  close(fds[0]);
-
-  ExceptionHandler::CrashContext context;
-  memset(&context, 0, sizeof(context));
-  context.tid = 1;
-
-  string dumpfile = temp_dir.path() + kMDWriterUnitTestFileName;
-
-  // Add information about the mapped memory. Report it as being larger than
-  // it actually is.
-  MappingInfo info;
-  info.start_addr = kMemoryAddress - memory_size;
-  info.size = memory_size * 3;
-  info.offset = 0;
-  strcpy(info.name, kMemoryName);
-
-  MappingList mappings;
-  AppMemoryList memory_list;
-  MappingEntry mapping;
-  mapping.first = info;
-  memcpy(mapping.second, kModuleGUID, sizeof(MDGUID));
-  mappings.push_back(mapping);
-  ASSERT_TRUE(WriteMinidump(dumpfile.c_str(), child, &context, sizeof(context),
-                            mappings, memory_list));
-
-  // Read the minidump. Load the module list, and ensure that
-  // the mmap'ed |memory| is listed with the given module name
-  // and debug ID.
-  Minidump minidump(dumpfile);
-  ASSERT_TRUE(minidump.Read());
-
-  MinidumpModuleList* module_list = minidump.GetModuleList();
-  ASSERT_TRUE(module_list);
-  const MinidumpModule* module =
-    module_list->GetModuleForAddress(kMemoryAddress);
-  ASSERT_TRUE(module);
-
-  EXPECT_EQ(info.start_addr, module->base_address());
-  EXPECT_EQ(info.size, module->size());
-  EXPECT_EQ(kMemoryName, module->code_file());
-  EXPECT_EQ(module_identifier, module->debug_identifier());
-
-  close(fds[1]);
-}
-
 TEST(MinidumpWriterTest, DeletedBinary) {
   const string kNumberOfThreadsArgument = "1";
   const string helper_path(GetHelperBinary());
@@ -523,7 +311,6 @@ TEST(MinidumpWriterTest, AdditionalMemory) {
   string templ = temp_dir.path() + kMDWriterUnitTestFileName;
   unlink(templ.c_str());
 
-  MappingList mappings;
   AppMemoryList memory_list;
 
   // Add the memory region to the list of memory to be included.
@@ -532,12 +319,26 @@ TEST(MinidumpWriterTest, AdditionalMemory) {
   app_memory.length = kMemorySize;
   memory_list.push_back(app_memory);
   ASSERT_TRUE(WriteMinidump(templ.c_str(), child, &context, sizeof(context),
-                            mappings, memory_list));
+                            memory_list));
 
   // Read the minidump. Ensure that the memory region is present
   Minidump minidump(templ);
   ASSERT_TRUE(minidump.Read());
 
+  uint32_t len;
+  // These streams are expected to be there
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_THREAD_LIST_STREAM, &len));
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_MEMORY_LIST_STREAM, &len));
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_EXCEPTION_STREAM, &len));
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_SYSTEM_INFO_STREAM, &len));
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_CPU_INFO, &len));
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_PROC_STATUS, &len));
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_CMD_LINE, &len));
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_ENVIRON, &len));
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_AUXV, &len));
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_MAPS, &len));
+  EXPECT_TRUE(minidump.SeekToStreamType(MD_LINUX_DSO_DEBUG, &len));
+

[Ted Mielczarek, 2013/05/17 19:54:46]

This hunk seems unrelated?

[glandium, 2013/05/17 20:14:48]

It's actually not. These streams test had been added to TEST(MinidumpWriterTest,
MappingInfo), I guess to avoid the boilerplate of adding a new test. This moves
these tests to some place else.

   MinidumpMemoryList* dump_memory_list = minidump.GetMemoryList();
   ASSERT_TRUE(dump_memory_list);
   const MinidumpMemoryRegion* region =
@@ -699,8 +500,7 @@ TEST(MinidumpWriterTest, MinidumpSizeLimit) {
     string normal_dump = temp_dir.path() +
         "/minidump-writer-unittest.dmp";
     ASSERT_TRUE(WriteMinidump(normal_dump.c_str(), -1,
-                              child_pid, NULL, 0,
-                              MappingList(), AppMemoryList()));
+                              child_pid, NULL, 0, AppMemoryList()));
     struct stat st;
     ASSERT_EQ(0, stat(normal_dump.c_str(), &st));
     ASSERT_GT(st.st_size, 0);
@@ -730,8 +530,7 @@ TEST(MinidumpWriterTest, MinidumpSizeLimit) {
     string same_dump = temp_dir.path() +
         "/minidump-writer-unittest-same.dmp";
     ASSERT_TRUE(WriteMinidump(same_dump.c_str(), minidump_size_limit,
-                              child_pid, NULL, 0,
-                              MappingList(), AppMemoryList()));
+                              child_pid, NULL, 0, AppMemoryList()));
     struct stat st;
     ASSERT_EQ(0, stat(same_dump.c_str(), &st));
     // Make sure limiting wasn't actually triggered.  NOTE: If you fail this,
@@ -763,8 +562,7 @@ TEST(MinidumpWriterTest, MinidumpSizeLimit) {
     string limit_dump = temp_dir.path() +
         "/minidump-writer-unittest-limit.dmp";
     ASSERT_TRUE(WriteMinidump(limit_dump.c_str(), minidump_size_limit,
-                              child_pid, NULL, 0,
-                              MappingList(), AppMemoryList()));
+                              child_pid, NULL, 0, AppMemoryList()));
     struct stat st;
     ASSERT_EQ(0, stat(limit_dump.c_str(), &st));
     ASSERT_GT(st.st_size, 0);