linux, win: allow disabling extra memory collection

Change-Id: If95c89d554c061522627681af78c8c0d1725df2c
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/3335359
Reviewed-by: Mark Mentovai <mark@chromium.org>

client/crashpad_client_linux_test.cc

@@ -26,6 +26,7 @@
 #include "client/annotation.h"
 #include "client/annotation_list.h"
 #include "client/crash_report_database.h"
+#include "client/crashpad_info.h"
 #include "client/simulate_crash.h"
 #include "gtest/gtest.h"
 #include "snapshot/annotation_snapshot.h"
@@ -73,12 +74,13 @@ struct StartHandlerForSelfTestOptions {
   bool set_first_chance_handler;
   bool crash_non_main_thread;
   bool client_uses_signals;
+  bool gather_indirectly_referenced_memory;
   CrashType crash_type;
 };
 
 class StartHandlerForSelfTest
     : public testing::TestWithParam<
-          std::tuple<bool, bool, bool, bool, CrashType>> {
+          std::tuple<bool, bool, bool, bool, bool, CrashType>> {
  public:
   StartHandlerForSelfTest() = default;
 
@@ -92,6 +94,7 @@ class StartHandlerForSelfTest
              options_.set_first_chance_handler,
              options_.crash_non_main_thread,
              options_.client_uses_signals,
+             options_.gather_indirectly_referenced_memory,
              options_.crash_type) = GetParam();
   }
 
@@ -147,7 +150,8 @@ void ValidateAttachment(const CrashReportDatabase::UploadReport* report) {
             0);
 }
 
-void ValidateExtraMemory(const ProcessSnapshotMinidump& minidump) {
+void ValidateExtraMemory(const StartHandlerForSelfTestOptions& options,
+                         const ProcessSnapshotMinidump& minidump) {
   // Verify that if we have an exception, then the code around the instruction
   // pointer is included in the extra memory.
   const ExceptionSnapshot* exception = minidump.Exception();
@@ -164,10 +168,11 @@ void ValidateExtraMemory(const ProcessSnapshotMinidump& minidump) {
       break;
     }
   }
-  EXPECT_TRUE(pc_found);
+  EXPECT_EQ(pc_found, options.gather_indirectly_referenced_memory);
 }
 
-void ValidateDump(const CrashReportDatabase::UploadReport* report) {
+void ValidateDump(const StartHandlerForSelfTestOptions& options,
+                  const CrashReportDatabase::UploadReport* report) {
   ProcessSnapshotMinidump minidump_snapshot;
   ASSERT_TRUE(minidump_snapshot.Initialize(report->Reader()));
 
@@ -184,7 +189,7 @@ void ValidateDump(const CrashReportDatabase::UploadReport* report) {
 #endif
   ValidateAttachment(report);
 
-  ValidateExtraMemory(minidump_snapshot);
+  ValidateExtraMemory(options, minidump_snapshot);
 
   for (const ModuleSnapshot* module : minidump_snapshot.Modules()) {
     for (const AnnotationSnapshot& annotation : module->AnnotationObjects()) {
@@ -330,6 +335,11 @@ CRASHPAD_CHILD_TEST_MAIN(StartHandlerForSelfTestChild) {
         client_handler, SA_ONSTACK, &old_actions));
   }
 
+  if (options.gather_indirectly_referenced_memory) {
+    CrashpadInfo::GetCrashpadInfo()->set_gather_indirectly_referenced_memory(
+        TriState::kEnabled, 1024 * 1024 * 4);
+  }
+
   base::FilePath handler_path = TestPaths::Executable().DirName().Append(
       FILE_PATH_LITERAL("crashpad_handler"));
 
@@ -442,7 +452,7 @@ class StartHandlerForSelfInChildTest : public MultiprocessExec {
     std::unique_ptr<const CrashReportDatabase::UploadReport> report;
     ASSERT_EQ(database->GetReportForUploading(reports[0].uuid, &report),
               CrashReportDatabase::kNoError);
-    ValidateDump(report.get());
+    ValidateDump(options_, report.get());
   }
 
   StartHandlerForSelfTestOptions options_;
@@ -471,6 +481,7 @@ INSTANTIATE_TEST_SUITE_P(
                      testing::Bool(),
                      testing::Bool(),
                      testing::Bool(),
+                     testing::Bool(),
                      testing::Values(CrashType::kSimulated,
                                      CrashType::kBuiltinTrap,
                                      CrashType::kInfiniteRecursion)));

snapshot/crashpad_info_client_options_test.cc

@@ -144,7 +144,7 @@ TEST(CrashpadInfoClientOptions, OneModule) {
     EXPECT_EQ(options.crashpad_handler_behavior, TriState::kUnset);
     EXPECT_EQ(options.system_crash_reporter_forwarding, TriState::kUnset);
     EXPECT_EQ(options.gather_indirectly_referenced_memory, TriState::kEnabled);
-    EXPECT_EQ(options.indirectly_referenced_memory_cap, 1234u);
+    EXPECT_LE(options.indirectly_referenced_memory_cap, 1234u);
   }
 }
 

snapshot/linux/capture_memory_delegate_linux.cc

@@ -57,7 +57,7 @@ void CaptureMemoryDelegateLinux::AddNewMemorySnapshot(
     return;
   if (range.size() == 0)
     return;
-  if (budget_remaining_ && *budget_remaining_ == 0)
+  if (!budget_remaining_ || *budget_remaining_ == 0)
     return;
   snapshots_->push_back(std::make_unique<internal::MemorySnapshotGeneric>());
   internal::MemorySnapshotGeneric* snapshot = snapshots_->back().get();

snapshot/linux/exception_snapshot_linux.cc

@@ -326,10 +326,12 @@ bool ExceptionSnapshotLinux::ReadContext<ContextTraits64>(
 
 #endif  // ARCH_CPU_X86_FAMILY
 
-bool ExceptionSnapshotLinux::Initialize(ProcessReaderLinux* process_reader,
+bool ExceptionSnapshotLinux::Initialize(
-                                        LinuxVMAddress siginfo_address,
+    ProcessReaderLinux* process_reader,
-                                        LinuxVMAddress context_address,
+    LinuxVMAddress siginfo_address,
-                                        pid_t thread_id) {
+    LinuxVMAddress context_address,
+    pid_t thread_id,
+    uint32_t* gather_indirectly_referenced_memory_cap) {
   INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
 
   thread_id_ = thread_id;
@@ -359,7 +361,10 @@ bool ExceptionSnapshotLinux::Initialize(ProcessReaderLinux* process_reader,
   }
 
   CaptureMemoryDelegateLinux capture_memory_delegate(
-      process_reader, thread, &extra_memory_, nullptr);
+      process_reader,
+      thread,
+      &extra_memory_,
+      gather_indirectly_referenced_memory_cap);
   CaptureMemory::PointedToByContext(context_, &capture_memory_delegate);
 
   INITIALIZATION_STATE_SET_VALID(initialized_);

snapshot/linux/exception_snapshot_linux.h

@@ -59,7 +59,8 @@ class ExceptionSnapshotLinux final : public ExceptionSnapshot {
   bool Initialize(ProcessReaderLinux* process_reader,
                   LinuxVMAddress siginfo_address,
                   LinuxVMAddress context_address,
-                  pid_t thread_id);
+                  pid_t thread_id,
+                  uint32_t* gather_indirectly_referenced_memory_cap);
 
   // ExceptionSnapshot:
 

snapshot/linux/exception_snapshot_linux_test.cc

@@ -320,7 +320,8 @@ TEST(ExceptionSnapshotLinux, SelfBasic) {
   ASSERT_TRUE(exception.Initialize(&process_reader,
                                    FromPointerCast<LinuxVMAddress>(&siginfo),
                                    FromPointerCast<LinuxVMAddress>(&context),
-                                   gettid()));
+                                   gettid(),
+                                   nullptr));
   EXPECT_EQ(exception.Exception(), static_cast<uint32_t>(siginfo.si_signo));
   EXPECT_EQ(exception.ExceptionInfo(), static_cast<uint32_t>(siginfo.si_code));
   EXPECT_EQ(exception.ExceptionAddress(),
@@ -393,7 +394,8 @@ class RaiseTest {
     ASSERT_TRUE(exception.Initialize(&process_reader,
                                      FromPointerCast<LinuxVMAddress>(siginfo),
                                      FromPointerCast<LinuxVMAddress>(context),
-                                     gettid()));
+                                     gettid(),
+                                     nullptr));
 
     EXPECT_EQ(exception.Exception(), static_cast<uint32_t>(kSigno));
 
@@ -464,7 +466,8 @@ class TimerTest {
     ASSERT_TRUE(exception.Initialize(&process_reader,
                                      FromPointerCast<LinuxVMAddress>(siginfo),
                                      FromPointerCast<LinuxVMAddress>(context),
-                                     gettid()));
+                                     gettid(),
+                                     nullptr));
 
     EXPECT_EQ(exception.Exception(), static_cast<uint32_t>(kSigno));
 

snapshot/linux/process_snapshot_linux.cc

@@ -42,10 +42,9 @@ bool ProcessSnapshotLinux::Initialize(PtraceConnection* connection) {
   client_id_.InitializeToZero();
   system_.Initialize(&process_reader_, &snapshot_time_);
 
-  GetCrashpadOptionsInternal((&options_));
-
-  InitializeThreads();
   InitializeModules();
+  GetCrashpadOptionsInternal((&options_));
+  InitializeThreads();
   InitializeAnnotations();
 
   INITIALIZATION_STATE_SET_VALID(initialized_);
@@ -83,11 +82,17 @@ bool ProcessSnapshotLinux::InitializeException(
     info.thread_id = exception_thread_id;
   }
 
+  uint32_t* budget_remaining_pointer =
+      options_.gather_indirectly_referenced_memory == TriState::kEnabled
+          ? &options_.indirectly_referenced_memory_cap
+          : nullptr;
+
   exception_.reset(new internal::ExceptionSnapshotLinux());
   if (!exception_->Initialize(&process_reader_,
                               info.siginfo_address,
                               info.context_address,
-                              info.thread_id)) {
+                              info.thread_id,
+                              budget_remaining_pointer)) {
     exception_.reset();
     return false;
   }
@@ -269,11 +274,11 @@ const ProcessMemory* ProcessSnapshotLinux::Memory() const {
 void ProcessSnapshotLinux::InitializeThreads() {
   const std::vector<ProcessReaderLinux::Thread>& process_reader_threads =
       process_reader_.Threads();
-  uint32_t* budget_remaining_pointer = nullptr;
+  uint32_t* budget_remaining_pointer =
-  uint32_t budget_remaining = options_.indirectly_referenced_memory_cap;
+      options_.gather_indirectly_referenced_memory == TriState::kEnabled
-  if (options_.gather_indirectly_referenced_memory == TriState::kEnabled) {
+          ? &options_.indirectly_referenced_memory_cap
-    budget_remaining_pointer = &budget_remaining;
+          : nullptr;
-  }
+
   for (const ProcessReaderLinux::Thread& process_reader_thread :
        process_reader_threads) {
     auto thread = std::make_unique<internal::ThreadSnapshotLinux>();

snapshot/win/capture_memory_delegate_win.cc

@@ -55,7 +55,7 @@ void CaptureMemoryDelegateWin::AddNewMemorySnapshot(
     return;
   if (range.size() == 0)
     return;
-  if (budget_remaining_ && *budget_remaining_ == 0)
+  if (!budget_remaining_ || *budget_remaining_ == 0)
     return;
   snapshots_->push_back(std::make_unique<internal::MemorySnapshotGeneric>());
   internal::MemorySnapshotGeneric* snapshot = snapshots_->back().get();

snapshot/win/exception_snapshot_win.cc

@@ -84,7 +84,8 @@ ExceptionSnapshotWin::~ExceptionSnapshotWin() {
 bool ExceptionSnapshotWin::Initialize(
     ProcessReaderWin* process_reader,
     DWORD thread_id,
-    WinVMAddress exception_pointers_address) {
+    WinVMAddress exception_pointers_address,
+    uint32_t* gather_indirectly_referenced_memory_cap) {
   INITIALIZATION_STATE_SET_INITIALIZING(initialized_);
 
   const ProcessReaderWin::Thread* thread = nullptr;
@@ -132,7 +133,10 @@ bool ExceptionSnapshotWin::Initialize(
 #endif
 
   CaptureMemoryDelegateWin capture_memory_delegate(
-      process_reader, *thread, &extra_memory_, nullptr);
+      process_reader,
+      *thread,
+      &extra_memory_,
+      gather_indirectly_referenced_memory_cap);
   CaptureMemory::PointedToByContext(context_, &capture_memory_delegate);
 
   INITIALIZATION_STATE_SET_VALID(initialized_);

snapshot/win/exception_snapshot_win.h

@@ -72,7 +72,8 @@ class ExceptionSnapshotWin final : public ExceptionSnapshot {
   //!     an appropriate message logged.
   bool Initialize(ProcessReaderWin* process_reader,
                   DWORD thread_id,
-                  WinVMAddress exception_pointers);
+                  WinVMAddress exception_pointers,
+                  uint32_t* gather_indirectly_referenced_memory_cap);
 
   // ExceptionSnapshot:
 

snapshot/win/process_snapshot_win.cc

@@ -63,24 +63,6 @@ bool ProcessSnapshotWin::Initialize(
   if (!process_reader_.Initialize(process, suspension_state))
     return false;
 
-  if (exception_information_address != 0) {
-    ExceptionInformation exception_information = {};
-    if (!process_reader_.Memory()->Read(exception_information_address,
-                                        sizeof(exception_information),
-                                        &exception_information)) {
-      LOG(WARNING) << "ReadMemory ExceptionInformation failed";
-      return false;
-    }
-
-    exception_.reset(new internal::ExceptionSnapshotWin());
-    if (!exception_->Initialize(&process_reader_,
-                                exception_information.thread_id,
-                                exception_information.exception_pointers)) {
-      exception_.reset();
-      return false;
-    }
-  }
-
   client_id_.InitializeToZero();
   system_.Initialize(&process_reader_);
 
@@ -96,10 +78,31 @@ bool ProcessSnapshotWin::Initialize(
   InitializeUnloadedModules();
 
   GetCrashpadOptionsInternal(&options_);
+  uint32_t* budget_remaining_pointer =
+      options_.gather_indirectly_referenced_memory == TriState::kEnabled
+          ? &options_.indirectly_referenced_memory_cap
+          : nullptr;
 
-  InitializeThreads(
+  if (exception_information_address != 0) {
-      options_.gather_indirectly_referenced_memory == TriState::kEnabled,
+    ExceptionInformation exception_information = {};
-      options_.indirectly_referenced_memory_cap);
+    if (!process_reader_.Memory()->Read(exception_information_address,
+                                        sizeof(exception_information),
+                                        &exception_information)) {
+      LOG(WARNING) << "ReadMemory ExceptionInformation failed";
+      return false;
+    }
+
+    exception_.reset(new internal::ExceptionSnapshotWin());
+    if (!exception_->Initialize(&process_reader_,
+                                exception_information.thread_id,
+                                exception_information.exception_pointers,
+                                budget_remaining_pointer)) {
+      exception_.reset();
+      return false;
+    }
+  }
+
+  InitializeThreads(budget_remaining_pointer);
 
   for (const MEMORY_BASIC_INFORMATION64& mbi :
        process_reader_.GetProcessInfo().MemoryInfo()) {
@@ -239,15 +242,9 @@ const ProcessMemory* ProcessSnapshotWin::Memory() const {
   return process_reader_.Memory();
 }
 
-void ProcessSnapshotWin::InitializeThreads(
+void ProcessSnapshotWin::InitializeThreads(uint32_t* budget_remaining_pointer) {
-    bool gather_indirectly_referenced_memory,
-    uint32_t indirectly_referenced_memory_cap) {
   const std::vector<ProcessReaderWin::Thread>& process_reader_threads =
       process_reader_.Threads();
-  uint32_t* budget_remaining_pointer = nullptr;
-  uint32_t budget_remaining = indirectly_referenced_memory_cap;
-  if (gather_indirectly_referenced_memory)
-    budget_remaining_pointer = &budget_remaining;
   for (const ProcessReaderWin::Thread& process_reader_thread :
        process_reader_threads) {
     auto thread = std::make_unique<internal::ThreadSnapshotWin>();

snapshot/win/process_snapshot_win.h

@@ -137,8 +137,7 @@ class ProcessSnapshotWin final : public ProcessSnapshot {
 
  private:
   // Initializes threads_ on behalf of Initialize().
-  void InitializeThreads(bool gather_indirectly_referenced_memory,
+  void InitializeThreads(uint32_t* indirectly_referenced_memory_cap);
-                         uint32_t indirectly_referenced_memory_cap);
 
   // Initializes modules_ on behalf of Initialize().
   void InitializeModules();