[llvm] r369809 - [ORC] Remove query dependencies when symbols are resolved.

[Lang Hames via llvm-commits]

Author: lhames
Date: Fri Aug 23 13:37:32 2019
New Revision: 369809

URL: http://llvm.org/viewvc/llvm-project?rev=369809&view=rev
Log:
[ORC] Remove query dependencies when symbols are resolved.

If the dependencies are not removed then a late failure (one symbol covered by
the query failing after others have already been resolved) can result in an
attempt to detach the query from already finalized symbol, resulting in an
assert/crash. This patch fixes the issue by removing query dependencies in
JITDylib::resolve for symbols that meet the required state.

Modified:
    llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
    llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp

Modified: llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp?rev=369809&r1=369808&r2=369809&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp Fri Aug 23 13:37:32 2019
@@ -1031,6 +1031,7 @@ Error JITDylib::resolve(const SymbolMap
       auto &MI = MaterializingInfos[Name];
       for (auto &Q : MI.takeQueriesMeeting(SymbolState::Resolved)) {
         Q->notifySymbolMetRequiredState(Name, ResolvedSym);
+        Q->removeQueryDependence(*this, Name);
         if (Q->isComplete())
           CompletedQueries.insert(std::move(Q));
       }

Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp?rev=369809&r1=369808&r2=369809&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp Fri Aug 23 13:37:32 2019
@@ -935,7 +935,7 @@ TEST_F(CoreAPIsStandardTest, FailResolut
   }
 }
 
-TEST_F(CoreAPIsStandardTest, FailEmissionEarly) {
+TEST_F(CoreAPIsStandardTest, FailEmissionAfterResolution) {
 
   cantFail(JD.define(absoluteSymbols({{Baz, BazSym}})));
 
@@ -970,6 +970,30 @@ TEST_F(CoreAPIsStandardTest, FailEmissio
       << "Unexpected success while trying to test error propagation";
 }
 
+TEST_F(CoreAPIsStandardTest, FailAfterPartialResolution) {
+
+  cantFail(JD.define(absoluteSymbols({{Foo, FooSym}})));
+
+  // Fail materialization of bar.
+  auto BarMU = std::make_unique<SimpleMaterializationUnit>(
+      SymbolFlagsMap({{Bar, BarSym.getFlags()}}),
+      [&](MaterializationResponsibility R) { R.failMaterialization(); });
+
+  cantFail(JD.define(std::move(BarMU)));
+
+  bool QueryHandlerRun = false;
+  ES.lookup(
+      JITDylibSearchList({{&JD, false}}), SymbolNameSet({Foo, Bar}),
+      SymbolState::Resolved,
+      [&](Expected<SymbolMap> Result) {
+        EXPECT_THAT_EXPECTED(std::move(Result), Failed())
+            << "Expected query to fail";
+        QueryHandlerRun = true;
+      },
+      NoDependenciesToRegister);
+  EXPECT_TRUE(QueryHandlerRun) << "Query handler never ran";
+}
+
 TEST_F(CoreAPIsStandardTest, TestLookupWithUnthreadedMaterialization) {
   auto MU = std::make_unique<SimpleMaterializationUnit>(
       SymbolFlagsMap({{Foo, JITSymbolFlags::Exported}}),