[PATCH] D89908: [GWP-ASan] Move random-related code in the allocator

[Mitch Phillips via Phabricator via llvm-commits]

hctim added inline comments.


================
Comment at: compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp:149
   if (State.GuardedPagePoolEnd == 0)
     return nullptr;
 
----------------
While we're here, can you also add `ThreadLocals.NextSampleCounter = AdjustedSampleRatePlusOne - 1;` to keep on the slow path for 2^31 more allocations after 2^31 have been done?


================
Comment at: compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp:157
     return nullptr;
+  ThreadLocals.RecursiveGuard = true;
 
----------------
I'd prefer if this could be a private class like `ScopedRecursiveGuard` inside GPA where the c'tor sets `ThreadLocals.RecursiveGuard = true;` and the d'tor sets `ThreadLocals.RecursiveGuard = false;`.


================
Comment at: compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp:284
 
+uint32_t GuardedPoolAllocator::getRandomUnsigned32() {
+  uint32_t RandomState = ThreadLocals.RandomState;
----------------
Originally the goal was to keep this PRNG as platform-specific implementation and just provide xorshift as a default (so if someone wanted their own proper PRNG they could provide it). After thinking, this is fine anyway, as if we want to provide platform-specific sampling then we want to allow other methods like 1-in-N-bytes and so will need a refactor anyway.


================
Comment at: compiler-rt/lib/gwp_asan/guarded_pool_allocator.h:222
+  // xorshift (32-bit output), extremely fast PRNG that uses arithmetic
+  // operations only. Seeded using walltime.
+  uint32_t getRandomUnsigned32();
----------------
nit: `Seeded using platform-specific mechanisms by initPRNG()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89908