Skip to content

Commit e2f86b5

Browse files
committed
[ORC][MachO] Remove misused MachOPlatform::BootstrapInfo::Mutex member.
MachOPlatform::BootstrapInfo::Mutex was meant to be used to synchronize access to the MachOPlatform::BootstrapInfo::ActiveGraphs member, but the latter was also modified under the MachOPlatform::PlatformMutex (in MachOPlatform::MachOPlatformPlugin::modifyPassConfig), leading to a data race. There have been external reports (rdar://151041549) of deadlocks on the MachOPlatform::BootstrapInfo::CV condition variable that are consistent with corruption of the ActiveGraphs member (though alternative explanations for the reported behavior exist, and it has been too rare in practice to verify). This patch removes the misused MachOPlatform::BootstrapInfo::Mutex member and synchronizes all accesses to ActiveGraphs using MachOPlatform::PlatformMutex instead. Since ActiveGraphs is only used during bootstrap the performance impact of this should be negligible. rdar://151041549 - possible fix.
1 parent a773356 commit e2f86b5

File tree

2 files changed

+4
-5
lines changed

2 files changed

+4
-5
lines changed

llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,6 @@ class MachOPlatform : public Platform {
194194

195195
// Data needed for bootstrap only.
196196
struct BootstrapInfo {
197-
std::mutex Mutex;
198197
std::condition_variable CV;
199198
size_t ActiveGraphs = 0;
200199
shared::AllocActions DeferredAAs;

llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ MachOPlatform::MachOPlatform(
570570

571571
// Step (3) Wait for any incidental linker work to complete.
572572
{
573-
std::unique_lock<std::mutex> Lock(BI.Mutex);
573+
std::unique_lock<std::mutex> Lock(PlatformMutex);
574574
BI.CV.wait(Lock, [&]() { return BI.ActiveGraphs == 0; });
575575
Bootstrap = nullptr;
576576
}
@@ -955,7 +955,7 @@ Error MachOPlatform::MachOPlatformPlugin::
955955

956956
Error MachOPlatform::MachOPlatformPlugin::bootstrapPipelineEnd(
957957
jitlink::LinkGraph &G) {
958-
std::lock_guard<std::mutex> Lock(MP.Bootstrap->Mutex);
958+
std::lock_guard<std::mutex> Lock(MP.PlatformMutex);
959959

960960
--MP.Bootstrap->ActiveGraphs;
961961
// Notify Bootstrap->CV while holding the mutex because the mutex is
@@ -1441,7 +1441,7 @@ Error MachOPlatform::MachOPlatformPlugin::registerObjectPlatformSections(
14411441
if (LLVM_LIKELY(!InBootstrapPhase))
14421442
G.allocActions().push_back(std::move(AllocActions));
14431443
else {
1444-
std::lock_guard<std::mutex> Lock(MP.Bootstrap->Mutex);
1444+
std::lock_guard<std::mutex> Lock(MP.PlatformMutex);
14451445
MP.Bootstrap->DeferredAAs.push_back(std::move(AllocActions));
14461446
}
14471447
}
@@ -1716,7 +1716,7 @@ Error MachOPlatform::MachOPlatformPlugin::addSymbolTableRegistration(
17161716
// If we're in the bootstrap phase then just record these symbols in the
17171717
// bootstrap object and then bail out -- registration will be attached to
17181718
// the bootstrap graph.
1719-
std::lock_guard<std::mutex> Lock(MP.Bootstrap->Mutex);
1719+
std::lock_guard<std::mutex> Lock(MP.PlatformMutex);
17201720
auto &SymTab = MP.Bootstrap->SymTab;
17211721
for (auto &[OriginalSymbol, NameSym] : JITSymTabInfo)
17221722
SymTab.push_back({NameSym->getAddress(), OriginalSymbol->getAddress(),

0 commit comments

Comments
 (0)