Don't generate minidump if crash thread doesn't ref principal mapping.

If the crashing thread doesn't reference the principal mapping we can
assume that not only is that thread uninteresting from a debugging
perspective, the whole crash is uninteresting. In that case we should
not generate a minidump at all.

BUG=703599

Change-Id: Ia25bbb8adb79d04dcaf3992c3d2474f3b9b1f796
Reviewed-on: https://chromium-review.googlesource.com/457338
Reviewed-by: Robert Sesek <rsesek@chromium.org>

1 files changed, 56 insertions, 6 deletions

diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest.cc b/src/client/linux/minidump_writer/minidump_writer_unittest.cc
index f3b78a22..8d7f232c 100644
--- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc
+++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc
@@ -215,8 +215,9 @@ TEST(MinidumpWriterTest, MappingInfo) {
   close(fds[1]);
 }
 
-// Test that stacks can be skipped while writing minidumps.
-TEST(MinidumpWriterTest, StacksSkippedIfRequested) {
+// Test that minidumping is skipped while writing minidumps if principal mapping
+// is not referenced.
+TEST(MinidumpWriterTest, MinidumpSkippedIfRequested) {
   int fds[2];
   ASSERT_NE(-1, pipe(fds));
 
@@ -239,20 +240,69 @@ TEST(MinidumpWriterTest, StacksSkippedIfRequested) {
   string templ = temp_dir.path() + kMDWriterUnitTestFileName;
 
   // pass an invalid principal mapping address, which will force
-  // WriteMinidump to not dump any thread stacks.
-  ASSERT_TRUE(WriteMinidump(templ.c_str(), child, &context, sizeof(context),
+  // WriteMinidump to not write a minidump.
+  ASSERT_FALSE(WriteMinidump(templ.c_str(), child, &context, sizeof(context),
                             true, static_cast<uintptr_t>(0x0102030405060708ull),
                             false));
+  close(fds[1]);
+}
+
+// Test that minidumping is skipped while writing minidumps if principal mapping
+// is not referenced.
+TEST(MinidumpWriterTest, MinidumpStacksSkippedIfRequested) {
+  int fds[2];
+  ASSERT_NE(-1, pipe(fds));
+
+  const pid_t child = fork();
+  if (child == 0) {
+    close(fds[1]);
+
+    // Create a thread that does not return, and only references libc (not the
+    // current executable). This thread should not be captured in the minidump.
+    pthread_t thread;
+    pthread_attr_t thread_attributes;
+    pthread_attr_init(&thread_attributes);
+    pthread_attr_setdetachstate(&thread_attributes, PTHREAD_CREATE_DETACHED);
+    sigset_t sigset;
+    sigemptyset(&sigset);
+    pthread_create(&thread, &thread_attributes,
+                   reinterpret_cast<void* (*)(void*)>(&sigsuspend), &sigset);
+
+    char b;
+    IGNORE_RET(HANDLE_EINTR(read(fds[0], &b, sizeof(b))));
+    close(fds[0]);
+    syscall(__NR_exit);
+  }
+  close(fds[0]);
 
-  // Read the minidump. And ensure that no thread memory was dumped.
+  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;
+
+  // Pass an invalid principal mapping address, which will force
+  // WriteMinidump to not dump any thread stacks.
+  ASSERT_TRUE(WriteMinidump(
+      templ.c_str(), child, &context, sizeof(context), true,
+      reinterpret_cast<uintptr_t>(google_breakpad::WriteFile), false));
+
+  // Read the minidump. And ensure that thread memory was dumped only for the
+  // main thread.
   Minidump minidump(templ);
   ASSERT_TRUE(minidump.Read());
 
   MinidumpThreadList *threads = minidump.GetThreadList();
+  int threads_with_stacks = 0;
   for (unsigned int i = 0; i < threads->thread_count(); ++i) {
     MinidumpThread *thread = threads->GetThreadAtIndex(i);
-    ASSERT_TRUE(thread->GetMemory() == nullptr);
+    if (thread->GetMemory()) {
+      ++threads_with_stacks;
+    }
   }
+  ASSERT_EQUAL(1, threads_with_stacks);
   close(fds[1]);
 }