Add API to skip dump if crashing thread doesn't reference a given module

This CL makes it possible to skip a dump if the crashing thread doesn't
have any pointers to a given module. The concrete use case is WebView
where we would like to skip generating microdump output when webview
is unreferenced by the stack and thus cannot be responsible for the
crash in a way that would be debuggable.

The range of interesting addresses is chosen by examining the process
mappings to find the one that contains a pointer that is known to be in
the right shared object (i.e. an appropriately chosen function pointer)
passed from the client.

If the extracted stack does not contain a pointer in this range, then we
do not generate a microdump. If the stack extraction fails, we still
generate a microdump (without a stack).

BUG=664460

Change-Id: If19406a13168264f7751245fc39591bd6cdbf5df
Reviewed-on: https://chromium-review.googlesource.com/419476
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Primiano Tucci <primiano@chromium.org>

10 files changed, 149 insertions, 62 deletions

diff --git a/src/client/linux/dump_writer_common/mapping_info.h b/src/client/linux/dump_writer_common/mapping_info.h
index 5f247cfd..c09e48ab 100644
--- a/src/client/linux/dump_writer_common/mapping_info.h
+++ b/src/client/linux/dump_writer_common/mapping_info.h
@@ -41,8 +41,21 @@ namespace google_breakpad {
 // One of these is produced for each mapping in the process (i.e. line in
 // /proc/$x/maps).
 struct MappingInfo {
+  // On Android, relocation packing can mean that the reported start
+  // address of the mapping must be adjusted by a bias in order to
+  // compensate for the compression of the relocation section. The
+  // following two members hold (after LateInit) the adjusted mapping
+  // range. See crbug.com/606972 for more information.
   uintptr_t start_addr;
   size_t size;
+  // When Android relocation packing causes |start_addr| and |size| to
+  // be modified with a load bias, we need to remember the unbiased
+  // address range. The following structure holds the original mapping
+  // address range as reported by the operating system.
+  struct {
+    uintptr_t start_addr;
+    uintptr_t end_addr;
+  } system_mapping_info;
   size_t offset;  // offset into the backed file.
   bool exec;  // true if the mapping has the execute bit set.
   char name[NAME_MAX];
diff --git a/src/client/linux/handler/exception_handler.cc b/src/client/linux/handler/exception_handler.cc
index c8cb768a..8565bbb0 100644
--- a/src/client/linux/handler/exception_handler.cc
+++ b/src/client/linux/handler/exception_handler.cc
@@ -592,6 +592,8 @@ bool ExceptionHandler::DoDump(pid_t crashing_process, const void* context,
         context,
         context_size,
         mapping_list_,
+        minidump_descriptor_.skip_dump_if_principal_mapping_not_referenced(),
+        minidump_descriptor_.address_within_principal_mapping(),
         *minidump_descriptor_.microdump_extra_info());
   }
   if (minidump_descriptor_.IsFD()) {
diff --git a/src/client/linux/handler/microdump_extra_info.h b/src/client/linux/handler/microdump_extra_info.h
index 40cba1c4..bf01f0c7 100644
--- a/src/client/linux/handler/microdump_extra_info.h
+++ b/src/client/linux/handler/microdump_extra_info.h
@@ -40,23 +40,11 @@ struct MicrodumpExtraInfo {
   const char* gpu_fingerprint;
   const char* process_type;
 
-  // |interest_range_start| and |interest_range_end| specify a range
-  // in the target process address space. Microdumps are only
-  // generated if the PC or a word on the captured stack point into
-  // this range, or |suppress_microdump_based_on_interest_range| is
-  // false.
-  bool suppress_microdump_based_on_interest_range;
-  uintptr_t interest_range_start;
-  uintptr_t interest_range_end;
-
   MicrodumpExtraInfo()
       : build_fingerprint(NULL),
         product_info(NULL),
         gpu_fingerprint(NULL),
-        process_type(NULL),
-        suppress_microdump_based_on_interest_range(false),
-        interest_range_start(0),
-        interest_range_end(0) {}
+        process_type(NULL) {}
 };
 
 }
diff --git a/src/client/linux/handler/minidump_descriptor.h b/src/client/linux/handler/minidump_descriptor.h
index 782a60a4..f601427c 100644
--- a/src/client/linux/handler/minidump_descriptor.h
+++ b/src/client/linux/handler/minidump_descriptor.h
@@ -53,14 +53,18 @@ class MinidumpDescriptor {
   MinidumpDescriptor()
       : mode_(kUninitialized),
         fd_(-1),
-        size_limit_(-1) {}
+        size_limit_(-1),
+        address_within_principal_mapping_(0),
+        skip_dump_if_principal_mapping_not_referenced_(false) {}
 
   explicit MinidumpDescriptor(const string& directory)
       : mode_(kWriteMinidumpToFile),
         fd_(-1),
         directory_(directory),
         c_path_(NULL),
-        size_limit_(-1) {
+        size_limit_(-1),
+        address_within_principal_mapping_(0),
+        skip_dump_if_principal_mapping_not_referenced_(false) {
     assert(!directory.empty());
   }
 
@@ -68,14 +72,18 @@ class MinidumpDescriptor {
       : mode_(kWriteMinidumpToFd),
         fd_(fd),
         c_path_(NULL),
-        size_limit_(-1) {
+        size_limit_(-1),
+        address_within_principal_mapping_(0),
+        skip_dump_if_principal_mapping_not_referenced_(false) {
     assert(fd != -1);
   }
 
   explicit MinidumpDescriptor(const MicrodumpOnConsole&)
       : mode_(kWriteMicrodumpToConsole),
         fd_(-1),
-        size_limit_(-1) {}
+        size_limit_(-1),
+        address_within_principal_mapping_(0),
+        skip_dump_if_principal_mapping_not_referenced_(false) {}
 
   explicit MinidumpDescriptor(const MinidumpDescriptor& descriptor);
   MinidumpDescriptor& operator=(const MinidumpDescriptor& descriptor);
@@ -101,6 +109,23 @@ class MinidumpDescriptor {
   off_t size_limit() const { return size_limit_; }
   void set_size_limit(off_t limit) { size_limit_ = limit; }
 
+  uintptr_t address_within_principal_mapping() const {
+    return address_within_principal_mapping_;
+  }
+  void set_address_within_principal_mapping(
+      uintptr_t address_within_principal_mapping) {
+    address_within_principal_mapping_ = address_within_principal_mapping;
+  }
+
+  bool skip_dump_if_principal_mapping_not_referenced() {
+    return skip_dump_if_principal_mapping_not_referenced_;
+  }
+  void set_skip_dump_if_principal_mapping_not_referenced(
+      bool skip_dump_if_principal_mapping_not_referenced) {
+    skip_dump_if_principal_mapping_not_referenced_ =
+        skip_dump_if_principal_mapping_not_referenced;
+  }
+
   MicrodumpExtraInfo* microdump_extra_info() {
     assert(IsMicrodumpOnConsole());
     return &microdump_extra_info_;
@@ -132,6 +157,16 @@ class MinidumpDescriptor {
 
   off_t size_limit_;
 
+  // This member points somewhere into the main module for this
+  // process (the module that is considerered interesting for the
+  // purposes of debugging crashes).
+  uintptr_t address_within_principal_mapping_;
+
+  // If set, threads that do not reference the address range
+  // associated with |address_within_principal_mapping_| will not have their
+  // stacks logged.
+  bool skip_dump_if_principal_mapping_not_referenced_;
+
   // The extra microdump data (e.g. product name/version, build
   // fingerprint, gpu fingerprint) that should be appended to the dump
   // (microdump only). Microdumps don't have the ability of appending
diff --git a/src/client/linux/microdump_writer/microdump_writer.cc b/src/client/linux/microdump_writer/microdump_writer.cc
index 6ed69ea9..8109a981 100644
--- a/src/client/linux/microdump_writer/microdump_writer.cc
+++ b/src/client/linux/microdump_writer/microdump_writer.cc
@@ -132,6 +132,8 @@ class MicrodumpWriter {
  public:
   MicrodumpWriter(const ExceptionHandler::CrashContext* context,
                   const MappingList& mappings,
+                  bool skip_dump_if_principal_mapping_not_referenced,
+                  uintptr_t address_within_principal_mapping,
                   const MicrodumpExtraInfo& microdump_extra_info,
                   LinuxDumper* dumper)
       : ucontext_(context ? &context->context : NULL),
@@ -140,6 +142,9 @@ class MicrodumpWriter {
 #endif
         dumper_(dumper),
         mapping_list_(mappings),
+        skip_dump_if_principal_mapping_not_referenced_(
+            skip_dump_if_principal_mapping_not_referenced),
+        address_within_principal_mapping_(address_within_principal_mapping),
         microdump_extra_info_(microdump_extra_info),
         log_line_(NULL),
         stack_copy_(NULL),
@@ -252,37 +257,22 @@ class MicrodumpWriter {
                              reinterpret_cast<const void*>(stack_lower_bound_),
                              stack_len_);
 
-    if (!microdump_extra_info_.suppress_microdump_based_on_interest_range)
-      return CAPTURE_OK;
+    if (!skip_dump_if_principal_mapping_not_referenced_) return CAPTURE_OK;
 
-    uintptr_t low_addr = microdump_extra_info_.interest_range_start;
-    uintptr_t high_addr = microdump_extra_info_.interest_range_end;
+    const MappingInfo* principal_mapping =
+        dumper_->FindMappingNoBias(address_within_principal_mapping_);
+    if (!principal_mapping) return CAPTURE_UNINTERESTING;
 
+    uintptr_t low_addr = principal_mapping->start_addr;
+    uintptr_t high_addr = principal_mapping->start_addr + principal_mapping->size;
     uintptr_t pc = UContextReader::GetInstructionPointer(ucontext_);
     if (low_addr <= pc && pc <= high_addr) return CAPTURE_OK;
 
-    // Loop over all stack words that would have been on the stack in
-    // the target process. (i.e. ones that are >= |stack_pointer_| and
-    // < |stack_lower_bound_| + |stack_len_| in the target
-    // process).
-    // |stack_lower_bound_| is page aligned, and thus also pointer
-    // aligned. Because the stack pointer might be unaligned, we round
-    // the offset down to word alignment. |stack_pointer_| >
-    // |stack_lower_bound_|, so this never results in a negative
-    // offset.
-    // Regardless of the alignment of |stack_copy_|, the memory
-    // starting at |stack_copy_| + |offset| represents an aligned word
-    // in the target process.
-    uintptr_t offset =
-        ((stack_pointer_ - stack_lower_bound_) & ~(sizeof(uintptr_t) - 1));
-    for (uint8_t* sp = stack_copy_ + offset;
-         sp <= stack_copy_ + stack_len_ - sizeof(uintptr_t);
-         sp += sizeof(uintptr_t)) {
-      uintptr_t addr;
-      my_memcpy(&addr, sp, sizeof(uintptr_t));
-      if (low_addr <= addr && addr <= high_addr) return CAPTURE_OK;
+    if (dumper_->StackHasPointerToMapping(stack_copy_, stack_len_,
+                                          stack_pointer_ - stack_lower_bound_,
+                                          *principal_mapping)) {
+      return CAPTURE_OK;
     }
-
     return CAPTURE_UNINTERESTING;
   }
 
@@ -588,6 +578,8 @@ class MicrodumpWriter {
 #endif
   LinuxDumper* dumper_;
   const MappingList& mapping_list_;
+  bool skip_dump_if_principal_mapping_not_referenced_;
+  uintptr_t address_within_principal_mapping_;
   const MicrodumpExtraInfo microdump_extra_info_;
   char* log_line_;
 
@@ -613,6 +605,8 @@ bool WriteMicrodump(pid_t crashing_process,
                     const void* blob,
                     size_t blob_size,
                     const MappingList& mappings,
+                    bool skip_dump_if_principal_mapping_not_referenced,
+                    uintptr_t address_within_principal_mapping,
                     const MicrodumpExtraInfo& microdump_extra_info) {
   LinuxPtraceDumper dumper(crashing_process);
   const ExceptionHandler::CrashContext* context = NULL;
@@ -625,7 +619,9 @@ bool WriteMicrodump(pid_t crashing_process,
     dumper.set_crash_signal(context->siginfo.si_signo);
     dumper.set_crash_thread(context->tid);
   }
-  MicrodumpWriter writer(context, mappings, microdump_extra_info, &dumper);
+  MicrodumpWriter writer(
+      context, mappings, skip_dump_if_principal_mapping_not_referenced,
+      address_within_principal_mapping, microdump_extra_info, &dumper);
   if (!writer.Init())
     return false;
   writer.Dump();
diff --git a/src/client/linux/microdump_writer/microdump_writer.h b/src/client/linux/microdump_writer/microdump_writer.h
index 7c742761..78af86df 100644
--- a/src/client/linux/microdump_writer/microdump_writer.h
+++ b/src/client/linux/microdump_writer/microdump_writer.h
@@ -58,6 +58,8 @@ bool WriteMicrodump(pid_t crashing_process,
                     const void* blob,
                     size_t blob_size,
                     const MappingList& mappings,
+                    bool skip_dump_if_main_module_not_referenced,
+                    uintptr_t address_within_main_module,
                     const MicrodumpExtraInfo& microdump_extra_info);
 
 }  // namespace google_breakpad
diff --git a/src/client/linux/microdump_writer/microdump_writer_unittest.cc b/src/client/linux/microdump_writer/microdump_writer_unittest.cc
index 42677852..5d028bd3 100644
--- a/src/client/linux/microdump_writer/microdump_writer_unittest.cc
+++ b/src/client/linux/microdump_writer/microdump_writer_unittest.cc
@@ -60,18 +60,11 @@ typedef testing::Test MicrodumpWriterTest;
 MicrodumpExtraInfo MakeMicrodumpExtraInfo(
     const char* build_fingerprint,
     const char* product_info,
-    const char* gpu_fingerprint,
-    bool suppress_microdump_based_on_interest_range = false,
-    uintptr_t interest_range_start = 0,
-    uintptr_t interest_range_end = 0) {
+    const char* gpu_fingerprint) {
   MicrodumpExtraInfo info;
   info.build_fingerprint = build_fingerprint;
   info.product_info = product_info;
   info.gpu_fingerprint = gpu_fingerprint;
-  info.suppress_microdump_based_on_interest_range =
-      suppress_microdump_based_on_interest_range;
-  info.interest_range_start = interest_range_start;
-  info.interest_range_end = interest_range_end;
   return info;
 }
 
@@ -82,7 +75,9 @@ bool ContainsMicrodump(const std::string& buf) {
 
 void CrashAndGetMicrodump(const MappingList& mappings,
                           const MicrodumpExtraInfo& microdump_extra_info,
-                          std::string* microdump) {
+                          std::string* microdump,
+                          bool skip_dump_if_principal_mapping_not_referenced = false,
+                          uintptr_t address_within_principal_mapping = 0) {
   int fds[2];
   ASSERT_NE(-1, pipe(fds));
 
@@ -116,7 +111,8 @@ void CrashAndGetMicrodump(const MappingList& mappings,
   ASSERT_NE(-1, dup2(err_fd, STDERR_FILENO));
 
   ASSERT_TRUE(WriteMicrodump(child, &context, sizeof(context), mappings,
-                             microdump_extra_info));
+                             skip_dump_if_principal_mapping_not_referenced,
+                             address_within_principal_mapping, microdump_extra_info));
 
   // Revert stderr back to the console.
   dup2(save_err, STDERR_FILENO);
@@ -239,13 +235,12 @@ TEST(MicrodumpWriterTest, NoOutputIfUninteresting) {
   const char kGPUFingerprint[] =
       "Qualcomm;Adreno (TM) 330;OpenGL ES 3.0 V@104.0 AU@  (GIT@Id3510ff6dc)";
   const MicrodumpExtraInfo kMicrodumpExtraInfo(
-      MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint,
-                             true, 0xdeadbeef, 0xdeadbeef - 1));
+      MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint));
 
   std::string buf;
   MappingList no_mappings;
 
-  CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf);
+  CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf, true, 0);
   ASSERT_FALSE(ContainsMicrodump(buf));
 }
 
@@ -259,15 +254,13 @@ TEST(MicrodumpWriterTest, OutputIfInteresting) {
       "Qualcomm;Adreno (TM) 330;OpenGL ES 3.0 V@104.0 AU@  (GIT@Id3510ff6dc)";
 
   const MicrodumpExtraInfo kMicrodumpExtraInfo(
-      MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint,
-                             true,
-                             reinterpret_cast<uintptr_t>(&__executable_start),
-                             reinterpret_cast<uintptr_t>(&__etext)));
+      MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint));
 
   std::string buf;
   MappingList no_mappings;
 
-  CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf);
+  CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf, true,
+                       reinterpret_cast<uintptr_t>(CrashAndGetMicrodump));
   ASSERT_TRUE(ContainsMicrodump(buf));
 }
 
diff --git a/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc
index ae0c965b..f5b299f8 100644
--- a/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc
+++ b/src/client/linux/minidump_writer/linux_core_dumper_unittest.cc
@@ -41,7 +41,7 @@ using namespace google_breakpad;
 
 TEST(LinuxCoreDumperTest, GetMappingAbsolutePath) {
   const LinuxCoreDumper dumper(getpid(), "core", "/tmp", "/mnt/root");
-  const MappingInfo mapping = { 0, 0, 0, false, "/usr/lib/libc.so" };
+  const MappingInfo mapping = {0, 0, {0, 0}, 0, false, "/usr/lib/libc.so"};
 
   char path[PATH_MAX];
   dumper.GetMappingAbsolutePath(mapping, path);
diff --git a/src/client/linux/minidump_writer/linux_dumper.cc b/src/client/linux/minidump_writer/linux_dumper.cc
index a95ec7ec..4a15f0cc 100644
--- a/src/client/linux/minidump_writer/linux_dumper.cc
+++ b/src/client/linux/minidump_writer/linux_dumper.cc
@@ -530,6 +530,8 @@ bool LinuxDumper::EnumerateMappings() {
           }
           MappingInfo* const module = new(allocator_) MappingInfo;
           my_memset(module, 0, sizeof(MappingInfo));
+          module->system_mapping_info.start_addr = start_addr;
+          module->system_mapping_info.end_addr = end_addr;
           module->start_addr = start_addr;
           module->size = end_addr - start_addr;
           module->offset = offset;
@@ -703,6 +705,31 @@ bool LinuxDumper::GetStackInfo(const void** stack, size_t* stack_len,
   return true;
 }
 
+bool LinuxDumper::StackHasPointerToMapping(const uint8_t* stack_copy,
+                                           size_t stack_len,
+                                           uintptr_t sp_offset,
+                                           const MappingInfo& mapping) {
+  // Loop over all stack words that would have been on the stack in
+  // the target process (i.e. are word aligned, and at addresses >=
+  // the stack pointer).  Regardless of the alignment of |stack_copy|,
+  // the memory starting at |stack_copy| + |offset| represents an
+  // aligned word in the target process.
+  const uintptr_t low_addr = mapping.system_mapping_info.start_addr;
+  const uintptr_t high_addr = mapping.system_mapping_info.end_addr;
+  const uintptr_t offset =
+      (sp_offset + sizeof(uintptr_t) - 1) & ~(sizeof(uintptr_t) - 1);
+
+  for (const uint8_t* sp = stack_copy + offset;
+       sp <= stack_copy + stack_len - sizeof(uintptr_t);
+       sp += sizeof(uintptr_t)) {
+    uintptr_t addr;
+    my_memcpy(&addr, sp, sizeof(uintptr_t));
+    if (low_addr <= addr && addr <= high_addr)
+      return true;
+  }
+  return false;
+}
+
 // Find the mapping which the given memory address falls in.
 const MappingInfo* LinuxDumper::FindMapping(const void* address) const {
   const uintptr_t addr = (uintptr_t) address;
@@ -716,6 +743,19 @@ const MappingInfo* LinuxDumper::FindMapping(const void* address) const {
   return NULL;
 }
 
+// Find the mapping which the given memory address falls in. Uses the
+// unadjusted mapping address range from the kernel, rather than the
+// biased range.
+const MappingInfo* LinuxDumper::FindMappingNoBias(uintptr_t address) const {
+  for (size_t i = 0; i < mappings_.size(); ++i) {
+    if (address >= mappings_[i]->system_mapping_info.start_addr &&
+        address < mappings_[i]->system_mapping_info.end_addr) {
+      return mappings_[i];
+    }
+  }
+  return NULL;
+}
+
 bool LinuxDumper::HandleDeletedFileInMapping(char* path) const {
   static const size_t kDeletedSuffixLen = sizeof(kDeletedSuffix) - 1;
 
diff --git a/src/client/linux/minidump_writer/linux_dumper.h b/src/client/linux/minidump_writer/linux_dumper.h
index c3c79926..0e20209b 100644
--- a/src/client/linux/minidump_writer/linux_dumper.h
+++ b/src/client/linux/minidump_writer/linux_dumper.h
@@ -103,6 +103,11 @@ class LinuxDumper {
   const wasteful_vector<pid_t> &threads() { return threads_; }
   const wasteful_vector<MappingInfo*> &mappings() { return mappings_; }
   const MappingInfo* FindMapping(const void* address) const;
+  // Find the mapping which the given memory address falls in. Unlike
+  // FindMapping, this method uses the unadjusted mapping address
+  // ranges from the kernel, rather than the ranges that have had the
+  // load bias applied.
+  const MappingInfo* FindMappingNoBias(uintptr_t address) const;
   const wasteful_vector<elf_aux_val_t>& auxv() { return auxv_; }
 
   // Find a block of memory to take as the stack given the top of stack pointer.
@@ -111,6 +116,19 @@ class LinuxDumper {
   //   stack_top: the current top of the stack
   bool GetStackInfo(const void** stack, size_t* stack_len, uintptr_t stack_top);
 
+  // Test whether |stack_copy| contains a pointer-aligned word that
+  // could be an address within a given mapping.
+  //   stack_copy: a copy of the stack to check. |stack_copy| might
+  //               not be word aligned, but it represents word aligned
+  //               data copied from another location.
+  //   stack_len: the length of the allocation pointed to by |stack_copy|.
+  //   sp_offset: the offset relative to stack_copy that reflects the
+  //              current value of the stack pointer.
+  //   mapping: the mapping against which to test stack words.
+  bool StackHasPointerToMapping(const uint8_t* stack_copy, size_t stack_len,
+                                uintptr_t sp_offset,
+                                const MappingInfo& mapping);
+
   PageAllocator* allocator() { return &allocator_; }
 
   // Copy content of |length| bytes from a given process |child|,