Do not generate a microdump if there are no webview pointers on the stack.

The stack interest range is passed in MicrodumpExtraInfo from the client.
If the extracted stack does not contain a pointer in this range, then we
assume that this is not a WebView crash, and do not generate a microdump.
If the stack extraction fails, we still generate a microdump (without a
stack).

BUG=664460

Change-Id: Ic762497f76f074a3621c7ec88a8c20ed768b9211
Reviewed-on: https://chromium-review.googlesource.com/412781
Reviewed-by: Primiano Tucci <primiano@chromium.org>

src/client/linux/handler/microdump_extra_info.h

@@ -40,11 +40,23 @@ 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) {}
+        process_type(NULL),
+        suppress_microdump_based_on_interest_range(false),
+        interest_range_start(0),
+        interest_range_end(0) {}
 };
 
 }

src/client/linux/microdump_writer/microdump_writer.cc

@@ -141,7 +141,11 @@ class MicrodumpWriter {
         dumper_(dumper),
         mapping_list_(mappings),
         microdump_extra_info_(microdump_extra_info),
-        log_line_(NULL) {
+        log_line_(NULL),
+        stack_copy_(NULL),
+        stack_len_(0),
+        stack_lower_bound_(0),
+        stack_pointer_(0) {
     log_line_ = reinterpret_cast<char*>(Alloc(kLineBufferSize));
     if (log_line_)
       log_line_[0] = '\0';  // Clear out the log line buffer.
@@ -159,8 +163,12 @@ class MicrodumpWriter {
     return dumper_->ThreadsSuspend() && dumper_->LateInit();
   }
 
-  bool Dump() {
+  void Dump() {
-    bool success;
+    CaptureResult stack_capture_result = CaptureCrashingThreadStack(-1);
+    if (stack_capture_result == CAPTURE_UNINTERESTING) {
+      return;
+    }
+
     LogLine("-----BEGIN BREAKPAD MICRODUMP-----");
     DumpProductInformation();
     DumpOSInformation();
@@ -169,15 +177,16 @@ class MicrodumpWriter {
 #if !defined(__LP64__)
     DumpFreeSpace();
 #endif
-    success = DumpCrashingThread();
+    if (stack_capture_result == CAPTURE_OK)
-    if (success)
+      DumpThreadStack();
-      success = DumpMappings();
+    DumpCPUState();
+    DumpMappings();
     LogLine("-----END BREAKPAD MICRODUMP-----");
-    dumper_->ThreadsResume();
-    return success;
   }
 
  private:
+  enum CaptureResult { CAPTURE_OK, CAPTURE_FAILED, CAPTURE_UNINTERESTING };
+
   // Writes one line to the system log.
   void LogLine(const char* msg) {
 #if defined(__ANDROID__)
@@ -221,7 +230,59 @@ class MicrodumpWriter {
   // Writes out the current line buffer on the system log.
   void LogCommitLine() {
     LogLine(log_line_);
-    my_strlcpy(log_line_, "", kLineBufferSize);
+    log_line_[0] = 0;
+  }
+
+  CaptureResult CaptureCrashingThreadStack(int max_stack_len) {
+    stack_pointer_ = UContextReader::GetStackPointer(ucontext_);
+
+    if (!dumper_->GetStackInfo(reinterpret_cast<const void**>(&stack_lower_bound_),
+                               &stack_len_, stack_pointer_)) {
+      return CAPTURE_FAILED;
+    }
+
+    if (max_stack_len >= 0 &&
+        stack_len_ > static_cast<size_t>(max_stack_len)) {
+      stack_len_ = max_stack_len;
+    }
+
+    stack_copy_ = reinterpret_cast<uint8_t*>(Alloc(stack_len_));
+    dumper_->CopyFromProcess(stack_copy_, dumper_->crash_thread(),
+                             reinterpret_cast<const void*>(stack_lower_bound_),
+                             stack_len_);
+
+    if (!microdump_extra_info_.suppress_microdump_based_on_interest_range)
+      return CAPTURE_OK;
+
+    uintptr_t low_addr = microdump_extra_info_.interest_range_start;
+    uintptr_t high_addr = microdump_extra_info_.interest_range_end;
+
+    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;
+    }
+
+    return CAPTURE_UNINTERESTING;
   }
 
   void DumpProductInformation() {
@@ -315,72 +376,28 @@ class MicrodumpWriter {
     LogCommitLine();
   }
 
-  bool DumpThreadStack(uint32_t thread_id,
+  void DumpThreadStack() {
-                       uintptr_t stack_pointer,
-                       int max_stack_len,
-                       uint8_t** stack_copy) {
-    *stack_copy = NULL;
-    const void* stack;
-    size_t stack_len;
-
-    if (!dumper_->GetStackInfo(&stack, &stack_len, stack_pointer)) {
-      // The stack pointer might not be available. In this case we don't hard
-      // fail, just produce a (almost useless) microdump w/o a stack section.
-      return true;
-    }
-
     LogAppend("S 0 ");
-    LogAppend(stack_pointer);
+    LogAppend(stack_pointer_);
     LogAppend(" ");
-    LogAppend(reinterpret_cast<uintptr_t>(stack));
+    LogAppend(stack_lower_bound_);
     LogAppend(" ");
-    LogAppend(stack_len);
+    LogAppend(stack_len_);
     LogCommitLine();
 
-    if (max_stack_len >= 0 &&
-        stack_len > static_cast<unsigned int>(max_stack_len)) {
-      stack_len = max_stack_len;
-    }
-
-    *stack_copy = reinterpret_cast<uint8_t*>(Alloc(stack_len));
-    dumper_->CopyFromProcess(*stack_copy, thread_id, stack, stack_len);
-
-    // Dump the content of the stack, splicing it into chunks which size is
-    // compatible with the max logcat line size (see LOGGER_ENTRY_MAX_PAYLOAD).
     const size_t STACK_DUMP_CHUNK_SIZE = 384;
-    for (size_t stack_off = 0; stack_off < stack_len;
+    for (size_t stack_off = 0; stack_off < stack_len_;
          stack_off += STACK_DUMP_CHUNK_SIZE) {
       LogAppend("S ");
-      LogAppend(reinterpret_cast<uintptr_t>(stack) + stack_off);
+      LogAppend(stack_lower_bound_ + stack_off);
       LogAppend(" ");
-      LogAppend(*stack_copy + stack_off,
+      LogAppend(stack_copy_ + stack_off,
-                std::min(STACK_DUMP_CHUNK_SIZE, stack_len - stack_off));
+                std::min(STACK_DUMP_CHUNK_SIZE, stack_len_ - stack_off));
       LogCommitLine();
     }
-    return true;
   }
 
-  // Write information about the crashing thread.
+  void DumpCPUState() {
-  bool DumpCrashingThread() {
-    const unsigned num_threads = dumper_->threads().size();
-
-    for (unsigned i = 0; i < num_threads; ++i) {
-      MDRawThread thread;
-      my_memset(&thread, 0, sizeof(thread));
-      thread.thread_id = dumper_->threads()[i];
-
-      // Dump only the crashing thread.
-      if (static_cast<pid_t>(thread.thread_id) != dumper_->crash_thread())
-        continue;
-
-      assert(ucontext_);
-      assert(!dumper_->IsPostMortem());
-
-      uint8_t* stack_copy;
-      const uintptr_t stack_ptr = UContextReader::GetStackPointer(ucontext_);
-      if (!DumpThreadStack(thread.thread_id, stack_ptr, -1, &stack_copy))
-        return false;
-
     RawContextCPU cpu;
     my_memset(&cpu, 0, sizeof(RawContextCPU));
 #if !defined(__ARM_EABI__) && !defined(__mips__)
@@ -388,14 +405,8 @@ class MicrodumpWriter {
 #else
     UContextReader::FillCPUContext(&cpu, ucontext_);
 #endif
-      DumpCPUState(&cpu);
-    }
-    return true;
-  }
-
-  void DumpCPUState(RawContextCPU* cpu) {
     LogAppend("C ");
-    LogAppend(cpu, sizeof(*cpu));
+    LogAppend(&cpu, sizeof(cpu));
     LogCommitLine();
   }
 
@@ -547,7 +558,7 @@ class MicrodumpWriter {
 #endif
 
   // Write information about the mappings in effect.
-  bool DumpMappings() {
+  void DumpMappings() {
     // First write all the mappings from the dumper
     for (unsigned i = 0; i < dumper_->mappings().size(); ++i) {
       const MappingInfo& mapping = *dumper_->mappings()[i];
@@ -566,7 +577,6 @@ class MicrodumpWriter {
          ++iter) {
       DumpModule(iter->first, false, 0, iter->second);
     }
-    return true;
   }
 
   void* Alloc(unsigned bytes) { return dumper_->allocator()->Alloc(bytes); }
@@ -579,6 +589,20 @@ class MicrodumpWriter {
   const MappingList& mapping_list_;
   const MicrodumpExtraInfo microdump_extra_info_;
   char* log_line_;
+
+  // The local copy of crashed process stack memory, beginning at
+  // |stack_lower_bound_|.
+  uint8_t* stack_copy_;
+
+  // The length of crashed process stack copy.
+  size_t stack_len_;
+
+  // The address of the page containing the stack pointer in the
+  // crashed process. |stack_lower_bound_| <= |stack_pointer_|
+  uintptr_t stack_lower_bound_;
+
+  // The stack pointer in the crashed process.
+  uintptr_t stack_pointer_;
 };
 }  // namespace
 
@@ -603,7 +627,8 @@ bool WriteMicrodump(pid_t crashing_process,
   MicrodumpWriter writer(context, mappings, microdump_extra_info, &dumper);
   if (!writer.Init())
     return false;
-  return writer.Dump();
+  writer.Dump();
+  return true;
 }
 
 }  // namespace google_breakpad

src/client/linux/microdump_writer/microdump_writer_unittest.cc

@@ -31,6 +31,7 @@
 #include <sys/syscall.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <ucontext.h>
 
 #include <sstream>
 #include <string>
@@ -47,6 +48,11 @@
 
 using namespace google_breakpad;
 
+extern "C" {
+extern char __executable_start;
+extern char __etext;
+}
+
 namespace {
 
 typedef testing::Test MicrodumpWriterTest;
@@ -54,18 +60,29 @@ typedef testing::Test MicrodumpWriterTest;
 MicrodumpExtraInfo MakeMicrodumpExtraInfo(
     const char* build_fingerprint,
     const char* product_info,
-    const char* gpu_fingerprint) {
+    const char* gpu_fingerprint,
+    bool suppress_microdump_based_on_interest_range = false,
+    uintptr_t interest_range_start = 0,
+    uintptr_t interest_range_end = 0) {
   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;
 }
 
-void CrashAndGetMicrodump(
+void AssertContainsMicrodump(const std::string& buf) {
-    const MappingList& mappings,
+  ASSERT_NE(std::string::npos, buf.find("-----BEGIN BREAKPAD MICRODUMP-----"));
+  ASSERT_NE(std::string::npos, buf.find("-----END BREAKPAD MICRODUMP-----"));
+}
+
+void CrashAndGetMicrodump(const MappingList& mappings,
                           const MicrodumpExtraInfo& microdump_extra_info,
-    scoped_array<char>* buf) {
+                          std::string* microdump) {
   int fds[2];
   ASSERT_NE(-1, pipe(fds));
 
@@ -86,7 +103,10 @@ void CrashAndGetMicrodump(
 
   ExceptionHandler::CrashContext context;
   memset(&context, 0, sizeof(context));
-
+  // Pretend the current context is the child context (which is
+  // approximately right) so that we have a valid stack pointer, and
+  // can fetch child stack data via ptrace.
+  getcontext(&context.context);
   // Set a non-zero tid to avoid tripping asserts.
   context.tid = child;
 
@@ -105,17 +125,17 @@ void CrashAndGetMicrodump(
   // Read back the stderr file and check for the microdump marker.
   fsync(err_fd);
   lseek(err_fd, 0, SEEK_SET);
-  const size_t kBufSize = 64 * 1024;
-  buf->reset(new char[kBufSize]);
-  ASSERT_GT(read(err_fd, buf->get(), kBufSize), 0);
 
+  microdump->clear();
+  char buf[1024];
+
+  while (true) {
+    int bytes_read = IGNORE_EINTR(read(err_fd, buf, 1024));
+    if (bytes_read <= 0) break;
+    microdump->append(buf, buf + bytes_read);
+  }
   close(err_fd);
   close(fds[1]);
-
-  ASSERT_NE(static_cast<char*>(0), strstr(
-      buf->get(), "-----BEGIN BREAKPAD MICRODUMP-----"));
-  ASSERT_NE(static_cast<char*>(0), strstr(
-      buf->get(), "-----END BREAKPAD MICRODUMP-----"));
 }
 
 void CheckMicrodumpContents(const string& microdump_content,
@@ -191,23 +211,64 @@ TEST(MicrodumpWriterTest, BasicWithMappings) {
   memcpy(mapping.second, kModuleGUID, sizeof(MDGUID));
   mappings.push_back(mapping);
 
-  scoped_array<char> buf;
+  std::string buf;
   CrashAndGetMicrodump(mappings, MicrodumpExtraInfo(), &buf);
+  AssertContainsMicrodump(buf);
 
 #ifdef __LP64__
-  ASSERT_NE(static_cast<char*>(0), strstr(
+  ASSERT_NE(std::string::npos,
-      buf.get(), "M 0000000000001000 000000000000002A 0000000000001000 "
+            buf.find("M 0000000000001000 000000000000002A 0000000000001000 "
                      "33221100554477668899AABBCCDDEEFF0 libfoo.so"));
 #else
-  ASSERT_NE(static_cast<char*>(0), strstr(
+  ASSERT_NE(std::string::npos,
-      buf.get(), "M 00001000 0000002A 00001000 "
+            buf.find("M 00001000 0000002A 00001000 "
                      "33221100554477668899AABBCCDDEEFF0 libfoo.so"));
 #endif
 
   // In absence of a product info in the minidump, the writer should just write
   // an unknown marker.
-  ASSERT_NE(static_cast<char*>(0), strstr(
+  ASSERT_NE(std::string::npos, buf.find("V UNKNOWN:0.0.0.0"));
-      buf.get(), "V UNKNOWN:0.0.0.0"));
+}
+
+// Ensure that no output occurs if the interest region is set, but
+// doesn't overlap anything on the stack.
+TEST(MicrodumpWriterTest, NoOutputIfUninteresting) {
+  const char kProductInfo[] = "MockProduct:42.0.2311.99";
+  const char kBuildFingerprint[] =
+      "aosp/occam/mako:5.1.1/LMY47W/12345678:userdegbug/dev-keys";
+  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));
+
+  std::string buf;
+  MappingList no_mappings;
+
+  CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf);
+  ASSERT_EQ(0, buf.size());
+}
+
+// Ensure that output occurs if the interest region is set, and
+// does overlap something on the stack.
+TEST(MicrodumpWriterTest, OutputIfInteresting) {
+  const char kProductInfo[] = "MockProduct:42.0.2311.99";
+  const char kBuildFingerprint[] =
+      "aosp/occam/mako:5.1.1/LMY47W/12345678:userdegbug/dev-keys";
+  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,
+                             reinterpret_cast<uintptr_t>(&__executable_start),
+                             reinterpret_cast<uintptr_t>(&__etext)));
+
+  std::string buf;
+  MappingList no_mappings;
+
+  CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf);
+  ASSERT_LT(0, buf.size());
 }
 
 // Ensure that the product info and build fingerprint metadata show up in the
@@ -220,38 +281,40 @@ TEST(MicrodumpWriterTest, BuildFingerprintAndProductInfo) {
       "Qualcomm;Adreno (TM) 330;OpenGL ES 3.0 V@104.0 AU@  (GIT@Id3510ff6dc)";
   const MicrodumpExtraInfo kMicrodumpExtraInfo(
       MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint));
-  scoped_array<char> buf;
+  std::string buf;
   MappingList no_mappings;
 
   CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf);
-  CheckMicrodumpContents(string(buf.get()), kMicrodumpExtraInfo);
+  AssertContainsMicrodump(buf);
+  CheckMicrodumpContents(buf, kMicrodumpExtraInfo);
 }
 
 TEST(MicrodumpWriterTest, NoProductInfo) {
   const char kBuildFingerprint[] = "foobar";
   const char kGPUFingerprint[] = "bazqux";
-  scoped_array<char> buf;
+  std::string buf;
   MappingList no_mappings;
 
   const MicrodumpExtraInfo kMicrodumpExtraInfoNoProductInfo(
       MakeMicrodumpExtraInfo(kBuildFingerprint, NULL, kGPUFingerprint));
 
   CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfoNoProductInfo, &buf);
-  CheckMicrodumpContents(string(buf.get()), kBuildFingerprint,
+  AssertContainsMicrodump(buf);
-                         "UNKNOWN:0.0.0.0", kGPUFingerprint);
+  CheckMicrodumpContents(buf, kBuildFingerprint, "UNKNOWN:0.0.0.0",
+                         kGPUFingerprint);
 }
 
 TEST(MicrodumpWriterTest, NoGPUInfo) {
   const char kProductInfo[] = "bazqux";
   const char kBuildFingerprint[] = "foobar";
-  scoped_array<char> buf;
+  std::string buf;
   MappingList no_mappings;
 
   const MicrodumpExtraInfo kMicrodumpExtraInfoNoGPUInfo(
       MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, NULL));
 
   CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfoNoGPUInfo, &buf);
-  CheckMicrodumpContents(string(buf.get()), kBuildFingerprint,
+  AssertContainsMicrodump(buf);
-                         kProductInfo, "UNKNOWN");
+  CheckMicrodumpContents(buf, kBuildFingerprint, kProductInfo, "UNKNOWN");
 }
 }  // namespace