[scudo] Remove dead code and other small cleanups. (#210454) Some dead code from the error info refactor is still around, so delete that. Also cleanup spelling and spacing. Modified the tests to use TestOnly functions so it's clear what is actually used. GitOrigin-RevId: f010423d3dfac774c6157686b6f14fb8d357c715
diff --git a/combined.h b/combined.h index 47f35a2..0652e46 100644 --- a/combined.h +++ b/combined.h
@@ -919,6 +919,7 @@ bool useMemoryTaggingTestOnly() const { return useMemoryTagging<AllocatorConfig>(Primary.Options.load()); } + void disableMemoryTagging() { // If we haven't been initialized yet, we need to initialize now in order to // prevent a future call to initThreadMaybe() from enabling memory tagging @@ -960,39 +961,6 @@ Primary.Options.clear(OptionBit::AddLargeAllocationSlack); } - const char *getStackDepotAddress() { - initThreadMaybe(); - AllocationRingBuffer *RB = getRingBuffer(); - return RB ? reinterpret_cast<char *>(RB->Depot) : nullptr; - } - - uptr getStackDepotSize() { - initThreadMaybe(); - AllocationRingBuffer *RB = getRingBuffer(); - return RB ? RB->StackDepotSize : 0; - } - - const char *getRegionInfoArrayAddress() const { - return Primary.getRegionInfoArrayAddress(); - } - - static uptr getRegionInfoArraySize() { - return PrimaryT::getRegionInfoArraySize(); - } - - const char *getRingBufferAddress() { - initThreadMaybe(); - return reinterpret_cast<char *>(getRingBuffer()); - } - - uptr getRingBufferSize() { - initThreadMaybe(); - AllocationRingBuffer *RB = getRingBuffer(); - return RB && RB->RingBufferElements - ? ringBufferSizeInBytes(RB->RingBufferElements) - : 0; - } - void getErrorInfo(uptr FaultAddr, size_t MinDistance, size_t MaxDistance, scudo_error_report *Reports, size_t &ReportIndex) { auto *RingBuffer = getRingBuffer(); @@ -1195,6 +1163,32 @@ return reinterpret_cast<uptr>(Begin); } + bool getRingBufferEnabledTestOnly() { + initThreadMaybe(); + return getRingBuffer() != nullptr; + } + + const void *getRingBufferAddressTestOnly() { + initThreadMaybe(); + return getRingBuffer(); + } + + bool getStackDepotEnabledTestOnly() { + initThreadMaybe(); + auto *RB = getRingBuffer(); + if (RB == nullptr) + return false; + return RB->Depot != nullptr; + } + + const void *getStackDepotAddressTestOnly() { + initThreadMaybe(); + auto *RB = getRingBuffer(); + if (RB == nullptr) + return nullptr; + return RB->Depot; + } + private: typedef typename PrimaryT::SizeClassMap SizeClassMap; @@ -1824,7 +1818,9 @@ MemMapT MemMap; MemMap.map( /*Addr=*/0U, - roundUp(ringBufferSizeInBytes(AllocationRingBufferSize), + roundUp(sizeof(AllocationRingBuffer) + + AllocationRingBufferSize * + sizeof(typename AllocationRingBuffer::Entry), getPageSizeCached()), "scudo:ring_buffer"); auto *RB = reinterpret_cast<AllocationRingBuffer *>(MemMap.getBase()); @@ -1852,19 +1848,6 @@ RawRingBufferMap.unmap(); atomic_store(&RingBufferAddress, 0, memory_order_release); } - - static constexpr size_t ringBufferSizeInBytes(u32 RingBufferElements) { - return sizeof(AllocationRingBuffer) + - RingBufferElements * sizeof(typename AllocationRingBuffer::Entry); - } - - static constexpr size_t ringBufferElementsFromBytes(size_t Bytes) { - if (Bytes < sizeof(AllocationRingBuffer)) { - return 0; - } - return (Bytes - sizeof(AllocationRingBuffer)) / - sizeof(typename AllocationRingBuffer::Entry); - } }; } // namespace scudo
diff --git a/mem_map_linux.cpp b/mem_map_linux.cpp index 2a5e8b5..d80284f 100644 --- a/mem_map_linux.cpp +++ b/mem_map_linux.cpp
@@ -128,7 +128,7 @@ while ((rc = madvise(Addr, Size, MADV_DONTNEED)) == -1 && errno == EAGAIN) { } if (rc == -1) { - // If we can't madvies the memory, then we still need to zero it. + // If we can't madvise the memory, then we still need to zero it. memset(Addr, 0, Size); } }
diff --git a/primary32.h b/primary32.h index c3705a2..0dbb780 100644 --- a/primary32.h +++ b/primary32.h
@@ -131,18 +131,9 @@ uptr tryReleaseToOS(uptr ClassId, ReleaseToOS ReleaseType); uptr releaseToOS(ReleaseToOS ReleaseType); - const char *getRegionInfoArrayAddress() const { return nullptr; } - static uptr getRegionInfoArraySize() { return 0; } - // Not supported in SizeClassAllocator32. BlockInfo findNearestBlock(UNUSED uptr Ptr) { return {}; } - // Not supported in SizeClassAllocator32. - static BlockInfo findNearestBlock(UNUSED const char *RegionInfoData, - UNUSED uptr Ptr) { - return {}; - } - AtomicOptions Options; private:
diff --git a/primary64.h b/primary64.h index b8ff156..f85401d 100644 --- a/primary64.h +++ b/primary64.h
@@ -89,9 +89,6 @@ static bool conditionVariableEnabled() { return Config::hasConditionVariableT(); } - static uptr getRegionInfoArraySize() { return sizeof(RegionInfoArray); } - static BlockInfo findNearestBlock(const char *RegionInfoData, - uptr Ptr) NO_THREAD_SAFETY_ANALYSIS; BlockInfo findNearestBlock(uptr Ptr); @@ -125,17 +122,15 @@ uptr tryReleaseToOS(uptr ClassId, ReleaseToOS ReleaseType); uptr releaseToOS(ReleaseToOS ReleaseType); - const char *getRegionInfoArrayAddress() const { - return reinterpret_cast<const char *>(RegionInfoArray); - } - uptr getCompactPtrBaseByClassId(uptr ClassId) { return getRegionInfo(ClassId)->RegionBeg; } + CompactPtrT compactPtr(uptr ClassId, uptr Ptr) { DCHECK_LE(ClassId, SizeClassMap::LargestClassId); return compactPtrInternal(getCompactPtrBaseByClassId(ClassId), Ptr); } + void *decompactPtr(uptr ClassId, CompactPtrT CompactPtr) { DCHECK_LE(ClassId, SizeClassMap::LargestClassId); return reinterpret_cast<void *>( @@ -1407,59 +1402,6 @@ } template <typename Config> -/* static */ BlockInfo SizeClassAllocator64<Config>::findNearestBlock( - const char *RegionInfoData, uptr Ptr) NO_THREAD_SAFETY_ANALYSIS { - const RegionInfo *RegionInfoArray = - reinterpret_cast<const RegionInfo *>(RegionInfoData); - - uptr ClassId; - uptr MinDistance = -1UL; - for (uptr I = 0; I != NumClasses; ++I) { - if (I == SizeClassMap::BatchClassId) - continue; - uptr Begin = RegionInfoArray[I].RegionBeg; - // TODO(chiahungduan): In fact, We need to lock the RegionInfo::MMLock. - // However, the RegionInfoData is passed with const qualifier and lock the - // mutex requires modifying RegionInfoData, which means we need to remove - // the const qualifier. This may lead to another undefined behavior (The - // first one is accessing `AllocatedUser` without locking. It's better to - // pass `RegionInfoData` as `void *` then we can lock the mutex properly. - uptr End = Begin + RegionInfoArray[I].MemMapInfo.AllocatedUser; - if (Begin > End || End - Begin < SizeClassMap::getSizeByClassId(I)) - continue; - uptr RegionDistance; - if (Begin <= Ptr) { - if (Ptr < End) - RegionDistance = 0; - else - RegionDistance = Ptr - End; - } else { - RegionDistance = Begin - Ptr; - } - - if (RegionDistance < MinDistance) { - MinDistance = RegionDistance; - ClassId = I; - } - } - - BlockInfo B = {}; - if (MinDistance <= 8192) { - B.RegionBegin = RegionInfoArray[ClassId].RegionBeg; - B.RegionEnd = - B.RegionBegin + RegionInfoArray[ClassId].MemMapInfo.AllocatedUser; - B.BlockSize = SizeClassMap::getSizeByClassId(ClassId); - B.BlockBegin = B.RegionBegin + uptr(sptr(Ptr - B.RegionBegin) / - sptr(B.BlockSize) * sptr(B.BlockSize)); - while (B.BlockBegin < B.RegionBegin) - B.BlockBegin += B.BlockSize; - while (B.RegionEnd < B.BlockBegin + B.BlockSize) - B.BlockBegin -= B.BlockSize; - } - return B; -} - -template <typename Config> uptr SizeClassAllocator64<Config>::releaseToOSMaybe(RegionInfo *Region, uptr ClassId, ReleaseToOS ReleaseType)
diff --git a/tests/combined_test.cpp b/tests/combined_test.cpp index 4116cf8..a35113e 100644 --- a/tests/combined_test.cpp +++ b/tests/combined_test.cpp
@@ -969,82 +969,49 @@ // 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()); + EXPECT_FALSE(Allocator->getRingBufferEnabledTestOnly()); } 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_TRUE(Allocator->getRingBufferEnabledTestOnly()); + const void *RingBufferAddress = Allocator->getRingBufferAddressTestOnly(); 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()); + EXPECT_TRUE(Allocator->getRingBufferEnabledTestOnly()); + EXPECT_EQ(RingBufferAddress, Allocator->getRingBufferAddressTestOnly()); } -SCUDO_TYPED_TEST(ScudoCombinedTest, RingBufferSize) { +SCUDO_TYPED_TEST(ScudoCombinedTest, RingBufferEnabled) { auto *Allocator = this->Allocator.get(); Allocator->setTrackAllocationStacks(true); - auto RingBufferSize = Allocator->getRingBufferSize(); - ASSERT_GT(RingBufferSize, 0u); - EXPECT_EQ(Allocator->getRingBufferAddress()[RingBufferSize - 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()); + EXPECT_TRUE(Allocator->getRingBufferEnabledTestOnly()); } 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()); + EXPECT_FALSE(Allocator->getStackDepotEnabledTestOnly()); } 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_TRUE(Allocator->getStackDepotEnabledTestOnly()); + const void *StackDepotAddress = Allocator->getStackDepotAddressTestOnly(); 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()); -} - -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'); -} - -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()); + EXPECT_TRUE(Allocator->getStackDepotEnabledTestOnly()); + EXPECT_EQ(StackDepotAddress, Allocator->getStackDepotAddressTestOnly()); } SCUDO_TYPED_TEST(ScudoCombinedTest, StackDepot) {
diff --git a/tests/error_info_test.cpp b/tests/error_info_test.cpp index d8d4dc4..2129d09 100644 --- a/tests/error_info_test.cpp +++ b/tests/error_info_test.cpp
@@ -49,8 +49,7 @@ GTEST_SKIP() << "MTE not supported or enabled"; } - EXPECT_GT(Allocator->getRingBufferSize(), 0u); - EXPECT_NE(nullptr, Allocator->getRingBufferAddress()); + EXPECT_TRUE(Allocator->getRingBufferEnabledTestOnly()); const scudo::uptr Size = 64U; void *P = nullptr;