Cold-Air-Networks/mirror-llvm-project
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 7

[scudo] Fix all deadlocks with condition code variables. (#212604)

Browse source 
Added a ScopedFLLock that automatically handles notify calls if there
are waiters. Replace all ScopedLock of FLLock with this new
ScopedFLLock. Updated the enable function to properly notify FLLock
waiters if necessary.

Added a new function for re-enabling in the child process after a fork
to clear the variables used by the condition code.
This commit is contained in:
Christopher Ferris 2026-07-29 22:02:32 +00:00 committed by GitHub
parent f3159b97be
commit 738cbf1883
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 124 additions and 68 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
compiler-rt/lib/scudo/standalone/combined.h
View file

@ -687,11 +687,11 @@ public:
disableRingBuffer();
}
void enable() NO_THREAD_SAFETY_ANALYSIS {
void enable(bool IsChild) NO_THREAD_SAFETY_ANALYSIS {
initThreadMaybe();
enableRingBuffer();
Secondary.enable();
Primary.enable();
Primary.enable(IsChild);
if (!AllocatorConfig::getQuarantineDisabled())
Quarantine.enable();
Stats.enable();

5
compiler-rt/lib/scudo/standalone/primary32.h
View file

@ -114,7 +114,7 @@ public:
CompactPtrT *Array, u32 Size);
void disable() NO_THREAD_SAFETY_ANALYSIS;
void enable() NO_THREAD_SAFETY_ANALYSIS;
void enable(bool IsChild) NO_THREAD_SAFETY_ANALYSIS;
template <typename F> void iterateOverBlocks(F Callback);
@ -403,7 +403,8 @@ void SizeClassAllocator32<Config>::disable() NO_THREAD_SAFETY_ANALYSIS {
}
template <typename Config>
void SizeClassAllocator32<Config>::enable() NO_THREAD_SAFETY_ANALYSIS {
void SizeClassAllocator32<Config>::enable(UNUSED bool IsChild)
NO_THREAD_SAFETY_ANALYSIS {
ByteMapMutex.unlock();
RegionsStashMutex.unlock();
getSizeClassInfo(SizeClassMap::BatchClassId)->Mutex.unlock();

159
compiler-rt/lib/scudo/standalone/primary64.h
View file

@ -86,7 +86,7 @@ public:
}
static bool canAllocate(uptr Size) { return Size <= SizeClassMap::MaxSize; }
static bool conditionVariableEnabled() {
static constexpr bool conditionVariableEnabled() {
return Config::hasConditionVariableT();
}
@ -107,7 +107,7 @@ public:
CompactPtrT *Array, u32 Size);
void disable() NO_THREAD_SAFETY_ANALYSIS;
void enable() NO_THREAD_SAFETY_ANALYSIS;
void enable(bool IsChild) NO_THREAD_SAFETY_ANALYSIS;
template <typename F> void iterateOverBlocks(F Callback);
@ -180,12 +180,13 @@ private:
HybridMutex MMLock ACQUIRED_BEFORE(FLLock);
// `RegionBeg` is initialized before thread creation and won't be changed.
uptr RegionBeg = 0;
u32 RandState GUARDED_BY(MMLock) = 0;
BlocksInfo FreeListInfo GUARDED_BY(FLLock);
PagesInfo MemMapInfo GUARDED_BY(MMLock);
ReleaseToOsInfo ReleaseInfo GUARDED_BY(MMLock) = {};
bool Exhausted GUARDED_BY(MMLock) = false;
bool isPopulatingFreeList GUARDED_BY(FLLock) = false;
bool IsPopulatingFreeList GUARDED_BY(FLLock) = false;
u16 NumWaiting GUARDED_BY(FLLock) = 0;
u32 RandState GUARDED_BY(MMLock) = 0;
};
struct RegionInfo : UnpaddedRegionInfo {
char Padding[SCUDO_CACHE_LINE_SIZE -
@ -193,6 +194,44 @@ private:
};
static_assert(sizeof(RegionInfo) % SCUDO_CACHE_LINE_SIZE == 0, "");
template <bool ConditionVariableEnabled = false>
class SCOPED_CAPABILITY ScopedFLLockBase {
public:
ScopedFLLockBase(HybridMutex &M, UNUSED RegionInfo *Region) ACQUIRE(M)
: Mutex(M) {
Mutex.lock();
}
~ScopedFLLockBase() RELEASE() { Mutex.unlock(); }
private:
HybridMutex &Mutex;
ScopedFLLockBase(const ScopedFLLockBase &) = delete;
void operator=(const ScopedFLLockBase &) = delete;
};
template <> class SCOPED_CAPABILITY ScopedFLLockBase<true> {
public:
ScopedFLLockBase(HybridMutex &M, RegionInfo *Region) ACQUIRE(M)
: Mutex(M), Region(Region) {
Mutex.lock();
}
~ScopedFLLockBase() RELEASE() {
if (Region->NumWaiting > 0)
Region->FLLockCV.notifyAll(Mutex);
Mutex.unlock();
}
private:
HybridMutex &Mutex;
RegionInfo *Region;
ScopedFLLockBase(const ScopedFLLockBase &) = delete;
void operator=(const ScopedFLLockBase &) = delete;
};
using ScopedFLLock = ScopedFLLockBase<conditionVariableEnabled()>;
RegionInfo *getRegionInfo(uptr ClassId) {
DCHECK_LT(ClassId, NumClasses);
return &RegionInfoArray[ClassId];
@ -361,10 +400,12 @@ void SizeClassAllocator64<Config>::init(s32 ReleaseToOsInterval)
shuffle(RegionInfoArray, NumClasses, &Seed);
}
// The binding should be done after region shuffling so that it won't bind
// the FLLock from the wrong region.
for (uptr I = 0; I < NumClasses; I++)
getRegionInfo(I)->FLLockCV.bindTestOnly(getRegionInfo(I)->FLLock);
if constexpr (SCUDO_DEBUG && conditionVariableEnabled()) {
// The binding should be done after region shuffling so that it won't bind
// the FLLock from the wrong region.
for (uptr I = 0; I < NumClasses; I++)
getRegionInfo(I)->FLLockCV.bindTestOnly(getRegionInfo(I)->FLLock);
}
// The default value in the primary config has the higher priority.
if (Config::getDefaultReleaseToOsIntervalMs() != INT32_MIN)
@ -424,7 +465,7 @@ void SizeClassAllocator64<Config>::verifyAllBlocksAreReleasedTestOnly() {
continue;
RegionInfo *Region = getRegionInfo(I);
ScopedLock ML(Region->MMLock);
ScopedLock FL(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
const uptr BlockSize = getSizeByClassId(I);
uptr TotalBlocks = 0;
for (BatchGroupT &BG : Region->FreeListInfo.BlockList) {
@ -441,7 +482,7 @@ void SizeClassAllocator64<Config>::verifyAllBlocksAreReleasedTestOnly() {
RegionInfo *Region = getRegionInfo(SizeClassMap::BatchClassId);
ScopedLock ML(Region->MMLock);
ScopedLock FL(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
const uptr BlockSize = getSizeByClassId(SizeClassMap::BatchClassId);
uptr TotalBlocks = 0;
for (BatchGroupT &BG : Region->FreeListInfo.BlockList) {
@ -472,7 +513,7 @@ u16 SizeClassAllocator64<Config>::popBlocks(
u16 PopCount = 0;
{
ScopedLock L(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
PopCount = popBlocksImpl(SizeClassAllocator, ClassId, Region, ToArray,
MaxBlockCount);
if (PopCount != 0U)
@ -481,7 +522,7 @@ u16 SizeClassAllocator64<Config>::popBlocks(
bool ReportRegionExhausted = false;
if (conditionVariableEnabled()) {
if constexpr (conditionVariableEnabled()) {
PopCount = popBlocksWithCV(SizeClassAllocator, ClassId, Region, ToArray,
MaxBlockCount, ReportRegionExhausted);
} else {
@ -491,7 +532,7 @@ u16 SizeClassAllocator64<Config>::popBlocks(
// doing that, always check the freelist before mapping new pages.
ScopedLock ML(Region->MMLock);
{
ScopedLock FL(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
PopCount = popBlocksImpl(SizeClassAllocator, ClassId, Region, ToArray,
MaxBlockCount);
if (PopCount != 0U)
@ -535,9 +576,9 @@ u16 SizeClassAllocator64<Config>::popBlocksWithCV(
// threads.
bool PopulateFreeList = false;
{
ScopedLock FL(Region->FLLock);
if (!Region->isPopulatingFreeList) {
Region->isPopulatingFreeList = true;
ScopedFLLock FL(Region->FLLock, Region);
if (!Region->IsPopulatingFreeList) {
Region->IsPopulatingFreeList = true;
PopulateFreeList = true;
}
}
@ -556,11 +597,10 @@ u16 SizeClassAllocator64<Config>::popBlocksWithCV(
// Before reacquiring the `FLLock`, the freelist may be used up again
// and some threads are waiting for the freelist refillment by the
// current thread. It's important to set
// `Region->isPopulatingFreeList` to false so the threads about to
// `Region->IsPopulatingFreeList` to false so the threads about to
// sleep will notice the status change.
ScopedLock FL(Region->FLLock);
Region->isPopulatingFreeList = false;
Region->FLLockCV.notifyAll(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
Region->IsPopulatingFreeList = false;
}
break;
@ -568,20 +608,20 @@ u16 SizeClassAllocator64<Config>::popBlocksWithCV(
// At here, there are two preconditions to be met before waiting,
// 1. The freelist is empty.
// 2. Region->isPopulatingFreeList == true, i.e, someone is still doing
// 2. Region->IsPopulatingFreeList == true, i.e, someone is still doing
// `populateFreeListAndPopBlocks()`.
//
// Note that it has the chance that freelist is empty but
// Region->isPopulatingFreeList == false because all the new populated
// Region->IsPopulatingFreeList == false because all the new populated
// blocks were used up right after the refillment. Therefore, we have to
// check if someone is still populating the freelist.
ScopedLock FL(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
PopCount = popBlocksImpl(SizeClassAllocator, ClassId, Region, ToArray,
MaxBlockCount);
if (PopCount != 0U)
break;
if (!Region->isPopulatingFreeList)
if (!Region->IsPopulatingFreeList)
continue;
// Now the freelist is empty and someone's doing the refillment. We will
@ -589,7 +629,9 @@ u16 SizeClassAllocator64<Config>::popBlocksWithCV(
// `populateFreeListAndPopBlocks()`. The refillment can be done by
// `populateFreeListAndPopBlocks()`, `pushBlocks()`,
// `pushBatchClassBlocks()` and `mergeGroupsToReleaseBack()`.
++Region->NumWaiting;
Region->FLLockCV.wait(Region->FLLock);
--Region->NumWaiting;
PopCount = popBlocksImpl(SizeClassAllocator, ClassId, Region, ToArray,
MaxBlockCount);
@ -736,8 +778,7 @@ u16 SizeClassAllocator64<Config>::populateFreeListAndPopBlocks(
for (u32 I = 0; I < NumberOfBlocks; I++, P += Size)
ShuffleArray[I] = compactPtrInternal(CompactPtrBase, P);
ScopedLock L(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
if (ClassId != SizeClassMap::BatchClassId) {
u32 N = 1;
uptr CurGroup = compactPtrGroup(ShuffleArray[0]);
@ -788,10 +829,8 @@ void SizeClassAllocator64<Config>::pushBlocks(
RegionInfo *Region = getRegionInfo(ClassId);
if (ClassId == SizeClassMap::BatchClassId) {
ScopedLock L(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
pushBatchClassBlocks(Region, Array, Size);
if (conditionVariableEnabled())
Region->FLLockCV.notifyAll(Region->FLLock);
return;
}
@ -819,10 +858,8 @@ void SizeClassAllocator64<Config>::pushBlocks(
}
{
ScopedLock L(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
pushBlocksImpl(SizeClassAllocator, ClassId, Region, Array, Size, SameGroup);
if (conditionVariableEnabled())
Region->FLLockCV.notifyAll(Region->FLLock);
}
}
@ -1068,14 +1105,34 @@ void SizeClassAllocator64<Config>::disable() NO_THREAD_SAFETY_ANALYSIS {
}
template <typename Config>
void SizeClassAllocator64<Config>::enable() NO_THREAD_SAFETY_ANALYSIS {
getRegionInfo(SizeClassMap::BatchClassId)->FLLock.unlock();
getRegionInfo(SizeClassMap::BatchClassId)->MMLock.unlock();
void SizeClassAllocator64<Config>::enable(bool IsChild)
NO_THREAD_SAFETY_ANALYSIS {
auto *BatchRegion = getRegionInfo(SizeClassMap::BatchClassId);
if constexpr (conditionVariableEnabled()) {
if (IsChild) {
BatchRegion->NumWaiting = 0;
BatchRegion->IsPopulatingFreeList = false;
} else if (BatchRegion->NumWaiting > 0) {
BatchRegion->FLLockCV.notifyAll(BatchRegion->FLLock);
}
}
BatchRegion->FLLock.unlock();
BatchRegion->MMLock.unlock();
for (uptr I = 0; I < NumClasses; I++) {
if (I == SizeClassMap::BatchClassId)
continue;
getRegionInfo(I)->FLLock.unlock();
getRegionInfo(I)->MMLock.unlock();
auto *Region = getRegionInfo(I);
if constexpr (conditionVariableEnabled()) {
if (IsChild) {
Region->NumWaiting = 0;
Region->IsPopulatingFreeList = false;
} else if (Region->NumWaiting > 0) {
Region->FLLockCV.notifyAll(Region->FLLock);
}
}
Region->FLLock.unlock();
Region->MMLock.unlock();
}
}
@ -1114,7 +1171,7 @@ void SizeClassAllocator64<Config>::getStats(ScopedString *Str) {
TotalMapped += Region->MemMapInfo.MappedUser;
}
{
ScopedLock L(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
PoppedBlocks += Region->FreeListInfo.PoppedBlocks;
PushedBlocks += Region->FreeListInfo.PushedBlocks;
}
@ -1127,8 +1184,8 @@ void SizeClassAllocator64<Config>::getStats(ScopedString *Str) {
for (uptr I = 0; I < NumClasses; I++) {
RegionInfo *Region = getRegionInfo(I);
ScopedLock L1(Region->MMLock);
ScopedLock L2(Region->FLLock);
ScopedLock MM(Region->MMLock);
ScopedFLLock FL(Region->FLLock, Region);
getStats(Str, I, Region);
}
}
@ -1202,7 +1259,7 @@ void SizeClassAllocator64<Config>::getRegionFragmentationInfo(
SinglyLinkedList<BatchGroupT> GroupsToRelease;
{
ScopedLock L(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
GroupsToRelease = Region->FreeListInfo.BlockList;
Region->FreeListInfo.BlockList.clear();
}
@ -1218,7 +1275,7 @@ void SizeClassAllocator64<Config>::getRegionFragmentationInfo(
mergeGroupsToReleaseBack(Region, GroupsToRelease);
}
ScopedLock L(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
const uptr PageSize = getPageSizeCached();
const uptr TotalBlocks = Region->MemMapInfo.AllocatedUser / BlockSize;
const uptr InUseBlocks =
@ -1250,7 +1307,7 @@ void SizeClassAllocator64<Config>::getMemoryGroupFragmentationInfoInRegion(
SinglyLinkedList<BatchGroupT> GroupsToRelease;
{
ScopedLock L(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
GroupsToRelease = Region->FreeListInfo.BlockList;
Region->FreeListInfo.BlockList.clear();
}
@ -1414,7 +1471,7 @@ uptr SizeClassAllocator64<Config>::releaseToOSMaybe(RegionInfo *Region,
SinglyLinkedList<BatchGroupT> GroupsToRelease;
{
ScopedLock L(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
BytesInFreeList =
Region->MemMapInfo.AllocatedUser - (Region->FreeListInfo.PoppedBlocks -
@ -1872,7 +1929,7 @@ template <typename Config>
void SizeClassAllocator64<Config>::mergeGroupsToReleaseBack(
RegionInfo *Region, SinglyLinkedList<BatchGroupT> &GroupsToRelease)
REQUIRES(Region->MMLock) EXCLUDES(Region->FLLock) {
ScopedLock L(Region->FLLock);
ScopedFLLock FL(Region->FLLock, Region);
// After merging two freelists, we may have redundant `BatchGroup`s that
// need to be recycled. The number of unused `BatchGroup`s is expected to be
@ -1949,10 +2006,8 @@ void SizeClassAllocator64<Config>::mergeGroupsToReleaseBack(
const u32 NeededSlots = UnusedBatch == nullptr ? 1U : 2U;
if (UNLIKELY(Idx + NeededSlots > MaxUnusedSize)) {
ScopedLock L(BatchClassRegion->FLLock);
ScopedFLLock FL(BatchClassRegion->FLLock, BatchClassRegion);
pushBatchClassBlocks(BatchClassRegion, Blocks, Idx);
if (conditionVariableEnabled())
BatchClassRegion->FLLockCV.notifyAll(BatchClassRegion->FLLock);
Idx = 0;
}
Blocks[Idx++] =
@ -1985,10 +2040,8 @@ void SizeClassAllocator64<Config>::mergeGroupsToReleaseBack(
}
if (Idx != 0) {
ScopedLock L(BatchClassRegion->FLLock);
ScopedFLLock FL(BatchClassRegion->FLLock, BatchClassRegion);
pushBatchClassBlocks(BatchClassRegion, Blocks, Idx);
if (conditionVariableEnabled())
BatchClassRegion->FLLockCV.notifyAll(BatchClassRegion->FLLock);
}
if (SCUDO_DEBUG) {
@ -1998,10 +2051,8 @@ void SizeClassAllocator64<Config>::mergeGroupsToReleaseBack(
CHECK_LT(Prev->CompactPtrGroupBase, Cur->CompactPtrGroupBase);
}
}
if (conditionVariableEnabled())
Region->FLLockCV.notifyAll(Region->FLLock);
}
} // namespace scudo
#endif // SCUDO_PRIMARY64_H_

10
compiler-rt/lib/scudo/standalone/tests/combined_test.cpp
View file

@ -560,7 +560,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, IterateOverChunks) {
EXPECT_NE(std::find(V->begin(), V->end(), P), V->end());
},
reinterpret_cast<void *>(&V));
Allocator->enable();
Allocator->enable(/*IsChild*/ false);
for (auto P : V)
Allocator->deallocate(P, Origin);
}
@ -1348,7 +1348,7 @@ void VerifyIterateOverUsableSize(AllocatorT &Allocator) {
(*Pointers)[reinterpret_cast<void *>(Base)] = Size;
},
reinterpret_cast<void *>(&Pointers));
Allocator.enable();
Allocator.enable(/*IsChild*/ false);
for (auto [Ptr, IterateSize] : Pointers) {
EXPECT_NE(0U, IterateSize)
@ -1538,7 +1538,7 @@ TEST(ScudoCombinedTest, QuarantineIterateOverChunks) {
(*Pointers)[Base] = Size;
},
reinterpret_cast<void *>(&Pointers));
Allocator->enable();
Allocator->enable(/*IsChild*/ false);
for (const auto [Base, Size] : Pointers) {
EXPECT_TRUE(false) << "Unexpected pointer found in iterateOverChunks "
@ -2206,7 +2206,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, AllocAfterFork) {
pid_t Pid = fork();
if (Pid == 0) {
// Child process: enable the allocator and allocate.
Allocator->enable();
Allocator->enable(/*IsChild*/ true);
for (size_t SizeLog = 3; SizeLog <= 20; SizeLog++) {
const scudo::uptr Size = 1UL << SizeLog;
void *P = Allocator->allocate(Size, Origin);
@ -2221,7 +2221,7 @@ SCUDO_TYPED_TEST(ScudoCombinedTest, AllocAfterFork) {
_exit(10);
}
// Parent process: enable the allocator and wait for child.
Allocator->enable();
Allocator->enable(/*IsChild*/ false);
EXPECT_NE(-1, Pid);
int Status;
EXPECT_EQ(Pid, waitpid(Pid, &Status, 0));

2
compiler-rt/lib/scudo/standalone/tests/primary_test.cpp
View file

@ -354,7 +354,7 @@ SCUDO_TYPED_TEST(ScudoPrimaryTest, PrimaryIterate) {
};
Allocator->disable();
Allocator->iterateOverBlocks(Lambda);
Allocator->enable();
Allocator->enable(/*IsChild*/ false);
EXPECT_EQ(Found, V.size());
while (!V.empty()) {
auto Pair = V.back();

12
compiler-rt/lib/scudo/standalone/wrappers_c.cpp
View file

@ -284,14 +284,18 @@ INTERFACE WEAK int SCUDO_PREFIX(malloc_iterate)(
return 0;
}
INTERFACE WEAK void SCUDO_PREFIX(malloc_enable)() { Allocator.enable(); }
INTERFACE WEAK void SCUDO_PREFIX(malloc_enable)() {
Allocator.enable(/*IsChild*/ false);
}
INTERFACE WEAK void SCUDO_PREFIX(malloc_enable_child)() {
Allocator.enable(/*IsChild*/ true);
}
INTERFACE WEAK void SCUDO_PREFIX(malloc_disable)() { Allocator.disable(); }
void SCUDO_PREFIX(malloc_postinit)() {
Allocator.initGwpAsan();
pthread_atfork(SCUDO_PREFIX(malloc_disable), SCUDO_PREFIX(malloc_enable),
SCUDO_PREFIX(malloc_enable));
SCUDO_PREFIX(malloc_enable_child));
}
INTERFACE WEAK int SCUDO_PREFIX(mallopt)(int param, int value) {
@ -380,7 +384,7 @@ INTERFACE WEAK int SCUDO_PREFIX(malloc_info)(UNUSED int options, FILE *stream) {
Allocator.disable();
Allocator.iterateOverChunks(0, -1ul, callback, sizes);
Allocator.enable();
Allocator.enable(/*IsChild*/ false);
fputs("<malloc version=\"scudo-1\">\n", stream);
for (scudo::uptr i = 0; i != max_size; ++i)