[compiler-rt] [scudo] Add two missing locks to enable/disable. (PR #79670)
Evgenii Stepanov via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 29 10:09:03 PST 2024
https://github.com/eugenis updated https://github.com/llvm/llvm-project/pull/79670
>From cccdbb69ed588099aa1366c17514a1c4bfb97367 Mon Sep 17 00:00:00 2001
From: Evgenii Stepanov <eugenis at google.com>
Date: Fri, 26 Jan 2024 16:29:13 -0800
Subject: [PATCH 1/2] [scudo] Add two missing locks to enable/disable.
The StackDepot lock was actually observed deadlocking in the code that uses
malloc after fork but before exec. The other one was found by code
inspection.
---
compiler-rt/lib/scudo/standalone/combined.h | 2 ++
compiler-rt/lib/scudo/standalone/primary32.h | 2 ++
compiler-rt/lib/scudo/standalone/primary64.h | 2 ++
compiler-rt/lib/scudo/standalone/release.h | 16 ++++++++++++++++
compiler-rt/lib/scudo/standalone/stack_depot.h | 8 ++++++++
5 files changed, 30 insertions(+)
diff --git a/compiler-rt/lib/scudo/standalone/combined.h b/compiler-rt/lib/scudo/standalone/combined.h
index 4624f83d142a0de..80774073522a72c 100644
--- a/compiler-rt/lib/scudo/standalone/combined.h
+++ b/compiler-rt/lib/scudo/standalone/combined.h
@@ -691,10 +691,12 @@ class Allocator {
Quarantine.disable();
Primary.disable();
Secondary.disable();
+ Depot.disable();
}
void enable() NO_THREAD_SAFETY_ANALYSIS {
initThreadMaybe();
+ Depot.enable();
Secondary.enable();
Primary.enable();
Quarantine.enable();
diff --git a/compiler-rt/lib/scudo/standalone/primary32.h b/compiler-rt/lib/scudo/standalone/primary32.h
index 4d03b282d000def..89fffc50dcd0096 100644
--- a/compiler-rt/lib/scudo/standalone/primary32.h
+++ b/compiler-rt/lib/scudo/standalone/primary32.h
@@ -261,6 +261,7 @@ template <typename Config> class SizeClassAllocator32 {
}
void disable() NO_THREAD_SAFETY_ANALYSIS {
+ RegionPageMap::disable();
// The BatchClassId must be locked last since other classes can use it.
for (sptr I = static_cast<sptr>(NumClasses) - 1; I >= 0; I--) {
if (static_cast<uptr>(I) == SizeClassMap::BatchClassId)
@@ -281,6 +282,7 @@ template <typename Config> class SizeClassAllocator32 {
continue;
getSizeClassInfo(I)->Mutex.unlock();
}
+ RegionPageMap::enable();
}
template <typename F> void iterateOverBlocks(F Callback) {
diff --git a/compiler-rt/lib/scudo/standalone/primary64.h b/compiler-rt/lib/scudo/standalone/primary64.h
index 9a642d23620e1ea..4adb53b3748ffe3 100644
--- a/compiler-rt/lib/scudo/standalone/primary64.h
+++ b/compiler-rt/lib/scudo/standalone/primary64.h
@@ -341,9 +341,11 @@ template <typename Config> class SizeClassAllocator64 {
}
getRegionInfo(SizeClassMap::BatchClassId)->MMLock.lock();
getRegionInfo(SizeClassMap::BatchClassId)->FLLock.lock();
+ RegionPageMap::disable();
}
void enable() NO_THREAD_SAFETY_ANALYSIS {
+ RegionPageMap::enable();
getRegionInfo(SizeClassMap::BatchClassId)->FLLock.unlock();
getRegionInfo(SizeClassMap::BatchClassId)->MMLock.unlock();
for (uptr I = 0; I < NumClasses; I++) {
diff --git a/compiler-rt/lib/scudo/standalone/release.h b/compiler-rt/lib/scudo/standalone/release.h
index b6f76a4d20585ab..6e90ec97e0f3c32 100644
--- a/compiler-rt/lib/scudo/standalone/release.h
+++ b/compiler-rt/lib/scudo/standalone/release.h
@@ -168,6 +168,14 @@ class BufferPool {
return Buf.BufferIndex != StaticBufferCount;
}
+ void disable() NO_THREAD_SAFETY_ANALYSIS {
+ Mutex.lock();
+ }
+
+ void enable() NO_THREAD_SAFETY_ANALYSIS {
+ Mutex.unlock();
+ }
+
private:
Buffer getDynamicBuffer(const uptr NumElements) {
// When using a heap-based buffer, precommit the pages backing the
@@ -325,6 +333,14 @@ class RegionPageMap {
uptr getBufferNumElements() const { return BufferNumElements; }
+ static void disable() NO_THREAD_SAFETY_ANALYSIS {
+ Buffers.disable();
+ }
+
+ static void enable() NO_THREAD_SAFETY_ANALYSIS {
+ Buffers.enable();
+ }
+
private:
// We may consider making this configurable if there are cases which may
// benefit from this.
diff --git a/compiler-rt/lib/scudo/standalone/stack_depot.h b/compiler-rt/lib/scudo/standalone/stack_depot.h
index 12c35eb2a4f3383..ab5e96680cab58c 100644
--- a/compiler-rt/lib/scudo/standalone/stack_depot.h
+++ b/compiler-rt/lib/scudo/standalone/stack_depot.h
@@ -136,6 +136,14 @@ class StackDepot {
u64 operator[](uptr RingPos) const {
return atomic_load_relaxed(&Ring[RingPos & RingMask]);
}
+
+ void disable() NO_THREAD_SAFETY_ANALYSIS {
+ RingEndMu.lock();
+ }
+
+ void enable() NO_THREAD_SAFETY_ANALYSIS {
+ RingEndMu.unlock();
+ }
};
} // namespace scudo
>From 7a9ca1d9267e470b50d3122766064a0d204ecb6b Mon Sep 17 00:00:00 2001
From: Evgenii Stepanov <eugenis at google.com>
Date: Mon, 29 Jan 2024 10:08:12 -0800
Subject: [PATCH 2/2] fixup: clang-format
---
compiler-rt/lib/scudo/standalone/release.h | 16 ++++------------
compiler-rt/lib/scudo/standalone/stack_depot.h | 8 ++------
2 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/compiler-rt/lib/scudo/standalone/release.h b/compiler-rt/lib/scudo/standalone/release.h
index 6e90ec97e0f3c32..a444a2ce3dc9a89 100644
--- a/compiler-rt/lib/scudo/standalone/release.h
+++ b/compiler-rt/lib/scudo/standalone/release.h
@@ -168,13 +168,9 @@ class BufferPool {
return Buf.BufferIndex != StaticBufferCount;
}
- void disable() NO_THREAD_SAFETY_ANALYSIS {
- Mutex.lock();
- }
+ void disable() NO_THREAD_SAFETY_ANALYSIS { Mutex.lock(); }
- void enable() NO_THREAD_SAFETY_ANALYSIS {
- Mutex.unlock();
- }
+ void enable() NO_THREAD_SAFETY_ANALYSIS { Mutex.unlock(); }
private:
Buffer getDynamicBuffer(const uptr NumElements) {
@@ -333,13 +329,9 @@ class RegionPageMap {
uptr getBufferNumElements() const { return BufferNumElements; }
- static void disable() NO_THREAD_SAFETY_ANALYSIS {
- Buffers.disable();
- }
+ static void disable() NO_THREAD_SAFETY_ANALYSIS { Buffers.disable(); }
- static void enable() NO_THREAD_SAFETY_ANALYSIS {
- Buffers.enable();
- }
+ static void enable() NO_THREAD_SAFETY_ANALYSIS { Buffers.enable(); }
private:
// We may consider making this configurable if there are cases which may
diff --git a/compiler-rt/lib/scudo/standalone/stack_depot.h b/compiler-rt/lib/scudo/standalone/stack_depot.h
index ab5e96680cab58c..bad30bdd85422d5 100644
--- a/compiler-rt/lib/scudo/standalone/stack_depot.h
+++ b/compiler-rt/lib/scudo/standalone/stack_depot.h
@@ -137,13 +137,9 @@ class StackDepot {
return atomic_load_relaxed(&Ring[RingPos & RingMask]);
}
- void disable() NO_THREAD_SAFETY_ANALYSIS {
- RingEndMu.lock();
- }
+ void disable() NO_THREAD_SAFETY_ANALYSIS { RingEndMu.lock(); }
- void enable() NO_THREAD_SAFETY_ANALYSIS {
- RingEndMu.unlock();
- }
+ void enable() NO_THREAD_SAFETY_ANALYSIS { RingEndMu.unlock(); }
};
} // namespace scudo
More information about the llvm-commits
mailing list