[llvm] a671cee - [ORC] Fix an EDU-update bug in ExecutionSession::IL_failSymbols.

[Lang Hames via llvm-commits]

Author: Lang Hames
Date: 2024-04-05T11:14:06-05:00
New Revision: a671ceec3343063a4e6f45c231791ff925abedb5

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

LOG: [ORC] Fix an EDU-update bug in ExecutionSession::IL_failSymbols.

We were catching a local variable, SymMI, by value instead of by reference
during EDU cleanup and this was leaving the dependence graph in an
inconsistent state that could lead to crashes on subsequent emits. Fixing this
bug required us to also avoid aliasing between SymMI and MI (which would have
caused cleanup to clear the MI.DependantEDUs set that we're iterating over).

No testcase: the crash only triggered in very specific circumstances
(including concurrent linking) in an out-of-tree ORC client. I'm working on a
session state verifier that could be turned on when compiling with
expensive-checks turned on and that should help us catch issues like this in
the future.

rdar://125164262

Coding my way home: 0.89527S, 89.61313W

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp
index 37c32f86e8d873..60265c4f72ff9f 100644
--- a/llvm/lib/ExecutionEngine/Orc/Core.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp
@@ -3379,12 +3379,17 @@ ExecutionSession::IL_failSymbols(JITDylib &JD,
       for (auto &DependantEDU : MI.DependantEDUs) {
 
         // Remove DependantEDU from all of its users DependantEDUs lists.
-        for (auto &[JD, Syms] : DependantEDU->Dependencies) {
-          for (auto Sym : Syms) {
-            assert(JD->Symbols.count(SymbolStringPtr(Sym)) && "Sym not in JD?");
-            assert(JD->MaterializingInfos.count(SymbolStringPtr(Sym)) &&
+        for (auto &[DepJD, DepSyms] : DependantEDU->Dependencies) {
+          for (auto DepSym : DepSyms) {
+            // Skip self-reference to avoid invalidating the MI.DependantEDUs
+            // map. We'll clear this later.
+            if (DepJD == &JD && DepSym == Name)
+              continue;
+            assert(DepJD->Symbols.count(SymbolStringPtr(DepSym)) &&
+                   "DepSym not in DepJD?");
+            assert(DepJD->MaterializingInfos.count(SymbolStringPtr(DepSym)) &&
                    "DependantEDU not registered with symbol it depends on");
-            auto SymMI = JD->MaterializingInfos[SymbolStringPtr(Sym)];
+            auto &SymMI = DepJD->MaterializingInfos[SymbolStringPtr(DepSym)];
             assert(SymMI.DependantEDUs.count(DependantEDU) &&
                    "DependantEDU missing from DependantEDUs list");
             SymMI.DependantEDUs.erase(DependantEDU);