[compiler-rt] [scudo] Use MemMap in BufferPool and RegionPageMap (PR #66788)
Fabio D'Urso via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 19 09:13:29 PDT 2023
https://github.com/fabio-d created https://github.com/llvm/llvm-project/pull/66788
None
>From ce1fdaf1fa1784389c6b1e8df6a056a5ce7cafc2 Mon Sep 17 00:00:00 2001
From: Fabio D'Urso <fdurso at google.com>
Date: Tue, 19 Sep 2023 17:48:24 +0200
Subject: [PATCH] [scudo] Use MemMap in BufferPool and RegionPageMap
---
compiler-rt/lib/scudo/standalone/release.h | 115 +++++++++---------
.../scudo/standalone/tests/release_test.cpp | 23 ++--
2 files changed, 66 insertions(+), 72 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/release.h b/compiler-rt/lib/scudo/standalone/release.h
index e138570e7993336..5916b094f98c4ee 100644
--- a/compiler-rt/lib/scudo/standalone/release.h
+++ b/compiler-rt/lib/scudo/standalone/release.h
@@ -105,8 +105,17 @@ template <uptr StaticBufferCount, uptr StaticBufferSize> class BufferPool {
static_assert(StaticBufferCount < SCUDO_WORDSIZE, "");
static_assert(isAligned(StaticBufferSize, SCUDO_CACHE_LINE_SIZE), "");
- // Return a buffer which is at least `BufferSize`.
- uptr *getBuffer(const uptr BufferSize) {
+ struct Buffer {
+ // Pointer to the buffer's memory, or nullptr if no buffer was allocated.
+ uptr *Data = nullptr;
+
+ uptr BufferIndex = ~static_cast<uptr>(0) /* poison to aid debugging */;
+ MemMapT MemMap = {}; // only valid if BufferIndex == StaticBufferCount
+ };
+
+ // Return a zero-initialized buffer which can contain at least `BufferSize`
+ // uptr elements.
+ Buffer getBuffer(const uptr BufferSize) {
if (UNLIKELY(BufferSize > StaticBufferSize))
return getDynamicBuffer(BufferSize);
@@ -125,67 +134,52 @@ template <uptr StaticBufferCount, uptr StaticBufferSize> class BufferPool {
if (index >= StaticBufferCount)
return getDynamicBuffer(BufferSize);
- const uptr Offset = index * StaticBufferSize;
- memset(&RawBuffer[Offset], 0, StaticBufferSize);
- return &RawBuffer[Offset];
+ Buffer Buf;
+ Buf.Data = &RawBuffer[index * StaticBufferSize];
+ Buf.BufferIndex = index;
+ memset(Buf.Data, 0, StaticBufferSize * sizeof(uptr));
+ return Buf;
}
- void releaseBuffer(uptr *Buffer, const uptr BufferSize) {
- const uptr index = getStaticBufferIndex(Buffer, BufferSize);
- if (index < StaticBufferCount) {
+ void releaseBuffer(Buffer Buf) {
+ DCHECK(Buf.Data != nullptr);
+ DCHECK_LE(Buf.BufferIndex, StaticBufferCount);
+ if (Buf.BufferIndex != StaticBufferCount) {
ScopedLock L(Mutex);
- DCHECK_EQ((Mask & (static_cast<uptr>(1) << index)), 0U);
- Mask |= static_cast<uptr>(1) << index;
+ DCHECK_EQ((Mask & (static_cast<uptr>(1) << Buf.BufferIndex)), 0U);
+ Mask |= static_cast<uptr>(1) << Buf.BufferIndex;
} else {
- unmap(reinterpret_cast<void *>(Buffer),
- roundUp(BufferSize, getPageSizeCached()));
+ Buf.MemMap.unmap(Buf.MemMap.getBase(), Buf.MemMap.getCapacity());
}
}
- bool isStaticBufferTestOnly(uptr *Buffer, uptr BufferSize) {
- return getStaticBufferIndex(Buffer, BufferSize) < StaticBufferCount;
+ bool isStaticBufferTestOnly(const Buffer &Buf) {
+ DCHECK(Buf.Data != nullptr);
+ DCHECK_LE(Buf.BufferIndex, StaticBufferCount);
+ return Buf.BufferIndex != StaticBufferCount;
}
private:
- uptr getStaticBufferIndex(uptr *Buffer, uptr BufferSize) {
- if (UNLIKELY(BufferSize > StaticBufferSize))
- return StaticBufferCount;
-
- const uptr BufferBase = reinterpret_cast<uptr>(Buffer);
- const uptr RawBufferBase = reinterpret_cast<uptr>(RawBuffer);
-
- if (BufferBase < RawBufferBase ||
- BufferBase >= RawBufferBase + sizeof(RawBuffer)) {
- return StaticBufferCount;
- }
-
- DCHECK_LE(BufferSize, StaticBufferSize);
- DCHECK_LE(BufferBase + BufferSize, RawBufferBase + sizeof(RawBuffer));
- DCHECK_EQ((BufferBase - RawBufferBase) % StaticBufferSize, 0U);
-
- const uptr index =
- (BufferBase - RawBufferBase) / (StaticBufferSize * sizeof(uptr));
- DCHECK_LT(index, StaticBufferCount);
- return index;
- }
-
- uptr *getDynamicBuffer(const uptr BufferSize) {
+ Buffer getDynamicBuffer(const uptr BufferSize) {
// When using a heap-based buffer, precommit the pages backing the
// Vmar by passing |MAP_PRECOMMIT| flag. This allows an optimization
// where page fault exceptions are skipped as the allocated memory
// is accessed. So far, this is only enabled on Fuchsia. It hasn't proven a
// performance benefit on other platforms.
const uptr MmapFlags = MAP_ALLOWNOMEM | (SCUDO_FUCHSIA ? MAP_PRECOMMIT : 0);
- return reinterpret_cast<uptr *>(
- map(nullptr, roundUp(BufferSize, getPageSizeCached()), "scudo:counters",
- MmapFlags, &MapData));
+ const uptr Size = roundUp(BufferSize * sizeof(uptr), getPageSizeCached());
+ Buffer Buf;
+ if (Buf.MemMap.map(/*Addr=*/0, Size, "scudo:counters", MmapFlags)) {
+ Buf.Data = reinterpret_cast<uptr *>(Buf.MemMap.getBase());
+ Buf.BufferIndex = StaticBufferCount;
+ }
+ return Buf;
}
HybridMutex Mutex;
// '1' means that buffer index is not used. '0' means the buffer is in use.
uptr Mask GUARDED_BY(Mutex) = ~static_cast<uptr>(0);
uptr RawBuffer[StaticBufferCount * StaticBufferSize] GUARDED_BY(Mutex);
- [[no_unique_address]] MapPlatformData MapData = {};
};
// A Region page map is used to record the usage of pages in the regions. It
@@ -207,16 +201,15 @@ class RegionPageMap {
PackingRatioLog(0),
BitOffsetMask(0),
SizePerRegion(0),
- BufferSize(0),
- Buffer(nullptr) {}
+ BufferSize(0) {}
RegionPageMap(uptr NumberOfRegions, uptr CountersPerRegion, uptr MaxValue) {
reset(NumberOfRegions, CountersPerRegion, MaxValue);
}
~RegionPageMap() {
if (!isAllocated())
return;
- Buffers.releaseBuffer(Buffer, BufferSize);
- Buffer = nullptr;
+ Buffers.releaseBuffer(Buffer);
+ Buffer = {};
}
// Lock of `StaticBuffer` is acquired conditionally and there's no easy way to
@@ -231,7 +224,7 @@ class RegionPageMap {
Regions = NumberOfRegion;
NumCounters = CountersPerRegion;
- constexpr uptr MaxCounterBits = sizeof(*Buffer) * 8UL;
+ constexpr uptr MaxCounterBits = sizeof(*Buffer.Data) * 8UL;
// Rounding counter storage size up to the power of two allows for using
// bit shifts calculating particular counter's Index and offset.
const uptr CounterSizeBits =
@@ -248,11 +241,11 @@ class RegionPageMap {
SizePerRegion =
roundUp(NumCounters, static_cast<uptr>(1U) << PackingRatioLog) >>
PackingRatioLog;
- BufferSize = SizePerRegion * sizeof(*Buffer) * Regions;
+ BufferSize = SizePerRegion * Regions;
Buffer = Buffers.getBuffer(BufferSize);
}
- bool isAllocated() const { return !!Buffer; }
+ bool isAllocated() const { return Buffer.Data != nullptr; }
uptr getCount() const { return NumCounters; }
@@ -261,7 +254,8 @@ class RegionPageMap {
DCHECK_LT(I, NumCounters);
const uptr Index = I >> PackingRatioLog;
const uptr BitOffset = (I & BitOffsetMask) << CounterSizeBitsLog;
- return (Buffer[Region * SizePerRegion + Index] >> BitOffset) & CounterMask;
+ return (Buffer.Data[Region * SizePerRegion + Index] >> BitOffset) &
+ CounterMask;
}
void inc(uptr Region, uptr I) const {
@@ -270,8 +264,8 @@ class RegionPageMap {
const uptr BitOffset = (I & BitOffsetMask) << CounterSizeBitsLog;
DCHECK_LT(BitOffset, SCUDO_WORDSIZE);
DCHECK_EQ(isAllCounted(Region, I), false);
- Buffer[Region * SizePerRegion + Index] += static_cast<uptr>(1U)
- << BitOffset;
+ Buffer.Data[Region * SizePerRegion + Index] += static_cast<uptr>(1U)
+ << BitOffset;
}
void incN(uptr Region, uptr I, uptr N) const {
@@ -282,7 +276,7 @@ class RegionPageMap {
const uptr BitOffset = (I & BitOffsetMask) << CounterSizeBitsLog;
DCHECK_LT(BitOffset, SCUDO_WORDSIZE);
DCHECK_EQ(isAllCounted(Region, I), false);
- Buffer[Region * SizePerRegion + Index] += N << BitOffset;
+ Buffer.Data[Region * SizePerRegion + Index] += N << BitOffset;
}
void incRange(uptr Region, uptr From, uptr To) const {
@@ -301,7 +295,7 @@ class RegionPageMap {
const uptr Index = I >> PackingRatioLog;
const uptr BitOffset = (I & BitOffsetMask) << CounterSizeBitsLog;
DCHECK_LT(BitOffset, SCUDO_WORDSIZE);
- Buffer[Region * SizePerRegion + Index] |= CounterMask << BitOffset;
+ Buffer.Data[Region * SizePerRegion + Index] |= CounterMask << BitOffset;
}
void setAsAllCountedRange(uptr Region, uptr From, uptr To) const {
DCHECK_LE(From, To);
@@ -327,6 +321,13 @@ class RegionPageMap {
uptr getBufferSize() const { return BufferSize; }
private:
+ // We may consider making this configurable if there are cases which may
+ // benefit from this.
+ static const uptr StaticBufferCount = 2U;
+ static const uptr StaticBufferSize = 512U;
+ using BufferPoolT = BufferPool<StaticBufferCount, StaticBufferSize>;
+ static BufferPoolT Buffers;
+
uptr Regions;
uptr NumCounters;
uptr CounterSizeBitsLog;
@@ -336,13 +337,7 @@ class RegionPageMap {
uptr SizePerRegion;
uptr BufferSize;
- uptr *Buffer;
-
- // We may consider making this configurable if there are cases which may
- // benefit from this.
- static const uptr StaticBufferCount = 2U;
- static const uptr StaticBufferSize = 512U;
- static BufferPool<StaticBufferCount, StaticBufferSize> Buffers;
+ BufferPoolT::Buffer Buffer;
};
template <class ReleaseRecorderT> class FreePagesRangeTracker {
diff --git a/compiler-rt/lib/scudo/standalone/tests/release_test.cpp b/compiler-rt/lib/scudo/standalone/tests/release_test.cpp
index 0ab49460807f297..21f472da8974c93 100644
--- a/compiler-rt/lib/scudo/standalone/tests/release_test.cpp
+++ b/compiler-rt/lib/scudo/standalone/tests/release_test.cpp
@@ -23,17 +23,16 @@ TEST(ScudoReleaseTest, RegionPageMap) {
// Various valid counter's max values packed into one word.
scudo::RegionPageMap PageMap2N(1U, 1U, 1UL << I);
ASSERT_TRUE(PageMap2N.isAllocated());
- EXPECT_EQ(sizeof(scudo::uptr), PageMap2N.getBufferSize());
+ EXPECT_EQ(1, PageMap2N.getBufferSize());
// Check the "all bit set" values too.
scudo::RegionPageMap PageMap2N1_1(1U, 1U, ~0UL >> I);
ASSERT_TRUE(PageMap2N1_1.isAllocated());
- EXPECT_EQ(sizeof(scudo::uptr), PageMap2N1_1.getBufferSize());
+ EXPECT_EQ(1, PageMap2N1_1.getBufferSize());
// Verify the packing ratio, the counter is Expected to be packed into the
// closest power of 2 bits.
scudo::RegionPageMap PageMap(1U, SCUDO_WORDSIZE, 1UL << I);
ASSERT_TRUE(PageMap.isAllocated());
- EXPECT_EQ(sizeof(scudo::uptr) * scudo::roundUpPowerOfTwo(I + 1),
- PageMap.getBufferSize());
+ EXPECT_EQ(scudo::roundUpPowerOfTwo(I + 1), PageMap.getBufferSize());
}
// Go through 1, 2, 4, 8, .. {32,64} bits per counter.
@@ -636,18 +635,18 @@ TEST(ScudoReleaseTest, BufferPool) {
using BufferPool = scudo::BufferPool<StaticBufferCount, StaticBufferSize>;
std::unique_ptr<BufferPool> Pool(new BufferPool());
- std::vector<std::pair<scudo::uptr *, scudo::uptr>> Buffers;
+ std::vector<BufferPool::Buffer> Buffers;
for (scudo::uptr I = 0; I < StaticBufferCount; ++I) {
- scudo::uptr *P = Pool->getBuffer(StaticBufferSize);
- EXPECT_TRUE(Pool->isStaticBufferTestOnly(P, StaticBufferSize));
- Buffers.emplace_back(P, StaticBufferSize);
+ BufferPool::Buffer Buffer = Pool->getBuffer(StaticBufferSize);
+ EXPECT_TRUE(Pool->isStaticBufferTestOnly(Buffer));
+ Buffers.push_back(Buffer);
}
// The static buffer is supposed to be used up.
- scudo::uptr *P = Pool->getBuffer(StaticBufferSize);
- EXPECT_FALSE(Pool->isStaticBufferTestOnly(P, StaticBufferSize));
+ BufferPool::Buffer Buffer = Pool->getBuffer(StaticBufferSize);
+ EXPECT_FALSE(Pool->isStaticBufferTestOnly(Buffer));
- Pool->releaseBuffer(P, StaticBufferSize);
+ Pool->releaseBuffer(Buffer);
for (auto &Buffer : Buffers)
- Pool->releaseBuffer(Buffer.first, Buffer.second);
+ Pool->releaseBuffer(Buffer);
}
More information about the llvm-commits
mailing list