[PATCH] D60593: [GwpAsan] Introduce GWP-ASan.

[Matt Morehouse via Phabricator via llvm-commits]

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