[PATCH] D60593: [GwpAsan] Introduce GWP-ASan.
Matt Morehouse via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 8 09:08:01 PDT 2019
morehouse added inline comments.
================
Comment at: compiler-rt/lib/gwp_asan/guarded_pool_allocator.h:97
+ // Return whether the allocation should be randomly chosen for sampling.
+ ALWAYS_INLINE bool shouldSample() {
+ // NextSampleCounter == 0 means we "should regenerate the counter".
----------------
The assembly you posted for this function looks way too slow. Is it actually optimized with `-O2`?
- The fast path (counter > 1) has *two* branches that check if `NextSampleCounter`'s TLS init function has run and one actual call to the TLS init function. Ouch! We shouldn't need these.
- Maybe we need to make the counter function-local or use `__thread` instead of `thread_local`.
- The counter decrement of `NextSampleCounter` requires 4 instructions? Load TLS offset from global, read TLS, increment, write TLS. Yikes. For reference, TCMalloc does this operation in 1 instruction.
- The `+ 1` operation on the `NextSampleCounter == 0` branch is done with a mov-add, rather than an lea.
Again, please triple-check that the binary you compiled is actually optimized. Make sure it is compiled with `clang++ -O2 ...`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60593/new/
https://reviews.llvm.org/D60593
More information about the llvm-commits
mailing list