From d780dcbaa5986f6e88f812d743d33d17b3f9a8e1 Mon Sep 17 00:00:00 2001
From: Colin Cross <ccross@android.com>
Date: Tue, 27 Nov 2018 16:14:53 -0800
Subject: [PATCH] Reduce logging around segfaults

The excessive logging of pages with segfaults is making it hard to
see what caused the problem, only log the first page that segfaults,
the range that was being walked when the first segfault happened,
and the total number of pages that segfaulted.

Bug: 120032857
Test: memunreachable_test --gtest_filter=HeapWalkerTest.segv
Change-Id: I71821a3f5be65f2fbcb36afc4b7b1ffa4a48e660
---
 libmemunreachable/HeapWalker.cpp | 16 +++++++++++++++-
 libmemunreachable/HeapWalker.h   | 10 ++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/libmemunreachable/HeapWalker.cpp b/libmemunreachable/HeapWalker.cpp
index 2403ad085..a046dad48 100644
--- a/libmemunreachable/HeapWalker.cpp
+++ b/libmemunreachable/HeapWalker.cpp
@@ -76,12 +76,14 @@ void HeapWalker::RecurseRoot(const Range& root) {
     Range range = to_do.back();
     to_do.pop_back();
 
+    walking_range_ = range;
     ForEachPtrInRange(range, [&](Range& ref_range, AllocationInfo* ref_info) {
       if (!ref_info->referenced_from_root) {
         ref_info->referenced_from_root = true;
         to_do.push_back(ref_range);
       }
     });
+    walking_range_ = Range{0, 0};
   }
 }
 
@@ -113,6 +115,10 @@ bool HeapWalker::DetectLeaks() {
 
   RecurseRoot(vals);
 
+  if (segv_page_count_ > 0) {
+    MEM_ALOGE("%zu pages skipped due to segfaults", segv_page_count_);
+  }
+
   return true;
 }
 
@@ -168,7 +174,15 @@ void HeapWalker::HandleSegFault(ScopedSignalHandler& handler, int signal, siginf
     handler.reset();
     return;
   }
-  MEM_ALOGW("failed to read page at %p, signal %d", si->si_addr, signal);
+  if (!segv_logged_) {
+    MEM_ALOGW("failed to read page at %p, signal %d", si->si_addr, signal);
+    if (walking_range_.begin != 0U) {
+      MEM_ALOGW("while walking range %p-%p", reinterpret_cast<void*>(walking_range_.begin),
+                reinterpret_cast<void*>(walking_range_.end));
+    }
+    segv_logged_ = true;
+  }
+  segv_page_count_++;
   if (!MapOverPage(si->si_addr)) {
     handler.reset();
   }
diff --git a/libmemunreachable/HeapWalker.h b/libmemunreachable/HeapWalker.h
index 92a832585..b37cc623b 100644
--- a/libmemunreachable/HeapWalker.h
+++ b/libmemunreachable/HeapWalker.h
@@ -53,7 +53,10 @@ class HeapWalker {
         roots_(allocator),
         root_vals_(allocator),
         segv_handler_(),
-        walking_ptr_(0) {
+        walking_ptr_(0),
+        walking_range_{0, 0},
+        segv_logged_(false),
+        segv_page_count_(0) {
     valid_allocations_range_.end = 0;
     valid_allocations_range_.begin = ~valid_allocations_range_.end;
 
@@ -100,7 +103,10 @@ class HeapWalker {
   allocator::vector<uintptr_t> root_vals_;
 
   ScopedSignalHandler segv_handler_;
-  uintptr_t walking_ptr_;
+  volatile uintptr_t walking_ptr_;
+  Range walking_range_;
+  bool segv_logged_;
+  size_t segv_page_count_;
 };
 
 template <class F>