[compiler-rt] Revert "[scudo] Only init RingBuffer when needed." (PR #91256)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Florian Mayer (fmayer)

<details>
<summary>Changes</summary>

Reverts llvm/llvm-project#<!-- -->85994

This broke Android because we don't assign the lazily-initialized ring buffer to the libc globals

---
Full diff: https://github.com/llvm/llvm-project/pull/91256.diff


2 Files Affected:

- (modified) compiler-rt/lib/scudo/standalone/combined.h (+10-21) 
- (modified) compiler-rt/lib/scudo/standalone/tests/combined_test.cpp (+12-66) 


``````````diff
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index 927513dea92dab..f59295382136bc 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -18,7 +18,6 @@
 #include "local_cache.h"
 #include "mem_map.h"
 #include "memtag.h"
-#include "mutex.h"
 #include "options.h"
 #include "quarantine.h"
 #include "report.h"
@@ -182,17 +181,17 @@ class Allocator {
     Quarantine.init(
         static_cast<uptr>(getFlags()->quarantine_size_kb << 10),
         static_cast<uptr>(getFlags()->thread_local_quarantine_size_kb << 10));
+
+    mapAndInitializeRingBuffer();
   }
 
-  void enableRingBuffer() NO_THREAD_SAFETY_ANALYSIS {
+  void enableRingBuffer() {
     AllocationRingBuffer *RB = getRingBuffer();
     if (RB)
       RB->Depot->enable();
-    RingBufferInitLock.unlock();
   }
 
-  void disableRingBuffer() NO_THREAD_SAFETY_ANALYSIS {
-    RingBufferInitLock.lock();
+  void disableRingBuffer() {
     AllocationRingBuffer *RB = getRingBuffer();
     if (RB)
       RB->Depot->disable();
@@ -919,11 +918,9 @@ class Allocator {
       DCHECK(!Primary.Options.load().get(OptionBit::TrackAllocationStacks));
       return;
     }
-
-    if (Track) {
-      initRingBufferMaybe();
+    if (Track)
       Primary.Options.set(OptionBit::TrackAllocationStacks);
-    } else
+    else
       Primary.Options.clear(OptionBit::TrackAllocationStacks);
   }
 
@@ -1098,9 +1095,6 @@ class Allocator {
                     0,
                 "invalid alignment");
 
-  // Lock to initialize the RingBuffer
-  HybridMutex RingBufferInitLock;
-
   // Pointer to memory mapped area starting with AllocationRingBuffer struct,
   // and immediately followed by Size elements of type Entry.
   atomic_uptr RingBufferAddress = {};
@@ -1555,16 +1549,11 @@ class Allocator {
         RBEntryStart)[N];
   }
 
-  void initRingBufferMaybe() {
-    ScopedLock L(RingBufferInitLock);
-    if (getRingBuffer() != nullptr)
+  void mapAndInitializeRingBuffer() {
+    if (getFlags()->allocation_ring_buffer_size <= 0)
       return;
-
-    int ring_buffer_size = getFlags()->allocation_ring_buffer_size;
-    if (ring_buffer_size <= 0)
-      return;
-
-    u32 AllocationRingBufferSize = static_cast<u32>(ring_buffer_size);
+    u32 AllocationRingBufferSize =
+        static_cast<u32>(getFlags()->allocation_ring_buffer_size);
 
     // We store alloc and free stacks for each entry.
     constexpr u32 kStacksPerRingBufferEntry = 2;
diff --git a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
index 1a36155bcd423a..6a311adc55e4bd 100644
--- a/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
@@ -867,86 +867,32 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, ReallocateInPlaceStress) {
   }
 }
 
-SCUDO_TYPED_TEST(ScudoCombinedTest, RingBufferDefaultDisabled) {
-  // The RingBuffer is not initialized until tracking is enabled for the
-  // first time.
-  auto *Allocator = this->Allocator.get();
-  EXPECT_EQ(0u, Allocator->getRingBufferSize());
-  EXPECT_EQ(nullptr, Allocator->getRingBufferAddress());
-}
-
-SCUDO_TYPED_TEST(ScudoCombinedTest, RingBufferInitOnce) {
-  auto *Allocator = this->Allocator.get();
-  Allocator->setTrackAllocationStacks(true);
-
-  auto RingBufferSize = Allocator->getRingBufferSize();
-  ASSERT_GT(RingBufferSize, 0u);
-  auto *RingBufferAddress = Allocator->getRingBufferAddress();
-  EXPECT_NE(nullptr, RingBufferAddress);
-
-  // Enable tracking again to verify that the initialization only happens once.
-  Allocator->setTrackAllocationStacks(true);
-  ASSERT_EQ(RingBufferSize, Allocator->getRingBufferSize());
-  EXPECT_EQ(RingBufferAddress, Allocator->getRingBufferAddress());
-}
-
 SCUDO_TYPED_TEST(ScudoCombinedTest, RingBufferSize) {
   auto *Allocator = this->Allocator.get();
-  Allocator->setTrackAllocationStacks(true);
-
-  auto RingBufferSize = Allocator->getRingBufferSize();
-  ASSERT_GT(RingBufferSize, 0u);
-  EXPECT_EQ(Allocator->getRingBufferAddress()[RingBufferSize - 1], '\0');
+  auto Size = Allocator->getRingBufferSize();
+  ASSERT_GT(Size, 0u);
+  EXPECT_EQ(Allocator->getRingBufferAddress()[Size - 1], '\0');
 }
 
 SCUDO_TYPED_TEST(ScudoCombinedTest, RingBufferAddress) {
   auto *Allocator = this->Allocator.get();
-  Allocator->setTrackAllocationStacks(true);
-
-  auto *RingBufferAddress = Allocator->getRingBufferAddress();
-  EXPECT_NE(RingBufferAddress, nullptr);
-  EXPECT_EQ(RingBufferAddress, Allocator->getRingBufferAddress());
-}
-
-SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepotDefaultDisabled) {
-  // The StackDepot is not initialized until tracking is enabled for the
-  // first time.
-  auto *Allocator = this->Allocator.get();
-  EXPECT_EQ(0u, Allocator->getStackDepotSize());
-  EXPECT_EQ(nullptr, Allocator->getStackDepotAddress());
-}
-
-SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepotInitOnce) {
-  auto *Allocator = this->Allocator.get();
-  Allocator->setTrackAllocationStacks(true);
-
-  auto StackDepotSize = Allocator->getStackDepotSize();
-  EXPECT_GT(StackDepotSize, 0u);
-  auto *StackDepotAddress = Allocator->getStackDepotAddress();
-  EXPECT_NE(nullptr, StackDepotAddress);
-
-  // Enable tracking again to verify that the initialization only happens once.
-  Allocator->setTrackAllocationStacks(true);
-  EXPECT_EQ(StackDepotSize, Allocator->getStackDepotSize());
-  EXPECT_EQ(StackDepotAddress, Allocator->getStackDepotAddress());
+  auto *Addr = Allocator->getRingBufferAddress();
+  EXPECT_NE(Addr, nullptr);
+  EXPECT_EQ(Addr, Allocator->getRingBufferAddress());
 }
 
 SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepotSize) {
   auto *Allocator = this->Allocator.get();
-  Allocator->setTrackAllocationStacks(true);
-
-  auto StackDepotSize = Allocator->getStackDepotSize();
-  EXPECT_GT(StackDepotSize, 0u);
-  EXPECT_EQ(Allocator->getStackDepotAddress()[StackDepotSize - 1], '\0');
+  auto Size = Allocator->getStackDepotSize();
+  ASSERT_GT(Size, 0u);
+  EXPECT_EQ(Allocator->getStackDepotAddress()[Size - 1], '\0');
 }
 
 SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepotAddress) {
   auto *Allocator = this->Allocator.get();
-  Allocator->setTrackAllocationStacks(true);
-
-  auto *StackDepotAddress = Allocator->getStackDepotAddress();
-  EXPECT_NE(StackDepotAddress, nullptr);
-  EXPECT_EQ(StackDepotAddress, Allocator->getStackDepotAddress());
+  auto *Addr = Allocator->getStackDepotAddress();
+  EXPECT_NE(Addr, nullptr);
+  EXPECT_EQ(Addr, Allocator->getStackDepotAddress());
 }
 
 SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepot) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/91256