[PATCH] D66189: [GWP-ASan] Implement stack frame compression.

Thu Aug 15 12:47:23 PDT 2019

vlad.tsyrklevich added inline comments.

================
Comment at: compiler-rt/lib/gwp_asan/options.inc:28

+GWP_ASAN_OPTION(
+    int, MaxTraceLength, 128,
----------------
hctim wrote:
> vlad.tsyrklevich wrote:
> > hctim wrote:
> > > vlad.tsyrklevich wrote:
> > > > Why make this an option? I don't see why users would need to tune it, I'd set it to something reasonable (64?) and just leave it as a constant.
> > > Agreed that runtime users shouldn't generally need to tune this, but this is more for the supporting allocator to tune. I'd expect that bionic on Android has vastly different requirements for scudo on fuchsia for example, and this allows them to tune it in the supporting allocator. Scudo currently exposes this to the user (as they use `optional/options_parser.cpp` from sanitizer-common to enumaret the options), but allocators don't have to expose this option to the user.
> > I don't understand how the bionic/scudo examples have different requirements. Why would an embedding allocator need to tune this? The actual amount of space (and hence the approximate number of frames) is already constant, all they can adjust is collecting fewer frames.
> That's a good point.
> 
> Maybe they should also be able to control the number of storage bytes as well?
Maybe in the future if we implement MESH and allocations pages don't dominate memory consumption, as it stands now the stack traces are a small part of the memory consumption so I would just leave it constant until a need arises. (This also has the side benefit of being able to replace the allocas with fixed-size arrays.)

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66189/new/

https://reviews.llvm.org/D66189