[llvm] r369975 - [ORC] Fix an overly aggressive assert.

[Lang Hames via llvm-commits]

Author: lhames
Date: Mon Aug 26 14:42:47 2019
New Revision: 369975

URL: http://llvm.org/viewvc/llvm-project?rev=369975&view=rev
Log:
[ORC] Fix an overly aggressive assert.

Symbols that have not been queried will not have MaterializingInfo entries,
so remove the assert that all failed symbols should have these entries.
Also updates the loop to only remove entries that were found earlier.

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=369975&r1=369974&r2=369975&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp Mon Aug 26 14:42:47 2019
@@ -1187,6 +1187,8 @@ void JITDylib::notifyFailed(const Symbol
   AsynchronousSymbolQuerySet FailedQueries;
 
   ES.runSessionLocked([&]() {
+    std::vector<const SymbolStringPtr *> MaterializerNamesToFail;
+
     for (auto &KV : FailedSymbols) {
       auto &Name = KV.first;
 
@@ -1207,6 +1209,7 @@ void JITDylib::notifyFailed(const Symbol
         continue;
 
       auto &MI = MII->second;
+      MaterializerNamesToFail.push_back(&KV.first);
 
       // Move all dependants to the error state and disconnect from them.
       for (auto &KV : MI.Dependants) {
@@ -1261,9 +1264,9 @@ void JITDylib::notifyFailed(const Symbol
       Q->detach();
 
     // Remove the MaterializingInfos.
-    for (auto &KV : FailedSymbols) {
-      assert(MaterializingInfos.count(KV.first) && "Expected MI for Name");
-      MaterializingInfos.erase(KV.first);
+    while (!MaterializerNamesToFail.empty()) {
+      MaterializingInfos.erase(*MaterializerNamesToFail.back());
+      MaterializerNamesToFail.pop_back();
     }
   });
 

Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp?rev=369975&r1=369974&r2=369975&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp Mon Aug 26 14:42:47 2019
@@ -718,6 +718,35 @@ TEST_F(CoreAPIsStandardTest, AddDependen
       << "Lookup on failed symbol should fail";
 }
 
+TEST_F(CoreAPIsStandardTest, FailMaterializerWithUnqueriedSymbols) {
+  // Make sure that symbols with no queries aganist them still
+  // fail correctly.
+
+  bool MaterializerRun = false;
+  auto MU = std::make_unique<SimpleMaterializationUnit>(
+      SymbolFlagsMap(
+          {{Foo, JITSymbolFlags::Exported}, {Bar, JITSymbolFlags::Exported}}),
+      [&](MaterializationResponsibility R) {
+        MaterializerRun = true;
+        R.failMaterialization();
+      });
+
+  cantFail(JD.define(std::move(MU)));
+
+  // Issue a query for Foo, but not bar.
+  EXPECT_THAT_EXPECTED(ES.lookup({&JD}, {Foo}), Failed())
+      << "Expected lookup to fail.";
+
+  // Check that the materializer (and therefore failMaterialization) ran.
+  EXPECT_TRUE(MaterializerRun) << "Expected materializer to have run by now";
+
+  // Check that subsequent queries against both symbols fail.
+  EXPECT_THAT_EXPECTED(ES.lookup({&JD}, {Foo}), Failed())
+      << "Expected lookup for Foo to fail.";
+  EXPECT_THAT_EXPECTED(ES.lookup({&JD}, {Bar}), Failed())
+      << "Expected lookup for Bar to fail.";
+}
+
 TEST_F(CoreAPIsStandardTest, DropMaterializerWhenEmpty) {
   bool DestructorRun = false;