[compiler-rt] [NFC] Make RingBuffer an atomic pointer (PR #82547)
Florian Mayer via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 21 17:20:32 PST 2024
https://github.com/fmayer updated https://github.com/llvm/llvm-project/pull/82547
>From 6d7fca1020511ab4e981b84346be362bf4e3ca29 Mon Sep 17 00:00:00 2001
From: Florian Mayer <fmayer at google.com>
Date: Wed, 21 Feb 2024 14:46:32 -0800
Subject: [PATCH 1/3] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20in?=
=?UTF-8?q?itial=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.4
---
.../lib/scudo/standalone/atomic_helpers.h | 5 +
compiler-rt/lib/scudo/standalone/combined.h | 131 ++++++++++--------
2 files changed, 77 insertions(+), 59 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/atomic_helpers.h b/compiler-rt/lib/scudo/standalone/atomic_helpers.h
index a68ffd16291c8e..87619b4784ec90 100644
--- a/compiler-rt/lib/scudo/standalone/atomic_helpers.h
+++ b/compiler-rt/lib/scudo/standalone/atomic_helpers.h
@@ -54,6 +54,11 @@ struct atomic_u64 {
alignas(8) volatile Type ValDoNotUse;
};
+struct atomic_voidptr {
+ typedef void *Type;
+ volatile Type ValDoNotUse;
+};
+
struct atomic_uptr {
typedef uptr Type;
volatile Type ValDoNotUse;
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index f3c3d757c9f128..b90adec7946e13 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -688,14 +688,16 @@ class Allocator {
Quarantine.disable();
Primary.disable();
Secondary.disable();
- if (Depot)
- Depot->disable();
+ AllocationRingBuffer *RB = getRingBuffer();
+ if (RB)
+ RB->Depot->disable();
}
void enable() NO_THREAD_SAFETY_ANALYSIS {
initThreadMaybe();
- if (Depot)
- Depot->enable();
+ AllocationRingBuffer *RB = getRingBuffer();
+ if (RB)
+ RB->Depot->enable();
Secondary.enable();
Primary.enable();
Quarantine.enable();
@@ -920,12 +922,14 @@ class Allocator {
const char *getStackDepotAddress() {
initThreadMaybe();
- return reinterpret_cast<char *>(Depot);
+ AllocationRingBuffer *RB = getRingBuffer();
+ return RB ? reinterpret_cast<char *>(RB->Depot) : nullptr;
}
uptr getStackDepotSize() {
initThreadMaybe();
- return StackDepotSize;
+ AllocationRingBuffer *RB = getRingBuffer();
+ return RB ? RB->StackDepotSize : 0;
}
const char *getRegionInfoArrayAddress() const {
@@ -938,12 +942,15 @@ class Allocator {
const char *getRingBufferAddress() {
initThreadMaybe();
- return RawRingBuffer;
+ return reinterpret_cast<char *>(getRingBuffer());
}
uptr getRingBufferSize() {
initThreadMaybe();
- return RingBufferElements ? ringBufferSizeInBytes(RingBufferElements) : 0;
+ AllocationRingBuffer *RB = getRingBuffer();
+ return RB && RB->RingBufferElements
+ ? ringBufferSizeInBytes(RB->RingBufferElements)
+ : 0;
}
static const uptr MaxTraceSize = 64;
@@ -1048,10 +1055,6 @@ class Allocator {
uptr GuardedAllocSlotSize = 0;
#endif // GWP_ASAN_HOOKS
- StackDepot *Depot = nullptr;
- uptr StackDepotSize = 0;
- MemMapT RawStackDepotMap;
-
struct AllocationRingBuffer {
struct Entry {
atomic_uptr Ptr;
@@ -1061,16 +1064,29 @@ class Allocator {
atomic_u32 DeallocationTrace;
atomic_u32 DeallocationTid;
};
-
+ StackDepot *Depot = nullptr;
+ uptr StackDepotSize = 0;
+ MemMapT RawRingBufferMap;
+ MemMapT RawStackDepotMap;
+ u32 RingBufferElements;
atomic_uptr Pos;
// An array of Size (at least one) elements of type Entry is immediately
// following to this struct.
};
// Pointer to memory mapped area starting with AllocationRingBuffer struct,
// and immediately followed by Size elements of type Entry.
- char *RawRingBuffer = {};
- u32 RingBufferElements = 0;
- MemMapT RawRingBufferMap;
+ atomic_voidptr RingBuffer = {};
+
+ AllocationRingBuffer *getRingBuffer() {
+ return reinterpret_cast<AllocationRingBuffer *>(
+ atomic_load(&RingBuffer, memory_order_acquire));
+ }
+
+ AllocationRingBuffer *getRingBufferIfEnabled(const Options &Options) {
+ if (!UNLIKELY(Options.get(OptionBit::TrackAllocationStacks)))
+ return nullptr;
+ return getRingBuffer();
+ }
// The following might get optimized out by the compiler.
NOINLINE void performSanityChecks() {
@@ -1259,27 +1275,22 @@ class Allocator {
storeEndMarker(RoundNewPtr, NewSize, BlockEnd);
}
- StackDepot *getDepotIfEnabled(const Options &Options) {
- if (!UNLIKELY(Options.get(OptionBit::TrackAllocationStacks)))
- return nullptr;
- return Depot;
- }
-
void storePrimaryAllocationStackMaybe(const Options &Options, void *Ptr) {
- auto *Depot = getDepotIfEnabled(Options);
- if (!Depot)
+ auto *RingBuffer = getRingBufferIfEnabled(Options);
+ if (!RingBuffer)
return;
auto *Ptr32 = reinterpret_cast<u32 *>(Ptr);
- Ptr32[MemTagAllocationTraceIndex] = collectStackTrace(Depot);
+ Ptr32[MemTagAllocationTraceIndex] = collectStackTrace(RingBuffer->Depot);
Ptr32[MemTagAllocationTidIndex] = getThreadID();
}
- void storeRingBufferEntry(void *Ptr, u32 AllocationTrace, u32 AllocationTid,
+ void storeRingBufferEntry(AllocationRingBuffer *RB, void *Ptr,
+ u32 AllocationTrace, u32 AllocationTid,
uptr AllocationSize, u32 DeallocationTrace,
u32 DeallocationTid) {
- uptr Pos = atomic_fetch_add(&getRingBuffer()->Pos, 1, memory_order_relaxed);
+ uptr Pos = atomic_fetch_add(&RB->Pos, 1, memory_order_relaxed);
typename AllocationRingBuffer::Entry *Entry =
- getRingBufferEntry(RawRingBuffer, Pos % RingBufferElements);
+ getRingBufferEntry(RB, Pos % RB->RingBufferElements);
// First invalidate our entry so that we don't attempt to interpret a
// partially written state in getSecondaryErrorInfo(). The fences below
@@ -1300,32 +1311,32 @@ class Allocator {
void storeSecondaryAllocationStackMaybe(const Options &Options, void *Ptr,
uptr Size) {
- auto *Depot = getDepotIfEnabled(Options);
- if (!Depot)
+ auto *RingBuffer = getRingBufferIfEnabled(Options);
+ if (!RingBuffer->Depot)
return;
- u32 Trace = collectStackTrace(Depot);
+ u32 Trace = collectStackTrace(RingBuffer->Depot);
u32 Tid = getThreadID();
auto *Ptr32 = reinterpret_cast<u32 *>(Ptr);
Ptr32[MemTagAllocationTraceIndex] = Trace;
Ptr32[MemTagAllocationTidIndex] = Tid;
- storeRingBufferEntry(untagPointer(Ptr), Trace, Tid, Size, 0, 0);
+ storeRingBufferEntry(RingBuffer, untagPointer(Ptr), Trace, Tid, Size, 0, 0);
}
void storeDeallocationStackMaybe(const Options &Options, void *Ptr,
u8 PrevTag, uptr Size) {
- auto *Depot = getDepotIfEnabled(Options);
- if (!Depot)
+ auto *RingBuffer = getRingBufferIfEnabled(Options);
+ if (!RingBuffer->Depot)
return;
auto *Ptr32 = reinterpret_cast<u32 *>(Ptr);
u32 AllocationTrace = Ptr32[MemTagAllocationTraceIndex];
u32 AllocationTid = Ptr32[MemTagAllocationTidIndex];
- u32 DeallocationTrace = collectStackTrace(Depot);
+ u32 DeallocationTrace = collectStackTrace(RingBuffer->Depot);
u32 DeallocationTid = getThreadID();
- storeRingBufferEntry(addFixedTag(untagPointer(Ptr), PrevTag),
+ storeRingBufferEntry(RingBuffer, addFixedTag(untagPointer(Ptr), PrevTag),
AllocationTrace, AllocationTid, Size,
DeallocationTrace, DeallocationTid);
}
@@ -1434,7 +1445,7 @@ class Allocator {
for (uptr I = Pos - 1; I != Pos - 1 - RingBufferElements &&
NextErrorReport != NumErrorReports;
--I) {
- auto *Entry = getRingBufferEntry(RingBufferPtr, I % RingBufferElements);
+ auto *Entry = getRingBufferEntry(RingBuffer, I % RingBufferElements);
uptr EntryPtr = atomic_load_relaxed(&Entry->Ptr);
if (!EntryPtr)
continue;
@@ -1502,12 +1513,14 @@ class Allocator {
}
static typename AllocationRingBuffer::Entry *
- getRingBufferEntry(char *RawRingBuffer, uptr N) {
+ getRingBufferEntry(AllocationRingBuffer *RB, uptr N) {
+ char *RawRingBuffer = reinterpret_cast<char *>(RB);
return &reinterpret_cast<typename AllocationRingBuffer::Entry *>(
&RawRingBuffer[sizeof(AllocationRingBuffer)])[N];
}
static const typename AllocationRingBuffer::Entry *
- getRingBufferEntry(const char *RawRingBuffer, uptr N) {
+ getRingBufferEntry(const AllocationRingBuffer *RB, uptr N) {
+ const char *RawRingBuffer = reinterpret_cast<const char *>(RB);
return &reinterpret_cast<const typename AllocationRingBuffer::Entry *>(
&RawRingBuffer[sizeof(AllocationRingBuffer)])[N];
}
@@ -1544,15 +1557,14 @@ class Allocator {
u32 RingSize = static_cast<u32>(TabSize * kFramesPerStack);
DCHECK(isPowerOfTwo(RingSize));
- StackDepotSize = sizeof(StackDepot) + sizeof(atomic_u64) * RingSize +
- sizeof(atomic_u32) * TabSize;
+ auto StackDepotSize = sizeof(StackDepot) + sizeof(atomic_u64) * RingSize +
+ sizeof(atomic_u32) * TabSize;
MemMapT DepotMap;
DepotMap.map(
/*Addr=*/0U, roundUp(StackDepotSize, getPageSizeCached()),
"scudo:stack_depot");
- Depot = reinterpret_cast<StackDepot *>(DepotMap.getBase());
+ auto *Depot = reinterpret_cast<StackDepot *>(DepotMap.getBase());
Depot->init(RingSize, TabSize);
- RawStackDepotMap = DepotMap;
MemMapT MemMap;
MemMap.map(
@@ -1560,9 +1572,14 @@ class Allocator {
roundUp(ringBufferSizeInBytes(AllocationRingBufferSize),
getPageSizeCached()),
"scudo:ring_buffer");
- RawRingBuffer = reinterpret_cast<char *>(MemMap.getBase());
- RawRingBufferMap = MemMap;
- RingBufferElements = AllocationRingBufferSize;
+ auto *RB = reinterpret_cast<AllocationRingBuffer *>(MemMap.getBase());
+ RB->RawRingBufferMap = MemMap;
+ RB->RingBufferElements = AllocationRingBufferSize;
+ RB->Depot = Depot;
+ RB->StackDepotSize = StackDepotSize;
+ RB->RawStackDepotMap = DepotMap;
+
+ atomic_store(&RingBuffer, RB, memory_order_release);
static_assert(sizeof(AllocationRingBuffer) %
alignof(typename AllocationRingBuffer::Entry) ==
0,
@@ -1570,16 +1587,16 @@ class Allocator {
}
void unmapRingBuffer() {
- auto *RingBuffer = getRingBuffer();
- if (RingBuffer != nullptr) {
- RawRingBufferMap.unmap(RawRingBufferMap.getBase(),
- RawRingBufferMap.getCapacity());
- }
- RawRingBuffer = nullptr;
- if (Depot) {
- RawStackDepotMap.unmap(RawStackDepotMap.getBase(),
- RawStackDepotMap.getCapacity());
+ AllocationRingBuffer *RB = getRingBuffer();
+ if (RB != nullptr) {
+ // N.B. because RawStackDepotMap is part of RawRingBufferMap, the order
+ // is very important.
+ RB->RawStackDepotMap.unmap(RB->RawStackDepotMap.getBase(),
+ RB->RawStackDepotMap.getCapacity());
+ RB->RawRingBufferMap.unmap(RB->RawRingBufferMap.getBase(),
+ RB->RawRingBufferMap.getCapacity());
}
+ atomic_store(&RingBuffer, nullptr, memory_order_release);
}
static constexpr size_t ringBufferSizeInBytes(u32 RingBufferElements) {
@@ -1594,10 +1611,6 @@ class Allocator {
return (Bytes - sizeof(AllocationRingBuffer)) /
sizeof(typename AllocationRingBuffer::Entry);
}
-
- inline AllocationRingBuffer *getRingBuffer() {
- return reinterpret_cast<AllocationRingBuffer *>(RawRingBuffer);
- }
};
} // namespace scudo
>From dafbdf47d29b5c360d7382b0be6562dc504b97ea Mon Sep 17 00:00:00 2001
From: Florian Mayer <fmayer at google.com>
Date: Wed, 21 Feb 2024 14:50:28 -0800
Subject: [PATCH 2/3] forgot to amend
Created using spr 1.3.4
---
compiler-rt/lib/scudo/standalone/combined.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index b90adec7946e13..4bb7f80beb2e46 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -1312,7 +1312,7 @@ class Allocator {
void storeSecondaryAllocationStackMaybe(const Options &Options, void *Ptr,
uptr Size) {
auto *RingBuffer = getRingBufferIfEnabled(Options);
- if (!RingBuffer->Depot)
+ if (!RingBuffer)
return;
u32 Trace = collectStackTrace(RingBuffer->Depot);
u32 Tid = getThreadID();
@@ -1327,7 +1327,7 @@ class Allocator {
void storeDeallocationStackMaybe(const Options &Options, void *Ptr,
u8 PrevTag, uptr Size) {
auto *RingBuffer = getRingBufferIfEnabled(Options);
- if (!RingBuffer->Depot)
+ if (!RingBuffer)
return;
auto *Ptr32 = reinterpret_cast<u32 *>(Ptr);
u32 AllocationTrace = Ptr32[MemTagAllocationTraceIndex];
>From 8fe68d417b2f37ba5ffd9281894455252a6b80ab Mon Sep 17 00:00:00 2001
From: Florian Mayer <fmayer at google.com>
Date: Wed, 21 Feb 2024 17:20:22 -0800
Subject: [PATCH 3/3] address comments
Created using spr 1.3.4
---
.../lib/scudo/standalone/atomic_helpers.h | 5 --
compiler-rt/lib/scudo/standalone/combined.h | 69 ++++++++++---------
2 files changed, 38 insertions(+), 36 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/atomic_helpers.h b/compiler-rt/lib/scudo/standalone/atomic_helpers.h
index 87619b4784ec90..a68ffd16291c8e 100644
--- a/compiler-rt/lib/scudo/standalone/atomic_helpers.h
+++ b/compiler-rt/lib/scudo/standalone/atomic_helpers.h
@@ -54,11 +54,6 @@ struct atomic_u64 {
alignas(8) volatile Type ValDoNotUse;
};
-struct atomic_voidptr {
- typedef void *Type;
- volatile Type ValDoNotUse;
-};
-
struct atomic_uptr {
typedef uptr Type;
volatile Type ValDoNotUse;
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index 4bb7f80beb2e46..4ac3c820c40e13 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -690,14 +690,14 @@ class Allocator {
Secondary.disable();
AllocationRingBuffer *RB = getRingBuffer();
if (RB)
- RB->Depot->disable();
+ RB->disable();
}
void enable() NO_THREAD_SAFETY_ANALYSIS {
initThreadMaybe();
AllocationRingBuffer *RB = getRingBuffer();
if (RB)
- RB->Depot->enable();
+ RB->enable();
Secondary.enable();
Primary.enable();
Quarantine.enable();
@@ -1068,18 +1068,22 @@ class Allocator {
uptr StackDepotSize = 0;
MemMapT RawRingBufferMap;
MemMapT RawStackDepotMap;
- u32 RingBufferElements;
+ u32 RingBufferElements = 0;
atomic_uptr Pos;
// An array of Size (at least one) elements of type Entry is immediately
// following to this struct.
+
+ void enable() { Depot->enable(); }
+
+ void disable() { Depot->disable(); }
};
// Pointer to memory mapped area starting with AllocationRingBuffer struct,
// and immediately followed by Size elements of type Entry.
- atomic_voidptr RingBuffer = {};
+ atomic_uptr RingBufferAddress = {};
AllocationRingBuffer *getRingBuffer() {
return reinterpret_cast<AllocationRingBuffer *>(
- atomic_load(&RingBuffer, memory_order_acquire));
+ atomic_load(&RingBufferAddress, memory_order_acquire));
}
AllocationRingBuffer *getRingBufferIfEnabled(const Options &Options) {
@@ -1276,11 +1280,11 @@ class Allocator {
}
void storePrimaryAllocationStackMaybe(const Options &Options, void *Ptr) {
- auto *RingBuffer = getRingBufferIfEnabled(Options);
- if (!RingBuffer)
+ AllocationRingBuffer *RB = getRingBufferIfEnabled(Options);
+ if (!RB)
return;
auto *Ptr32 = reinterpret_cast<u32 *>(Ptr);
- Ptr32[MemTagAllocationTraceIndex] = collectStackTrace(RingBuffer->Depot);
+ Ptr32[MemTagAllocationTraceIndex] = collectStackTrace(RB->Depot);
Ptr32[MemTagAllocationTidIndex] = getThreadID();
}
@@ -1311,32 +1315,32 @@ class Allocator {
void storeSecondaryAllocationStackMaybe(const Options &Options, void *Ptr,
uptr Size) {
- auto *RingBuffer = getRingBufferIfEnabled(Options);
- if (!RingBuffer)
+ auto *RB = getRingBufferIfEnabled(Options);
+ if (!RB)
return;
- u32 Trace = collectStackTrace(RingBuffer->Depot);
+ u32 Trace = collectStackTrace(RB->Depot);
u32 Tid = getThreadID();
auto *Ptr32 = reinterpret_cast<u32 *>(Ptr);
Ptr32[MemTagAllocationTraceIndex] = Trace;
Ptr32[MemTagAllocationTidIndex] = Tid;
- storeRingBufferEntry(RingBuffer, untagPointer(Ptr), Trace, Tid, Size, 0, 0);
+ storeRingBufferEntry(RB, untagPointer(Ptr), Trace, Tid, Size, 0, 0);
}
void storeDeallocationStackMaybe(const Options &Options, void *Ptr,
u8 PrevTag, uptr Size) {
- auto *RingBuffer = getRingBufferIfEnabled(Options);
- if (!RingBuffer)
+ auto *RB = getRingBufferIfEnabled(Options);
+ if (!RB)
return;
auto *Ptr32 = reinterpret_cast<u32 *>(Ptr);
u32 AllocationTrace = Ptr32[MemTagAllocationTraceIndex];
u32 AllocationTid = Ptr32[MemTagAllocationTidIndex];
- u32 DeallocationTrace = collectStackTrace(RingBuffer->Depot);
+ u32 DeallocationTrace = collectStackTrace(RB->Depot);
u32 DeallocationTid = getThreadID();
- storeRingBufferEntry(RingBuffer, addFixedTag(untagPointer(Ptr), PrevTag),
+ storeRingBufferEntry(RB, addFixedTag(untagPointer(Ptr), PrevTag),
AllocationTrace, AllocationTid, Size,
DeallocationTrace, DeallocationTid);
}
@@ -1514,15 +1518,17 @@ class Allocator {
static typename AllocationRingBuffer::Entry *
getRingBufferEntry(AllocationRingBuffer *RB, uptr N) {
- char *RawRingBuffer = reinterpret_cast<char *>(RB);
+ char *RBEntryStart =
+ &reinterpret_cast<char *>(RB)[sizeof(AllocationRingBuffer)];
return &reinterpret_cast<typename AllocationRingBuffer::Entry *>(
- &RawRingBuffer[sizeof(AllocationRingBuffer)])[N];
+ RBEntryStart)[N];
}
static const typename AllocationRingBuffer::Entry *
getRingBufferEntry(const AllocationRingBuffer *RB, uptr N) {
- const char *RawRingBuffer = reinterpret_cast<const char *>(RB);
+ const char *RBEntryStart =
+ &reinterpret_cast<const char *>(RB)[sizeof(AllocationRingBuffer)];
return &reinterpret_cast<const typename AllocationRingBuffer::Entry *>(
- &RawRingBuffer[sizeof(AllocationRingBuffer)])[N];
+ RBEntryStart)[N];
}
void mapAndInitializeRingBuffer() {
@@ -1557,7 +1563,7 @@ class Allocator {
u32 RingSize = static_cast<u32>(TabSize * kFramesPerStack);
DCHECK(isPowerOfTwo(RingSize));
- auto StackDepotSize = sizeof(StackDepot) + sizeof(atomic_u64) * RingSize +
+ uptr StackDepotSize = sizeof(StackDepot) + sizeof(atomic_u64) * RingSize +
sizeof(atomic_u32) * TabSize;
MemMapT DepotMap;
DepotMap.map(
@@ -1579,7 +1585,8 @@ class Allocator {
RB->StackDepotSize = StackDepotSize;
RB->RawStackDepotMap = DepotMap;
- atomic_store(&RingBuffer, RB, memory_order_release);
+ atomic_store(&RingBufferAddress, reinterpret_cast<uptr>(RB),
+ memory_order_release);
static_assert(sizeof(AllocationRingBuffer) %
alignof(typename AllocationRingBuffer::Entry) ==
0,
@@ -1588,15 +1595,15 @@ class Allocator {
void unmapRingBuffer() {
AllocationRingBuffer *RB = getRingBuffer();
- if (RB != nullptr) {
- // N.B. because RawStackDepotMap is part of RawRingBufferMap, the order
- // is very important.
- RB->RawStackDepotMap.unmap(RB->RawStackDepotMap.getBase(),
- RB->RawStackDepotMap.getCapacity());
- RB->RawRingBufferMap.unmap(RB->RawRingBufferMap.getBase(),
- RB->RawRingBufferMap.getCapacity());
- }
- atomic_store(&RingBuffer, nullptr, memory_order_release);
+ if (RB == nullptr)
+ return;
+ // N.B. because RawStackDepotMap is part of RawRingBufferMap, the order
+ // is very important.
+ RB->RawStackDepotMap.unmap(RB->RawStackDepotMap.getBase(),
+ RB->RawStackDepotMap.getCapacity());
+ RB->RawRingBufferMap.unmap(RB->RawRingBufferMap.getBase(),
+ RB->RawRingBufferMap.getCapacity());
+ atomic_store(&RingBufferAddress, 0, memory_order_release);
}
static constexpr size_t ringBufferSizeInBytes(u32 RingBufferElements) {
More information about the llvm-commits
mailing list