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

[Mitch Phillips via llvm-commits]

Author: hctim
Date: Tue Jun 25 15:29:05 2019
New Revision: 364356

URL: http://llvm.org/viewvc/llvm-project?rev=364356&view=rev
Log:
[GWP-ASan] Guard against recursive allocs. Pack TLS for perf.

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.

Reviewers: vlad.tsyrklevich, morehouse, eugenis

Reviewed By: eugenis

Subscribers: kubamracek, #sanitizers, llvm-commits, eugenis

Tags: #sanitizers, #llvm

Differential Revision: https://reviews.llvm.org/D63736

Modified:
    compiler-rt/trunk/lib/gwp_asan/guarded_pool_allocator.cpp
    compiler-rt/trunk/lib/gwp_asan/guarded_pool_allocator.h

Modified: compiler-rt/trunk/lib/gwp_asan/guarded_pool_allocator.cpp
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/gwp_asan/guarded_pool_allocator.cpp?rev=364356&r1=364355&r2=364356&view=diff
==============================================================================
--- compiler-rt/trunk/lib/gwp_asan/guarded_pool_allocator.cpp (original)
+++ compiler-rt/trunk/lib/gwp_asan/guarded_pool_allocator.cpp Tue Jun 25 15:29:05 2019
@@ -49,7 +49,9 @@ void GuardedPoolAllocator::AllocationMet
 
 void GuardedPoolAllocator::AllocationMetadata::RecordDeallocation() {
   IsDeallocated = true;
-  // TODO(hctim): Implement stack trace collection.
+  // TODO(hctim): Implement stack trace collection. Ensure that the unwinder is
+  // not called if we have our recursive flag called, otherwise non-reentrant
+  // unwinders may deadlock.
   DeallocationTrace.ThreadID = getThreadID();
 }
 
@@ -124,7 +126,28 @@ void GuardedPoolAllocator::init(const op
     installSignalHandlers();
 }
 
+namespace {
+class ScopedBoolean {
+public:
+  ScopedBoolean(bool &B) : Bool(B) { Bool = true; }
+  ~ScopedBoolean() { Bool = false; }
+
+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;
 
@@ -385,8 +408,7 @@ struct ScopedEndOfReportDecorator {
   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;
   }
@@ -395,6 +417,7 @@ void GuardedPoolAllocator::reportErrorIn
   // 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);
@@ -429,5 +452,7 @@ void GuardedPoolAllocator::reportErrorIn
   // 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

Modified: compiler-rt/trunk/lib/gwp_asan/guarded_pool_allocator.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/gwp_asan/guarded_pool_allocator.h?rev=364356&r1=364355&r2=364356&view=diff
==============================================================================
--- compiler-rt/trunk/lib/gwp_asan/guarded_pool_allocator.h (original)
+++ compiler-rt/trunk/lib/gwp_asan/guarded_pool_allocator.h Tue Jun 25 15:29:05 2019
@@ -96,14 +96,11 @@ public:
   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
@@ -245,9 +242,23 @@ private:
   // GWP-ASan is disabled, we wish to never spend wasted cycles recalculating
   // the sample rate.
   uint32_t AdjustedSampleRate = UINT32_MAX;
-  // Thread-local decrementing counter that indicates that a given allocation
-  // should be sampled when it reaches zero.
-  static TLS_INITIAL_EXEC uint64_t NextSampleCounter;
+
+  // 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 alignas(8) ThreadLocalPackedVariables {
+    constexpr ThreadLocalPackedVariables() {}
+    // Thread-local decrementing counter that indicates that a given allocation
+    // should be sampled when it reaches zero.
+    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