Commit 77f953f1 authored by Vlad Tsyrklevich's avatar Vlad Tsyrklevich Committed by Commit Bot

Reland "GWP-ASan: Pack stack traces to save space"

This is a reland of a692ce8d, I forgot
to update a length test from kMaxStackFrames to kMaxPackedTraceLength in
the original change.

Original change's description:
> GWP-ASan: Pack stack traces to save space
>
> Make use of the stack trace packing from https://crrev.com/627865 to
> reduce memory usage to store stacks on 64-bit systems by ~60% percent. A
> GWP-ASan microbenchmark did not show this change to noticably regress
> performance.
>
> Bug: 896019, 921237
> Change-Id: Iad6e803f18ff12e5fd3079b9821d11bc0918bead
> Reviewed-on: https://chromium-review.googlesource.com/c/1448522
> Auto-Submit: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
> Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
> Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#628034}

Bug: 896019, 921237, 927488
Change-Id: I3160735a22e197b18911874e3667504f759552ee
Reviewed-on: https://chromium-review.googlesource.com/c/1449105Reviewed-by: default avatarVitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628260}
parent 0457a30a
......@@ -17,6 +17,7 @@
#include "components/crash/core/common/crash_key.h"
#include "components/gwp_asan/common/allocator_state.h"
#include "components/gwp_asan/common/crash_key_name.h"
#include "components/gwp_asan/common/pack_stack_trace.h"
namespace gwp_asan {
namespace internal {
......@@ -174,10 +175,13 @@ void GuardedPageAllocator::RecordAllocationInSlot(size_t slot,
slots_[slot].alloc_size = size;
slots_[slot].alloc_ptr = reinterpret_cast<uintptr_t>(ptr);
void* trace[AllocatorState::kMaxStackFrames];
size_t len =
base::debug::CollectStackTrace(trace, AllocatorState::kMaxStackFrames);
slots_[slot].alloc.trace_len = Pack(reinterpret_cast<uintptr_t*>(trace), len,
slots_[slot].alloc.packed_trace,
sizeof(slots_[slot].alloc.packed_trace));
slots_[slot].alloc.tid = base::PlatformThread::CurrentId();
slots_[slot].alloc.trace_len = base::debug::CollectStackTrace(
reinterpret_cast<void**>(&slots_[slot].alloc.trace),
AllocatorState::kMaxStackFrames);
slots_[slot].alloc.trace_collected = true;
slots_[slot].dealloc.tid = base::kInvalidThreadId;
......@@ -187,10 +191,14 @@ void GuardedPageAllocator::RecordAllocationInSlot(size_t slot,
}
void GuardedPageAllocator::RecordDeallocationInSlot(size_t slot) {
void* trace[AllocatorState::kMaxStackFrames];
size_t len =
base::debug::CollectStackTrace(trace, AllocatorState::kMaxStackFrames);
slots_[slot].dealloc.trace_len =
Pack(reinterpret_cast<uintptr_t*>(trace), len,
slots_[slot].dealloc.packed_trace,
sizeof(slots_[slot].dealloc.packed_trace));
slots_[slot].dealloc.tid = base::PlatformThread::CurrentId();
slots_[slot].dealloc.trace_len = base::debug::CollectStackTrace(
reinterpret_cast<void**>(&slots_[slot].dealloc.trace),
AllocatorState::kMaxStackFrames);
slots_[slot].dealloc.trace_collected = true;
}
......
......@@ -15,6 +15,7 @@ namespace internal {
// TODO: Delete out-of-line constexpr defininitons once C++17 is in use.
constexpr size_t AllocatorState::kGpaMaxPages;
constexpr size_t AllocatorState::kMaxStackFrames;
constexpr size_t AllocatorState::kMaxPackedTraceLength;
AllocatorState::AllocatorState() {}
......
......@@ -40,6 +40,9 @@ class AllocatorState {
static constexpr size_t kGpaMaxPages = 256;
// Maximum number of stack trace frames to collect.
static constexpr size_t kMaxStackFrames = 60;
// Number of bytes to allocate for packed stack traces. This can hold
// approximately kMaxStackFrames under normal conditions.
static constexpr size_t kMaxPackedTraceLength = 200;
enum class ErrorType {
kUseAfterFree = 0,
......@@ -65,9 +68,9 @@ class AllocatorState {
// (De)allocation thread id or base::kInvalidThreadId if no (de)allocation
// occurred.
base::PlatformThreadId tid = base::kInvalidThreadId;
// Stack trace contents.
uintptr_t trace[kMaxStackFrames];
// Stack trace length.
// Packed stack trace.
uint8_t packed_trace[kMaxPackedTraceLength];
// Length used to encode the packed stack trace.
size_t trace_len = 0;
// Whether a stack trace has been collected for this (de)allocation.
bool trace_collected = false;
......
......@@ -14,6 +14,7 @@
#include "build/build_config.h"
#include "components/gwp_asan/common/allocator_state.h"
#include "components/gwp_asan/common/crash_key_name.h"
#include "components/gwp_asan/common/pack_stack_trace.h"
#include "components/gwp_asan/crash_handler/crash.pb.h"
#include "third_party/crashpad/crashpad/client/annotation.h"
#include "third_party/crashpad/crashpad/snapshot/cpu_context.h"
......@@ -153,17 +154,26 @@ void CrashAnalyzer::ReadAllocationInfo(
if (!slot_info.trace_len || !slot_info.trace_collected)
return;
if (slot_info.trace_len > AllocatorState::kMaxStackFrames) {
if (slot_info.trace_len > AllocatorState::kMaxPackedTraceLength) {
DLOG(ERROR) << "Stack trace length is corrupted: " << slot_info.trace_len;
return;
}
// On 32-bit platforms we can't copy directly to
uintptr_t unpacked_stack_trace[AllocatorState::kMaxPackedTraceLength];
size_t unpacked_len =
Unpack(slot_info.packed_trace, slot_info.trace_len, unpacked_stack_trace,
AllocatorState::kMaxPackedTraceLength);
if (!unpacked_len) {
DLOG(ERROR) << "Failed to unpack stack trace.";
return;
}
// On 32-bit platforms we can't copy directly into
// proto_info->mutable_stack_trace()->mutable_data().
proto_info->mutable_stack_trace()->Resize(slot_info.trace_len, 0);
proto_info->mutable_stack_trace()->Resize(unpacked_len, 0);
uint64_t* output = proto_info->mutable_stack_trace()->mutable_data();
for (size_t i = 0; i < slot_info.trace_len; i++)
output[i] = slot_info.trace[i];
for (size_t i = 0; i < unpacked_len; i++)
output[i] = unpacked_stack_trace[i];
}
} // namespace internal
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment