From efaebfc482b7a44161015ef7f6661aa4526b1c26 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Mon, 19 Aug 2019 10:15:58 -0700 Subject: [PATCH] fuchsia: Capture from SP (+slop) to stack base, rather than entire stack Stack mappings can be enormous for some processes dwarfing all other data and making the .dmp useless. It isn't useful to capture beyond the stack pointer, so grab only from the stack base to the stack pointer. In the default config (safestack enabled), this isn't a major problem. However, Chromium has safestack disabled, along with a large stack size, so dumps with many threads become very large. Bug: https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=6425 Bug: chromium:821951 Change-Id: Iebefc5fe43e3d1bc4d8b66c107d3ab8ae5b3f68b Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1758702 Commit-Queue: Scott Graham Reviewed-by: Francois Rousseau --- snapshot/fuchsia/process_reader_fuchsia.cc | 16 +++++++++++++++- snapshot/fuchsia/process_reader_fuchsia_test.cc | 3 ++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/snapshot/fuchsia/process_reader_fuchsia.cc b/snapshot/fuchsia/process_reader_fuchsia.cc index e3eaf752..ecad29a4 100644 --- a/snapshot/fuchsia/process_reader_fuchsia.cc +++ b/snapshot/fuchsia/process_reader_fuchsia.cc @@ -61,8 +61,22 @@ void GetStackRegions( << "stack range is unexpectedly marked executable, continuing anyway"; } + // The stack covers [range_with_sp.base, range_with_sp.base + + // range_with_sp.size). The stack pointer (sp) can be anywhere in that range. + // It starts at the end of the range (range_with_sp.base + range_with_sp.size) + // and goes downwards until range_with_sp.base. Capture the part of the stack + // that is currently used: [sp, range_with_sp.base + range_with_sp.size). + + // Capture up to kExtraCaptureSize additional bytes of stack, but only if + // present in the region that was already found. + constexpr uint64_t kExtraCaptureSize = 128; + const uint64_t start_address = + std::max(sp >= kExtraCaptureSize ? sp - kExtraCaptureSize : sp, + range_with_sp.base); + const size_t region_size = + range_with_sp.size - (start_address - range_with_sp.base); stack_regions->push_back( - CheckedRange(range_with_sp.base, range_with_sp.size)); + CheckedRange(start_address, region_size)); // TODO(scottmg): https://crashpad.chromium.org/bug/196, once the retrievable // registers include FS and similar for ARM, retrieve the region for the diff --git a/snapshot/fuchsia/process_reader_fuchsia_test.cc b/snapshot/fuchsia/process_reader_fuchsia_test.cc index 369ec9a3..581fb4e4 100644 --- a/snapshot/fuchsia/process_reader_fuchsia_test.cc +++ b/snapshot/fuchsia/process_reader_fuchsia_test.cc @@ -174,7 +174,8 @@ class ThreadsChildTest : public MultiprocessExec { for (size_t i = 1; i < 6; ++i) { ASSERT_GT(threads[i].stack_regions.size(), 0u); - EXPECT_EQ(threads[i].stack_regions[0].size(), i * 4096u); + EXPECT_GT(threads[i].stack_regions[0].size(), 0u); + EXPECT_LE(threads[i].stack_regions[0].size(), i * 4096u); } }