win: Cap indirect memory gathering

Add a user-configurable cap on the amount of memory that is gathered by
dereferencing thread stacks. (SyzyAsan stores a tremendously large
number of pointers on the stack, so the dumps were ending up in the ~25M
range.)

Also reduce the range around pointers somewhat.

Change-Id: I6bce57d86bd2f6a796e1580c530909e089ec00ed
Reviewed-on: https://chromium-review.googlesource.com/338463
Reviewed-by: Mark Mentovai <mark@chromium.org>

client/crashpad_info.cc

@@ -96,10 +96,12 @@ CrashpadInfo::CrashpadInfo()
     : signature_(kSignature),
       size_(sizeof(*this)),
       version_(kCrashpadInfoVersion),
+      indirectly_referenced_memory_cap_(0),
+      padding_0_(0),
       crashpad_handler_behavior_(TriState::kUnset),
       system_crash_reporter_forwarding_(TriState::kUnset),
       gather_indirectly_referenced_memory_(TriState::kUnset),
-      padding_0_(0),
+      padding_1_(0),
       extra_memory_ranges_(nullptr),
       simple_annotations_(nullptr),
       user_data_minidump_stream_head_(nullptr)

client/crashpad_info.h

@@ -148,9 +148,17 @@ struct CrashpadInfo {
   //! This causes Crashpad to include pages of data referenced by locals or
   //! other stack memory. Turning this on can increase the size of the minidump
   //! significantly.
+  //!
+  //! \param[in] gather_indirectly_referenced_memory Whether extra memory should
+  //!     be gathered.
+  //! \param[in] limit The amount of memory in bytes after which no more
+  //!     indirectly gathered memory should be captured. This value is only used
+  //!     when \a gather_indirectly_referenced_memory is TriState::kEnabled.
   void set_gather_indirectly_referenced_memory(
-      TriState gather_indirectly_referenced_memory) {
+      TriState gather_indirectly_referenced_memory,
+      uint32_t limit) {
     gather_indirectly_referenced_memory_ = gather_indirectly_referenced_memory;
+    indirectly_referenced_memory_cap_ = limit;
   }
 
   //! \brief Adds a custom stream to the minidump.
@@ -192,10 +200,12 @@ struct CrashpadInfo {
   uint32_t signature_;  // kSignature
   uint32_t size_;  // The size of the entire CrashpadInfo structure.
   uint32_t version_;  // kVersion
+  uint32_t indirectly_referenced_memory_cap_;
+  uint32_t padding_0_;
   TriState crashpad_handler_behavior_;
   TriState system_crash_reporter_forwarding_;
   TriState gather_indirectly_referenced_memory_;
-  uint8_t padding_0_;
+  uint8_t padding_1_;
   SimpleAddressRangeBag* extra_memory_ranges_;  // weak
   SimpleStringDictionary* simple_annotations_;  // weak
   internal::UserDataMinidumpStreamListEntry* user_data_minidump_stream_head_;

handler/win/crashy_test_program.cc

@@ -223,7 +223,8 @@ int CrashyMain(int argc, wchar_t* argv[]) {
   printf("%p, %p\n", offset_pointer, &offset_pointer);
 
   crashpad::CrashpadInfo::GetCrashpadInfo()
-      ->set_gather_indirectly_referenced_memory(TriState::kEnabled);
+      ->set_gather_indirectly_referenced_memory(
+          TriState::kEnabled, std::numeric_limits<uint32_t>::max());
 
   std::vector<uint8_t> data_stream1(128, 'x');
   crashpad::CrashpadInfo::GetCrashpadInfo()->AddUserDataMinidumpStream(

snapshot/capture_memory.cc

@@ -38,9 +38,9 @@ void MaybeCaptureMemoryAround(CaptureMemory::Delegate* delegate,
   if (address > max_address - non_address_offset)
     return;
 
-  const uint64_t kRegisterByteOffset = 256;
+  const uint64_t kRegisterByteOffset = 128;
   const uint64_t target = address - kRegisterByteOffset;
-  const uint64_t size = 1024;
+  const uint64_t size = 512;
   static_assert(kRegisterByteOffset <= size / 2,
                 "negative offset too large");
   auto ranges =

snapshot/crashpad_info_client_options.h

@@ -62,6 +62,9 @@ struct CrashpadInfoClientOptions {
 
   //! \sa CrashpadInfo::set_gather_indirectly_referenced_memory()
   TriState gather_indirectly_referenced_memory;
+
+  //! \sa CrashpadInfo::set_gather_indirectly_referenced_memory()
+  uint32_t indirectly_referenced_memory_cap;
 };
 
 }  // namespace crashpad

snapshot/crashpad_info_client_options_test.cc

@@ -61,7 +61,8 @@ class ScopedUnsetCrashpadInfoOptions {
   ~ScopedUnsetCrashpadInfoOptions() {
     crashpad_info_->set_crashpad_handler_behavior(TriState::kUnset);
     crashpad_info_->set_system_crash_reporter_forwarding(TriState::kUnset);
-    crashpad_info_->set_gather_indirectly_referenced_memory(TriState::kUnset);
+    crashpad_info_->set_gather_indirectly_referenced_memory(TriState::kUnset,
+                                                            0);
   }
 
  private:
@@ -94,6 +95,7 @@ TEST(CrashpadInfoClientOptions, OneModule) {
   EXPECT_EQ(TriState::kUnset, options.crashpad_handler_behavior);
   EXPECT_EQ(TriState::kUnset, options.system_crash_reporter_forwarding);
   EXPECT_EQ(TriState::kUnset, options.gather_indirectly_referenced_memory);
+  EXPECT_EQ(0u, options.indirectly_referenced_memory_cap);
 
   CrashpadInfo* crashpad_info = CrashpadInfo::GetCrashpadInfo();
   ASSERT_TRUE(crashpad_info);
@@ -107,6 +109,7 @@ TEST(CrashpadInfoClientOptions, OneModule) {
     EXPECT_EQ(TriState::kEnabled, options.crashpad_handler_behavior);
     EXPECT_EQ(TriState::kUnset, options.system_crash_reporter_forwarding);
     EXPECT_EQ(TriState::kUnset, options.gather_indirectly_referenced_memory);
+    EXPECT_EQ(0u, options.indirectly_referenced_memory_cap);
   }
 
   {
@@ -118,17 +121,20 @@ TEST(CrashpadInfoClientOptions, OneModule) {
     EXPECT_EQ(TriState::kUnset, options.crashpad_handler_behavior);
     EXPECT_EQ(TriState::kDisabled, options.system_crash_reporter_forwarding);
     EXPECT_EQ(TriState::kUnset, options.gather_indirectly_referenced_memory);
+    EXPECT_EQ(0u, options.indirectly_referenced_memory_cap);
   }
 
   {
     ScopedUnsetCrashpadInfoOptions unset(crashpad_info);
 
-    crashpad_info->set_gather_indirectly_referenced_memory(TriState::kEnabled);
+    crashpad_info->set_gather_indirectly_referenced_memory(TriState::kEnabled,
+                                                           1234);
 
     options = SelfProcessSnapshotAndGetCrashpadOptions();
     EXPECT_EQ(TriState::kUnset, options.crashpad_handler_behavior);
     EXPECT_EQ(TriState::kUnset, options.system_crash_reporter_forwarding);
     EXPECT_EQ(TriState::kEnabled, options.gather_indirectly_referenced_memory);
+    EXPECT_EQ(1234u, options.indirectly_referenced_memory_cap);
   }
 }
 

snapshot/mac/process_snapshot_mac.cc

@@ -107,6 +107,8 @@ void ProcessSnapshotMac::GetCrashpadOptions(
     if (local_options.gather_indirectly_referenced_memory == TriState::kUnset) {
       local_options.gather_indirectly_referenced_memory =
           module_options.gather_indirectly_referenced_memory;
+      local_options.indirectly_referenced_memory_cap =
+          module_options.indirectly_referenced_memory_cap;
     }
 
     // If non-default values have been found for all options, the loop can end

snapshot/mac/process_types/crashpad_info.proctype

@@ -27,6 +27,8 @@ PROCESS_TYPE_STRUCT_BEGIN(CrashpadInfo)
   PROCESS_TYPE_STRUCT_MEMBER(uint32_t, signature)
   PROCESS_TYPE_STRUCT_MEMBER(uint32_t, size)
   PROCESS_TYPE_STRUCT_MEMBER(uint32_t, version)
+  PROCESS_TYPE_STRUCT_MEMBER(uint32_t, indirectly_referenced_memory_cap)
+  PROCESS_TYPE_STRUCT_MEMBER(uint32_t, padding_0)
   PROCESS_TYPE_STRUCT_MEMBER(uint8_t, crashpad_handler_behavior)  // TriState
 
   // TriState
@@ -35,7 +37,7 @@ PROCESS_TYPE_STRUCT_BEGIN(CrashpadInfo)
   // TriState
   PROCESS_TYPE_STRUCT_MEMBER(uint8_t, gather_indirectly_referenced_memory)
 
-  PROCESS_TYPE_STRUCT_MEMBER(uint8_t, padding_0)
+  PROCESS_TYPE_STRUCT_MEMBER(uint8_t, padding_1)
 
   // SimpleAddressRangeBag*
   PROCESS_TYPE_STRUCT_MEMBER(Pointer, extra_memory_ranges)

snapshot/win/capture_memory_delegate_win.cc

@@ -14,6 +14,7 @@
 
 #include "snapshot/win/capture_memory_delegate_win.h"
 
+#include "base/numerics/safe_conversions.h"
 #include "snapshot/win/memory_snapshot_win.h"
 
 namespace crashpad {
@@ -22,10 +23,12 @@ namespace internal {
 CaptureMemoryDelegateWin::CaptureMemoryDelegateWin(
     ProcessReaderWin* process_reader,
     const ProcessReaderWin::Thread& thread,
-    PointerVector<MemorySnapshotWin>* snapshots)
+    PointerVector<MemorySnapshotWin>* snapshots,
+    uint32_t* budget_remaining)
     : stack_(thread.stack_region_address, thread.stack_region_size),
       process_reader_(process_reader),
-      snapshots_(snapshots) {}
+      snapshots_(snapshots),
+      budget_remaining_(budget_remaining) {}
 
 bool CaptureMemoryDelegateWin::Is64Bit() const {
   return process_reader_->Is64Bit();
@@ -49,9 +52,20 @@ void CaptureMemoryDelegateWin::AddNewMemorySnapshot(
     return;
   if (range.size() == 0)
     return;
+  if (budget_remaining_ && *budget_remaining_ == 0)
+    return;
   internal::MemorySnapshotWin* snapshot = new internal::MemorySnapshotWin();
   snapshot->Initialize(process_reader_, range.base(), range.size());
   snapshots_->push_back(snapshot);
+  if (budget_remaining_) {
+    if (!base::IsValueInRangeForNumericType<int64_t>(range.size())) {
+      *budget_remaining_ = 0;
+    } else {
+      int64_t temp = *budget_remaining_;
+      temp -= range.size();
+      *budget_remaining_ = base::saturated_cast<uint32_t>(temp);
+    }
+  }
 }
 
 }  // namespace internal

snapshot/win/capture_memory_delegate_win.h

@@ -35,9 +35,13 @@ class CaptureMemoryDelegateWin : public CaptureMemory::Delegate {
   //!     already captured elsewhere.
   //! \param[in] snapshots A vector of MemorySnapshotWin to which the captured
   //!     memory will be added.
+  //! \param[in] budget_remaining If non-null, a pointer to the remaining number
+  //!     of bytes to capture. If this is `0`, no further memory will be
+  //!     captured.
   CaptureMemoryDelegateWin(ProcessReaderWin* process_reader,
                            const ProcessReaderWin::Thread& thread,
-                           PointerVector<MemorySnapshotWin>* snapshots);
+                           PointerVector<MemorySnapshotWin>* snapshots,
+                           uint32_t* budget_remaining);
 
   // MemoryCaptureDelegate:
   bool Is64Bit() const override;
@@ -50,6 +54,7 @@ class CaptureMemoryDelegateWin : public CaptureMemory::Delegate {
   CheckedRange<uint64_t, uint64_t> stack_;
   ProcessReaderWin* process_reader_;
   PointerVector<MemorySnapshotWin>* snapshots_;
+  uint32_t* budget_remaining_;
 };
 
 }  // namespace internal

snapshot/win/exception_snapshot_win.cc

@@ -91,7 +91,7 @@ bool ExceptionSnapshotWin::Initialize(ProcessReaderWin* process_reader,
   }
 
   CaptureMemoryDelegateWin capture_memory_delegate(
-      process_reader, *thread, &extra_memory_);
+      process_reader, *thread, &extra_memory_, nullptr);
   CaptureMemory::PointedToByContext(context_, &capture_memory_delegate);
 
   INITIALIZATION_STATE_SET_VALID(initialized_);

snapshot/win/module_snapshot_win.cc

@@ -223,6 +223,8 @@ void ModuleSnapshotWin::GetCrashpadOptionsInternal(
   if (!pe_image_reader_->GetCrashpadInfo(&crashpad_info)) {
     options->crashpad_handler_behavior = TriState::kUnset;
     options->system_crash_reporter_forwarding = TriState::kUnset;
+    options->gather_indirectly_referenced_memory = TriState::kUnset;
+    options->indirectly_referenced_memory_cap = 0;
     return;
   }
 
@@ -237,6 +239,9 @@ void ModuleSnapshotWin::GetCrashpadOptionsInternal(
   options->gather_indirectly_referenced_memory =
       CrashpadInfoClientOptions::TriStateFromCrashpadInfo(
           crashpad_info.gather_indirectly_referenced_memory);
+
+  options->indirectly_referenced_memory_cap =
+      crashpad_info.indirectly_referenced_memory_cap;
 }
 
 const VS_FIXEDFILEINFO* ModuleSnapshotWin::VSFixedFileInfo() const {

snapshot/win/pe_image_reader.h

@@ -39,10 +39,12 @@ struct CrashpadInfo {
   uint32_t signature;
   uint32_t size;
   uint32_t version;
+  uint32_t indirectly_referenced_memory_cap;
+  uint32_t padding_0;
   uint8_t crashpad_handler_behavior;  // TriState.
   uint8_t system_crash_reporter_forwarding;  // TriState.
   uint8_t gather_indirectly_referenced_memory;  // TriState.
-  uint8_t padding_0;
+  uint8_t padding_1;
   typename Traits::Pointer extra_address_ranges;
   typename Traits::Pointer simple_annotations;
   typename Traits::Pointer user_data_minidump_stream_head;

snapshot/win/process_snapshot_win.cc

@@ -72,8 +72,9 @@ bool ProcessSnapshotWin::Initialize(
 
   GetCrashpadOptionsInternal(&options_);
 
-  InitializeThreads(options_.gather_indirectly_referenced_memory ==
-                    TriState::kEnabled);
+  InitializeThreads(
+      options_.gather_indirectly_referenced_memory == TriState::kEnabled,
+      options_.indirectly_referenced_memory_cap);
 
   for (const MEMORY_BASIC_INFORMATION64& mbi :
        process_reader_.GetProcessInfo().MemoryInfo()) {
@@ -230,15 +231,20 @@ std::vector<const MemorySnapshot*> ProcessSnapshotWin::ExtraMemory() const {
 }
 
 void ProcessSnapshotWin::InitializeThreads(
-    bool gather_indirectly_referenced_memory) {
+    bool gather_indirectly_referenced_memory,
+    uint32_t indirectly_referenced_memory_cap) {
   const std::vector<ProcessReaderWin::Thread>& process_reader_threads =
       process_reader_.Threads();
   for (const ProcessReaderWin::Thread& process_reader_thread :
        process_reader_threads) {
     auto thread = make_scoped_ptr(new internal::ThreadSnapshotWin());
+    uint32_t* budget_remaining_pointer = nullptr;
+    uint32_t budget_remaining = indirectly_referenced_memory_cap;
+    if (gather_indirectly_referenced_memory)
+      budget_remaining_pointer = &budget_remaining;
     if (thread->Initialize(&process_reader_,
                            process_reader_thread,
-                           gather_indirectly_referenced_memory)) {
+                           budget_remaining_pointer)) {
       threads_.push_back(thread.release());
     }
   }
@@ -322,6 +328,8 @@ void ProcessSnapshotWin::GetCrashpadOptionsInternal(
     if (local_options.gather_indirectly_referenced_memory == TriState::kUnset) {
       local_options.gather_indirectly_referenced_memory =
           module_options.gather_indirectly_referenced_memory;
+      local_options.indirectly_referenced_memory_cap =
+          module_options.indirectly_referenced_memory_cap;
     }
 
     // If non-default values have been found for all options, the loop can end

snapshot/win/process_snapshot_win.h

@@ -16,6 +16,7 @@
 #define CRASHPAD_SNAPSHOT_WIN_PROCESS_SNAPSHOT_WIN_H_
 
 #include <windows.h>
+#include <stdint.h>
 #include <sys/time.h>
 #include <sys/types.h>
 
@@ -143,7 +144,8 @@ class ProcessSnapshotWin final : public ProcessSnapshot {
 
  private:
   // Initializes threads_ on behalf of Initialize().
-  void InitializeThreads(bool gather_indirectly_referenced_memory);
+  void InitializeThreads(bool gather_indirectly_referenced_memory,
+                         uint32_t indirectly_referenced_memory_cap);
 
   // Initializes modules_ on behalf of Initialize().
   void InitializeModules();

snapshot/win/thread_snapshot_win.cc

@@ -40,7 +40,7 @@ ThreadSnapshotWin::~ThreadSnapshotWin() {
 bool ThreadSnapshotWin::Initialize(
     ProcessReaderWin* process_reader,
     const ProcessReaderWin::Thread& process_reader_thread,
-    bool gather_indirectly_referenced_memory) {
+    uint32_t* gather_indirectly_referenced_memory_bytes_remaining) {
   INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
 
   thread_ = process_reader_thread;
@@ -79,10 +79,14 @@ bool ThreadSnapshotWin::Initialize(
 #endif  // ARCH_CPU_X86_64
 
   CaptureMemoryDelegateWin capture_memory_delegate(
-      process_reader, thread_, &pointed_to_memory_);
+      process_reader,
+      thread_,
+      &pointed_to_memory_,
+      gather_indirectly_referenced_memory_bytes_remaining);
   CaptureMemory::PointedToByContext(context_, &capture_memory_delegate);
-  if (gather_indirectly_referenced_memory)
+  if (gather_indirectly_referenced_memory_bytes_remaining) {
     CaptureMemory::PointedToByMemoryRange(stack_, &capture_memory_delegate);
+  }
 
   INITIALIZATION_STATE_SET_VALID(initialized_);
   return true;

snapshot/win/thread_snapshot_win.h

@@ -48,14 +48,17 @@ class ThreadSnapshotWin final : public ThreadSnapshot {
   //!     the thread.
   //! \param[in] process_reader_thread The thread within the ProcessReaderWin
   //!     for which the snapshot should be created.
-  //! \param[in] gather_indirectly_referenced_memory If `true`, adds extra
-  //!     memory regions to the snapshot pointed to by the thread's stack.
+  //! \param[in] gather_indirectly_referenced_memory_bytes_remaining. If
+  //!     non-null, add extra memory regions to the snapshot pointed to by the
+  //!     thread's stack. The size of the regions added is subtracted from the
+  //!     count, and when it's `0`, no more regions will be added.
   //!
   //! \return `true` if the snapshot could be created, `false` otherwise with
   //!     an appropriate message logged.
-  bool Initialize(ProcessReaderWin* process_reader,
-                  const ProcessReaderWin::Thread& process_reader_thread,
-                  bool gather_indirectly_referenced_memory);
+  bool Initialize(
+      ProcessReaderWin* process_reader,
+      const ProcessReaderWin::Thread& process_reader_thread,
+      uint32_t* gather_indirectly_referenced_memory_bytes_remaining);
 
   // ThreadSnapshot: