[GWP-ASan] Guard against recursive allocs. Pack TLS for perf.

[hctim]

Summary

Add a recursivity guard for GPA::allocate(). This means that any
recursive allocations will fall back to the supporting allocator. In future
patches, we will introduce stack trace collection support. The unwinder will be
provided by the supporting allocator, and we can't guarantee they don't call
malloc() (e.g. backtrace() on posix may call dlopen(), which may call malloc().

Furthermore, this patch packs the new TLS recursivity guard into a thread local
struct, so that TLS variables should be hopefully not fall across cache lines.

[hctim]

• /s/reentrant/recursive

[eugenis]

compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp

131

Set to true in constructor, false in destructor. This !Bool stuff makes the code harder to read for no reason at all.

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

249

alignas(8), at least

[hctim]

• Add alignment and explicitly set ScopedBoolean values.

[eugenis]

Do we need similar protection in deallocate?

[hctim]

In D63736#1556349, @eugenis wrote:

Do we need similar protection in deallocate?

We shouldn't need to. GPA::deallocate() may call malloc() and free(), but can only be called with guarded allocations. Recursion is okay (and desirable, as the unwinder's implementation is okay to be sampled) in deallocate().

[vlad.tsyrklevich]

In D63736#1556463, @hctim wrote:

In D63736#1556349, @eugenis wrote:

Do we need similar protection in deallocate?

We shouldn't need to. GPA::deallocate() may call malloc() and free(), but can only be called with guarded allocations. Recursion is okay (and desirable, as the unwinder's implementation is okay to be sampled) in deallocate().

Don't we hold the Mutex during AllocationMetadata::RecordDeallocation()? Doesn't that mean that the unwinder could cause deadlock during deallocation?

[eugenis]

In D63736#1556524, @vlad.tsyrklevich wrote:

In D63736#1556463, @hctim wrote:

In D63736#1556349, @eugenis wrote:

Do we need similar protection in deallocate?

We shouldn't need to. GPA::deallocate() may call malloc() and free(), but can only be called with guarded allocations. Recursion is okay (and desirable, as the unwinder's implementation is okay to be sampled) in deallocate().

Don't we hold the Mutex during AllocationMetadata::RecordDeallocation()? Doesn't that mean that the unwinder could cause deadlock during deallocation?

We can not assume that the unwinder is reentrant, so we need to guard all unwinder calls. We can not really fallback to the system allocator in free(), which means that the rest of the deallocation code will have to be reentrant.

[hctim]

In D63736#1556534, @eugenis wrote:

We can not assume that the unwinder is reentrant, so we need to guard all unwinder calls. We can not really fallback to the system allocator in free(), which means that the rest of the deallocation code will have to be reentrant.

So in summary:

• The unwinder can call any combination of malloc(), free(), in any way/shape/form it wants. The unwinder is assumed to not be reentrant.
• GPA::allocate() will automatically fallback to the default allocator if it detects reentrant behaviour.
• GPA::deallocate() has no way to fallback (as it must deallocate any guarded allocations). In this case, we have to avoid reentrant calls to deallocate(), and so we never call the unwinder recursively.

SGTY?

Also, we don't want to place the reentrant-guard around the entirety of RecordDeallocation() (as then the slot won't be marked as deallocated in the metadata). The ScopedBool has to wrap the call to the unwinder only, and therefore must be in a follow up patch.

[hctim]

• Added note for future implementation to recursive-guard the unwinder on dealloc.

[eugenis]

LGTM

compiler-rt/lib/gwp_asan/guarded_pool_allocator.h

public:
   // pool using the provided options. See options.inc for runtime configuration
   // options.
   void init(const options::Options &Opts);
 
   // Return whether the allocation should be randomly chosen for sampling.
   ALWAYS_INLINE bool shouldSample() {
     // NextSampleCounter == 0 means we "should regenerate the counter".
     //                   == 1 means we "should sample this allocation".
-    if (UNLIKELY(NextSampleCounter == 0)) {
-      // GuardedPagePoolEnd == 0 if GWP-ASan is disabled.
-      if (UNLIKELY(GuardedPagePoolEnd == 0))
-        return false;
-      NextSampleCounter = (getRandomUnsigned32() % AdjustedSampleRate) + 1;
-    }
+    if (UNLIKELY(ThreadLocals.NextSampleCounter == 0))
+      ThreadLocals.NextSampleCounter =
+          (getRandomUnsigned32() % AdjustedSampleRate) + 1;
 
-    return UNLIKELY(--NextSampleCounter == 0);
+    return UNLIKELY(--ThreadLocals.NextSampleCounter == 0);
   }
 
   // Returns whether the provided pointer is a current sampled allocation that
   // is owned by this pool.
   ALWAYS_INLINE bool pointerIsMine(const void *Ptr) const {
     uintptr_t P = reinterpret_cast<uintptr_t>(Ptr);
     return GuardedPagePool <= P && P < GuardedPagePoolEnd;
   }
private:
 
   // The adjusted sample rate for allocation sampling. Default *must* be
   // nonzero, as dynamic initialisation may call malloc (e.g. from libstdc++)
   // before GPA::init() is called. This would cause an error in shouldSample(),
   // where we would calculate modulo zero. This value is set UINT32_MAX, as when
   // GWP-ASan is disabled, we wish to never spend wasted cycles recalculating
   // the sample rate.
   uint32_t AdjustedSampleRate = UINT32_MAX;
+
+  // Pack the thread local variables into a struct to ensure that they're in
+  // the same cache line for performance reasons. These are the most touched
+  // variables in GWP-ASan.
+  struct ThreadLocalPackedVariables {
+    constexpr ThreadLocalPackedVariables() {}
     // Thread-local decrementing counter that indicates that a given allocation
     // should be sampled when it reaches zero.
-  static TLS_INITIAL_EXEC uint64_t NextSampleCounter;
+    uint32_t NextSampleCounter = 0;
+    // Guard against recursivity. Unwinders often contain complex behaviour that
+    // may not be safe for the allocator (i.e. the unwinder calls dlopen(),
+    // which calls malloc()). When recursive behaviour is detected, we will
+    // automatically fall back to the supporting allocator to supply the
+    // allocation.
+    bool RecursiveGuard = false;
+  };
+  static TLS_INITIAL_EXEC ThreadLocalPackedVariables ThreadLocals;
 };
 } // namespace gwp_asan
 
 #endif // GWP_ASAN_GUARDED_POOL_ALLOCATOR_H_

compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp

void GuardedPoolAllocator::init(const options::Options &Opts) {
 
   // Ensure that signal handlers are installed as late as possible, as the class
   // is not thread-safe until init() is finished, and thus a SIGSEGV may cause a
   // race to members if recieved during init().
   if (Opts.InstallSignalHandlers)
     installSignalHandlers();
 }
 
+namespace {
+class ScopedBoolean {
+public:
+  ScopedBoolean(bool &B) : Bool(B) { Bool = !Bool; }
+  ~ScopedBoolean() { Bool = !Bool; }
+
+private:
+  bool &Bool;
+};
+} // anonymous namespace
+
 void *GuardedPoolAllocator::allocate(size_t Size) {
+  // GuardedPagePoolEnd == 0 when GWP-ASan is disabled. If we are disabled, fall
+  // back to the supporting allocator.
+  if (GuardedPagePoolEnd == 0)
+    return nullptr;
+
+  // Protect against recursivity.
+  if (ThreadLocals.RecursiveGuard)
+    return nullptr;
+  ScopedBoolean SB(ThreadLocals.RecursiveGuard);
+
   if (Size == 0 || Size > maximumAllocationSize())
     return nullptr;
 
   size_t Index;
   {
     ScopedLock L(PoolMutex);
     Index = reserveSlot();
   }

 }
 
 struct ScopedEndOfReportDecorator {
   ScopedEndOfReportDecorator(options::Printf_t Printf) : Printf(Printf) {}
   ~ScopedEndOfReportDecorator() { Printf("*** End GWP-ASan report ***\n"); }
   options::Printf_t Printf;
 };
 
-void GuardedPoolAllocator::reportErrorInternal(uintptr_t AccessPtr,
-                                               Error E) {
+void GuardedPoolAllocator::reportErrorInternal(uintptr_t AccessPtr, Error E) {
   if (!pointerIsMine(reinterpret_cast<void *>(AccessPtr))) {
     return;
   }
 
   // Attempt to prevent races to re-use the same slot that triggered this error.
   // This does not guarantee that there are no races, because another thread can
   // take the locks during the time that the signal handler is being called.
   PoolMutex.tryLock();
+  ThreadLocals.RecursiveGuard = true;
 
   Printf("*** GWP-ASan detected a memory error ***\n");
   ScopedEndOfReportDecorator Decorator(Printf);
 
   AllocationMetadata *Meta = nullptr;
 
   if (E == Error::UNKNOWN) {
     E = diagnoseUnknownError(AccessPtr, &Meta);
void GuardedPoolAllocator::reportErrorInternal(uintptr_t AccessPtr, Error E) {
 
   printThreadInformation(E, AccessPtr, Meta, Printf);
   // TODO(hctim): Implement stack unwinding here. Ask the caller to provide us
   // with the base pointer, and we unwind the stack to give a stack trace for
   // the access.
   // TODO(hctim): Implement dumping here of allocation/deallocation traces.
 }
 
-TLS_INITIAL_EXEC uint64_t GuardedPoolAllocator::NextSampleCounter = 0;
+TLS_INITIAL_EXEC
+GuardedPoolAllocator::ThreadLocalPackedVariables
+    GuardedPoolAllocator::ThreadLocals;
 } // namespace gwp_asan