[llvm] e2f86b5 - [ORC][MachO] Remove misused MachOPlatform::BootstrapInfo::Mutex member.

[Lang Hames via llvm-commits]

Author: Lang Hames
Date: 2025-05-29T17:35:26+10:00
New Revision: e2f86b5584959ec2b000d183841c8fb7c3402388

URL: https://github.com/llvm/llvm-project/commit/e2f86b5584959ec2b000d183841c8fb7c3402388
DIFF: https://github.com/llvm/llvm-project/commit/e2f86b5584959ec2b000d183841c8fb7c3402388.diff

LOG: [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.

Added: 
    

Modified: 
    llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
    llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h b/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
index b1b27fb8066bb..e764af267cdfa 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/MachOPlatform.h
@@ -194,7 +194,6 @@ class MachOPlatform : public Platform {
 
   // Data needed for bootstrap only.
   struct BootstrapInfo {
-    std::mutex Mutex;
     std::condition_variable CV;
     size_t ActiveGraphs = 0;
     shared::AllocActions DeferredAAs;

diff  --git a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
index 369a047f65076..6a5fb51cd6d5e 100644
--- a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
@@ -570,7 +570,7 @@ MachOPlatform::MachOPlatform(
 
   // Step (3) Wait for any incidental linker work to complete.
   {
-    std::unique_lock<std::mutex> Lock(BI.Mutex);
+    std::unique_lock<std::mutex> Lock(PlatformMutex);
     BI.CV.wait(Lock, [&]() { return BI.ActiveGraphs == 0; });
     Bootstrap = nullptr;
   }
@@ -955,7 +955,7 @@ Error MachOPlatform::MachOPlatformPlugin::
 
 Error MachOPlatform::MachOPlatformPlugin::bootstrapPipelineEnd(
     jitlink::LinkGraph &G) {
-  std::lock_guard<std::mutex> Lock(MP.Bootstrap->Mutex);
+  std::lock_guard<std::mutex> Lock(MP.PlatformMutex);
 
   --MP.Bootstrap->ActiveGraphs;
   // Notify Bootstrap->CV while holding the mutex because the mutex is
@@ -1441,7 +1441,7 @@ Error MachOPlatform::MachOPlatformPlugin::registerObjectPlatformSections(
     if (LLVM_LIKELY(!InBootstrapPhase))
       G.allocActions().push_back(std::move(AllocActions));
     else {
-      std::lock_guard<std::mutex> Lock(MP.Bootstrap->Mutex);
+      std::lock_guard<std::mutex> Lock(MP.PlatformMutex);
       MP.Bootstrap->DeferredAAs.push_back(std::move(AllocActions));
     }
   }
@@ -1716,7 +1716,7 @@ Error MachOPlatform::MachOPlatformPlugin::addSymbolTableRegistration(
     // If we're in the bootstrap phase then just record these symbols in the
     // bootstrap object and then bail out -- registration will be attached to
     // the bootstrap graph.
-    std::lock_guard<std::mutex> Lock(MP.Bootstrap->Mutex);
+    std::lock_guard<std::mutex> Lock(MP.PlatformMutex);
     auto &SymTab = MP.Bootstrap->SymTab;
     for (auto &[OriginalSymbol, NameSym] : JITSymTabInfo)
       SymTab.push_back({NameSym->getAddress(), OriginalSymbol->getAddress(),