[llvm] a9e75b1 - [ORC][MachO] Fix race condition during MachOPlatform bootstrap.

[Lang Hames via llvm-commits]

Author: Lang Hames
Date: 2024-08-09T17:27:15+10:00
New Revision: a9e75b1d4d18d09a63f120df4781013c1866b4ff

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

LOG: [ORC][MachO] Fix race condition during MachOPlatform bootstrap.

In 93509b4462a74 MachOPlatform was updated to store object symbols in a shared
vector during bootstrap (this table is later attached to the
bootstrap-completion graph once the ORC runtime's symbol table registration
code is ready). The shared vector was not guarded with a mutex, so use of a
concurrent dispatcher could lead to races during bootstrap. This commit fixes
the issue by guarding access to the table with the BootstrapInfo mutex.

No testcase since this only manifests rarely when both a concurrent dispatcher
and the ORC runtime are used. Once we add a concurrent dispatcher option to
llvm-jitlink we may be able to test this with a regression test in the ORC
runtime and TSan enabled.

rdar://133520308

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
index 0d117f7cf8734f..a71afe1a3162f0 100644
--- a/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/MachOPlatform.cpp
@@ -1697,22 +1697,24 @@ Error MachOPlatform::MachOPlatformPlugin::addSymbolTableRegistration(
     HeaderAddr = I->second;
   }
 
-  SymbolTableVector LocalSymTab;
-  auto &SymTab = LLVM_LIKELY(!InBootstrapPhase) ? LocalSymTab
-                                                : MP.Bootstrap.load()->SymTab;
+  if (LLVM_UNLIKELY(InBootstrapPhase)) {
+    // 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.load()->Mutex);
+    auto &SymTab = MP.Bootstrap.load()->SymTab;
+    for (auto &[OriginalSymbol, NameSym] : JITSymTabInfo)
+      SymTab.push_back({NameSym->getAddress(), OriginalSymbol->getAddress(),
+                        flagsForSymbol(*OriginalSymbol)});
+    return Error::success();
+  }
+
+  SymbolTableVector SymTab;
   for (auto &[OriginalSymbol, NameSym] : JITSymTabInfo)
     SymTab.push_back({NameSym->getAddress(), OriginalSymbol->getAddress(),
                       flagsForSymbol(*OriginalSymbol)});
 
-  // Bail out if we're in the bootstrap phase -- registration of thees symbols
-  // will be attached to the bootstrap graph.
-  if (LLVM_UNLIKELY(InBootstrapPhase))
-    return Error::success();
-
-  shared::AllocActions &allocActions = LLVM_LIKELY(!InBootstrapPhase)
-                                           ? G.allocActions()
-                                           : MP.Bootstrap.load()->DeferredAAs;
-  allocActions.push_back(
+  G.allocActions().push_back(
       {cantFail(WrapperFunctionCall::Create<SPSRegisterSymbolsArgs>(
            MP.RegisterObjectSymbolTable.Addr, HeaderAddr, SymTab)),
        cantFail(WrapperFunctionCall::Create<SPSRegisterSymbolsArgs>(