From df1fd395bed1c87151e331d7f716a76df3ba4046 Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Mon, 6 May 2024 12:00:07 -0700 Subject: Revert "[scudo] Only init RingBuffer when needed. (#85994)" This reverts commit 0dbd804a690720688d8234d8bdaee8f8f4fdcddc. --- compiler-rt/lib/scudo/standalone/combined.h | 31 +++------ .../lib/scudo/standalone/tests/combined_test.cpp | 78 ++++------------------ 2 files changed, 22 insertions(+), 87 deletions(-) diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h index 927513d..f592953 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 @@ public: Quarantine.init( static_cast(getFlags()->quarantine_size_kb << 10), static_cast(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 @@ public: 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 @@ private: 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 @@ private: 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(ring_buffer_size); + u32 AllocationRingBufferSize = + static_cast(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 1a36155..6a311ad 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) { -- cgit v1.1