[llvm] r369808 - [ORC] Fix a FIXME: Propagate errors to dependencies.

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 23 13:37:31 PDT 2019


Author: lhames
Date: Fri Aug 23 13:37:31 2019
New Revision: 369808

URL: http://llvm.org/viewvc/llvm-project?rev=369808&view=rev
Log:
[ORC] Fix a FIXME: Propagate errors to dependencies.

When symbols are failed (via MaterializationResponsibility::failMaterialization)
any symbols depending on them will now be moved to an error state. Attempting
to resolve or emit a symbol in the error state (via the notifyResolved or
notifyEmitted methods on MaterializationResponsibility) will result in an error.
If notifyResolved or notifyEmitted return an error due to failure of a
dependence then the caller should log or discard the error and call
failMaterialization to propagate the failure to any queries waiting on the
symbols being resolved/emitted (plus their dependencies).

Modified:
    llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h
    llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
    llvm/trunk/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
    llvm/trunk/lib/ExecutionEngine/Orc/LazyReexports.cpp
    llvm/trunk/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
    llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
    llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
    llvm/trunk/unittests/ExecutionEngine/Orc/LazyCallThroughAndReexportsTest.cpp

Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h?rev=369808&r1=369807&r2=369808&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/Core.h Fri Aug 23 13:37:31 2019
@@ -123,13 +123,13 @@ class FailedToMaterialize : public Error
 public:
   static char ID;
 
-  FailedToMaterialize(SymbolNameSet Symbols);
+  FailedToMaterialize(std::shared_ptr<SymbolDependenceMap> Symbols);
   std::error_code convertToErrorCode() const override;
   void log(raw_ostream &OS) const override;
-  const SymbolNameSet &getSymbols() const { return Symbols; }
+  const SymbolDependenceMap &getSymbols() const { return *Symbols; }
 
 private:
-  SymbolNameSet Symbols;
+  std::shared_ptr<SymbolDependenceMap> Symbols;
 };
 
 /// Used to notify clients when symbols can not be found during a lookup.
@@ -204,12 +204,26 @@ public:
   /// symbols must be ones covered by this MaterializationResponsibility
   /// instance. Individual calls to this method may resolve a subset of the
   /// symbols, but all symbols must have been resolved prior to calling emit.
-  void notifyResolved(const SymbolMap &Symbols);
+  ///
+  /// This method will return an error if any symbols being resolved have been
+  /// moved to the error state due to the failure of a dependency. If this
+  /// method returns an error then clients should log it and call
+  /// failMaterialize. If no dependencies have been registered for the
+  /// symbols covered by this MaterializationResponsibiility then this method
+  /// is guaranteed to return Error::success() and can be wrapped with cantFail.
+  Error notifyResolved(const SymbolMap &Symbols);
 
   /// Notifies the target JITDylib (and any pending queries on that JITDylib)
   /// that all symbols covered by this MaterializationResponsibility instance
   /// have been emitted.
-  void notifyEmitted();
+  ///
+  /// This method will return an error if any symbols being resolved have been
+  /// moved to the error state due to the failure of a dependency. If this
+  /// method returns an error then clients should log it and call
+  /// failMaterialize. If no dependencies have been registered for the
+  /// symbols covered by this MaterializationResponsibiility then this method
+  /// is guaranteed to return Error::success() and can be wrapped with cantFail.
+  Error notifyEmitted();
 
   /// Adds new symbols to the JITDylib and this responsibility instance.
   ///        JITDylib entries start out in the materializing state.
@@ -628,11 +642,13 @@ private:
     void addQuery(std::shared_ptr<AsynchronousSymbolQuery> Q);
     void removeQuery(const AsynchronousSymbolQuery &Q);
     AsynchronousSymbolQueryList takeQueriesMeeting(SymbolState RequiredState);
+    AsynchronousSymbolQueryList takeAllPendingQueries() {
+      return std::move(PendingQueries);
+    }
     bool hasQueriesPending() const { return !PendingQueries.empty(); }
     const AsynchronousSymbolQueryList &pendingQueries() const {
       return PendingQueries;
     }
-
   private:
     AsynchronousSymbolQueryList PendingQueries;
   };
@@ -699,9 +715,9 @@ private:
                    SymbolNameSet &Unresolved, bool MatchNonExported,
                    MaterializationUnitList &MUs);
 
-  void lodgeQueryImpl(std::shared_ptr<AsynchronousSymbolQuery> &Q,
-                      SymbolNameSet &Unresolved, bool MatchNonExported,
-                      MaterializationUnitList &MUs);
+  Error lodgeQueryImpl(std::shared_ptr<AsynchronousSymbolQuery> &Q,
+                       SymbolNameSet &Unresolved, bool MatchNonExported,
+                       MaterializationUnitList &MUs);
 
   bool lookupImpl(std::shared_ptr<AsynchronousSymbolQuery> &Q,
                   std::vector<std::unique_ptr<MaterializationUnit>> &MUs,
@@ -720,14 +736,21 @@ private:
 
   SymbolNameSet getRequestedSymbols(const SymbolFlagsMap &SymbolFlags) const;
 
+  // Move a symbol to the failure state.
+  // Detaches the symbol from all dependencies, moves all dependants to the
+  // error state (but does not fail them), deletes the MaterializingInfo for
+  // the symbol (if present) and returns the set of queries that need to be
+  // notified of the failure.
+  AsynchronousSymbolQuerySet failSymbol(const SymbolStringPtr &Name);
+
   void addDependencies(const SymbolStringPtr &Name,
                        const SymbolDependenceMap &Dependants);
 
-  void resolve(const SymbolMap &Resolved);
+  Error resolve(const SymbolMap &Resolved);
 
-  void emit(const SymbolFlagsMap &Emitted);
+  Error emit(const SymbolFlagsMap &Emitted);
 
-  void notifyFailed(const SymbolNameSet &FailedSymbols);
+  void notifyFailed(const SymbolFlagsMap &FailedSymbols);
 
   ExecutionSession &ES;
   std::string JITDylibName;

Modified: llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp?rev=369808&r1=369807&r2=369808&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/Core.cpp Fri Aug 23 13:37:31 2019
@@ -151,6 +151,8 @@ raw_ostream &operator<<(raw_ostream &OS,
 }
 
 raw_ostream &operator<<(raw_ostream &OS, const JITSymbolFlags &Flags) {
+  if (Flags.hasError())
+    OS << "[*ERROR*]";
   if (Flags.isCallable())
     OS << "[Callable]";
   else
@@ -244,9 +246,10 @@ raw_ostream &operator<<(raw_ostream &OS,
   llvm_unreachable("Invalid state");
 }
 
-FailedToMaterialize::FailedToMaterialize(SymbolNameSet Symbols)
+FailedToMaterialize::FailedToMaterialize(
+    std::shared_ptr<SymbolDependenceMap> Symbols)
     : Symbols(std::move(Symbols)) {
-  assert(!this->Symbols.empty() && "Can not fail to resolve an empty set");
+  assert(!this->Symbols->empty() && "Can not fail to resolve an empty set");
 }
 
 std::error_code FailedToMaterialize::convertToErrorCode() const {
@@ -254,7 +257,7 @@ std::error_code FailedToMaterialize::con
 }
 
 void FailedToMaterialize::log(raw_ostream &OS) const {
-  OS << "Failed to materialize symbols: " << Symbols;
+  OS << "Failed to materialize symbols: " << *Symbols;
 }
 
 SymbolsNotFound::SymbolsNotFound(SymbolNameSet Symbols)
@@ -367,7 +370,7 @@ SymbolNameSet MaterializationResponsibil
   return JD.getRequestedSymbols(SymbolFlags);
 }
 
-void MaterializationResponsibility::notifyResolved(const SymbolMap &Symbols) {
+Error MaterializationResponsibility::notifyResolved(const SymbolMap &Symbols) {
   LLVM_DEBUG({
     dbgs() << "In " << JD.getName() << " resolving " << Symbols << "\n";
   });
@@ -385,17 +388,20 @@ void MaterializationResponsibility::noti
   }
 #endif
 
-  JD.resolve(Symbols);
+  return JD.resolve(Symbols);
 }
 
-void MaterializationResponsibility::notifyEmitted() {
+Error MaterializationResponsibility::notifyEmitted() {
 
   LLVM_DEBUG({
     dbgs() << "In " << JD.getName() << " emitting " << SymbolFlags << "\n";
   });
 
-  JD.emit(SymbolFlags);
+  if (auto Err = JD.emit(SymbolFlags))
+    return Err;
+
   SymbolFlags.clear();
+  return Error::success();
 }
 
 Error MaterializationResponsibility::defineMaterializing(
@@ -417,11 +423,7 @@ void MaterializationResponsibility::fail
            << SymbolFlags << "\n";
   });
 
-  SymbolNameSet FailedSymbols;
-  for (auto &KV : SymbolFlags)
-    FailedSymbols.insert(KV.first);
-
-  JD.notifyFailed(FailedSymbols);
+  JD.notifyFailed(SymbolFlags);
   SymbolFlags.clear();
 }
 
@@ -485,8 +487,9 @@ StringRef AbsoluteSymbolsMaterialization
 
 void AbsoluteSymbolsMaterializationUnit::materialize(
     MaterializationResponsibility R) {
-  R.notifyResolved(Symbols);
-  R.notifyEmitted();
+  // No dependencies, so these calls can't fail.
+  cantFail(R.notifyResolved(Symbols));
+  cantFail(R.notifyEmitted());
 }
 
 void AbsoluteSymbolsMaterializationUnit::discard(const JITDylib &JD,
@@ -625,6 +628,7 @@ void ReExportsMaterializationUnit::mater
     };
 
     auto OnComplete = [QueryInfo](Expected<SymbolMap> Result) {
+      auto &ES = QueryInfo->R.getTargetJITDylib().getExecutionSession();
       if (Result) {
         SymbolMap ResolutionMap;
         for (auto &KV : QueryInfo->Aliases) {
@@ -633,10 +637,17 @@ void ReExportsMaterializationUnit::mater
           ResolutionMap[KV.first] = JITEvaluatedSymbol(
               (*Result)[KV.second.Aliasee].getAddress(), KV.second.AliasFlags);
         }
-        QueryInfo->R.notifyResolved(ResolutionMap);
-        QueryInfo->R.notifyEmitted();
+        if (auto Err = QueryInfo->R.notifyResolved(ResolutionMap)) {
+          ES.reportError(std::move(Err));
+          QueryInfo->R.failMaterialization();
+          return;
+        }
+        if (auto Err = QueryInfo->R.notifyEmitted()) {
+          ES.reportError(std::move(Err));
+          QueryInfo->R.failMaterialization();
+          return;
+        }
       } else {
-        auto &ES = QueryInfo->R.getTargetJITDylib().getExecutionSession();
         ES.reportError(Result.takeError());
         QueryInfo->R.failMaterialization();
       }
@@ -830,29 +841,115 @@ JITDylib::getRequestedSymbols(const Symb
   });
 }
 
+JITDylib::AsynchronousSymbolQuerySet
+JITDylib::failSymbol(const SymbolStringPtr &Name) {
+  assert(Symbols.count(Name) && "Name not in symbol table");
+  assert(Symbols[Name].getFlags().hasError() &&
+         "Failing symbol not in the error state");
+
+  auto MII = MaterializingInfos.find(Name);
+  if (MII == MaterializingInfos.end())
+    return AsynchronousSymbolQuerySet();
+
+  auto &MI = MII->second;
+
+  // Visit all dependants.
+  for (auto &KV : MI.Dependants) {
+    auto &DependantJD = *KV.first;
+    for (auto &DependantName : KV.second) {
+      assert(DependantJD.Symbols.count(DependantName) &&
+             "No symbol with DependantName in DependantJD");
+      auto &DependantSymTabEntry = DependantJD.Symbols[DependantName];
+      DependantSymTabEntry.setFlags(DependantSymTabEntry.getFlags() |
+                                    JITSymbolFlags::HasError);
+
+      assert(DependantJD.MaterializingInfos.count(DependantName) &&
+             "Dependant symbol does not have MaterializingInfo?");
+      auto &DependantMI = DependantJD.MaterializingInfos[DependantName];
+
+      assert(DependantMI.UnemittedDependencies.count(this) &&
+             "No unemitted dependency recorded for this JD?");
+      auto UnemittedDepsI = DependantMI.UnemittedDependencies.find(this);
+      assert(UnemittedDepsI != DependantMI.UnemittedDependencies.end() &&
+             "No unemitted dependency on this JD");
+      assert(UnemittedDepsI->second.count(Name) &&
+             "No unemitted dependency on symbol Name in this JD");
+
+      UnemittedDepsI->second.erase(Name);
+      if (UnemittedDepsI->second.empty())
+        DependantMI.UnemittedDependencies.erase(UnemittedDepsI);
+    }
+  }
+
+  // Visit all unemitted dependencies and disconnect from them.
+  for (auto &KV : MI.UnemittedDependencies) {
+    auto &DependencyJD = *KV.first;
+    for (auto &DependencyName : KV.second) {
+      assert(DependencyJD.MaterializingInfos.count(DependencyName) &&
+             "Dependency does not have MaterializingInfo");
+      auto &DependencyMI = DependencyJD.MaterializingInfos[DependencyName];
+      auto DependantsI = DependencyMI.Dependants.find(this);
+      assert(DependantsI != DependencyMI.Dependants.end() &&
+             "No dependnts entry recorded for this JD");
+      assert(DependantsI->second.count(Name) &&
+             "No dependants entry recorded for Name");
+      DependantsI->second.erase(Name);
+      if (DependantsI->second.empty())
+        DependencyMI.Dependants.erase(DependantsI);
+    }
+  }
+
+  AsynchronousSymbolQuerySet QueriesToFail;
+  for (auto &Q : MI.takeAllPendingQueries())
+    QueriesToFail.insert(std::move(Q));
+  return QueriesToFail;
+}
+
 void JITDylib::addDependencies(const SymbolStringPtr &Name,
                                const SymbolDependenceMap &Dependencies) {
   assert(Symbols.count(Name) && "Name not in symbol table");
   assert(Symbols[Name].isInMaterializationPhase() &&
          "Can not add dependencies for a symbol that is not materializing");
 
+  // If Name is already in an error state then just bail out.
+  if (Symbols[Name].getFlags().hasError())
+    return;
+
   auto &MI = MaterializingInfos[Name];
   assert(!MI.IsEmitted && "Can not add dependencies to an emitted symbol");
 
+  bool DependsOnSymbolInErrorState = false;
+
+  // Register dependencies, record whether any depenendency is in the error
+  // state.
   for (auto &KV : Dependencies) {
     assert(KV.first && "Null JITDylib in dependency?");
     auto &OtherJITDylib = *KV.first;
     auto &DepsOnOtherJITDylib = MI.UnemittedDependencies[&OtherJITDylib];
 
     for (auto &OtherSymbol : KV.second) {
+
+      // Check the sym entry for the dependency.
+      auto SymI = OtherJITDylib.Symbols.find(OtherSymbol);
+
 #ifndef NDEBUG
       // Assert that this symbol exists and has not been emitted already.
-      auto SymI = OtherJITDylib.Symbols.find(OtherSymbol);
       assert(SymI != OtherJITDylib.Symbols.end() &&
              (SymI->second.getState() != SymbolState::Ready &&
               "Dependency on emitted symbol"));
 #endif
 
+      // If the dependency is in an error state then note this and continue,
+      // we will move this symbol to the error state below.
+      if (SymI->second.getFlags().hasError()) {
+        DependsOnSymbolInErrorState = true;
+        continue;
+      }
+
+      // If the dependency was not in the error state then add it to
+      // our list of dependencies.
+      assert(OtherJITDylib.MaterializingInfos.count(OtherSymbol) &&
+             "No MaterializingInfo for dependency");
       auto &OtherMI = OtherJITDylib.MaterializingInfos[OtherSymbol];
 
       if (OtherMI.IsEmitted)
@@ -866,63 +963,133 @@ void JITDylib::addDependencies(const Sym
     if (DepsOnOtherJITDylib.empty())
       MI.UnemittedDependencies.erase(&OtherJITDylib);
   }
+
+  // If this symbol dependended on any symbols in the error state then move
+  // this symbol to the error state too.
+  if (DependsOnSymbolInErrorState)
+    Symbols[Name].setFlags(Symbols[Name].getFlags() | JITSymbolFlags::HasError);
 }
 
-void JITDylib::resolve(const SymbolMap &Resolved) {
-  auto CompletedQueries = ES.runSessionLocked([&, this]() {
-    AsynchronousSymbolQuerySet CompletedQueries;
+Error JITDylib::resolve(const SymbolMap &Resolved) {
+  SymbolNameSet SymbolsInErrorState;
+  AsynchronousSymbolQuerySet CompletedQueries;
+
+  ES.runSessionLocked([&, this]() {
+    struct WorklistEntry {
+      SymbolTable::iterator SymI;
+      JITEvaluatedSymbol ResolvedSym;
+    };
+
+    std::vector<WorklistEntry> Worklist;
+    Worklist.reserve(Resolved.size());
+
+    // Build worklist and check for any symbols in the error state.
     for (const auto &KV : Resolved) {
-      auto &Name = KV.first;
-      auto Sym = KV.second;
 
-      auto I = Symbols.find(Name);
+      assert(!KV.second.getFlags().hasError() &&
+             "Resolution result can not have error flag set");
+
+      auto SymI = Symbols.find(KV.first);
 
-      assert(I != Symbols.end() && "Symbol not found");
-      assert(!I->second.hasMaterializerAttached() &&
+      assert(SymI != Symbols.end() && "Symbol not found");
+      assert(!SymI->second.hasMaterializerAttached() &&
              "Resolving symbol with materializer attached?");
-      assert(I->second.getState() == SymbolState::Materializing &&
+      assert(SymI->second.getState() == SymbolState::Materializing &&
              "Symbol should be materializing");
-      assert(I->second.getAddress() == 0 && "Symbol has already been resolved");
+      assert(SymI->second.getAddress() == 0 &&
+             "Symbol has already been resolved");
+
+      if (SymI->second.getFlags().hasError())
+        SymbolsInErrorState.insert(KV.first);
+      else {
+        assert((KV.second.getFlags() & ~JITSymbolFlags::Weak) ==
+                   (SymI->second.getFlags() & ~JITSymbolFlags::Weak) &&
+               "Resolved flags should match the declared flags");
+
+        Worklist.push_back({SymI, KV.second});
+      }
+    }
+
+    // If any symbols were in the error state then bail out.
+    if (!SymbolsInErrorState.empty())
+      return;
+
+    while (!Worklist.empty()) {
+      auto SymI = Worklist.back().SymI;
+      auto ResolvedSym = Worklist.back().ResolvedSym;
+      Worklist.pop_back();
 
-      assert((Sym.getFlags() & ~JITSymbolFlags::Weak) ==
-                 (I->second.getFlags() & ~JITSymbolFlags::Weak) &&
-             "Resolved flags should match the declared flags");
+      auto &Name = SymI->first;
 
-      // Once resolved, symbols can never be weak.
-      JITSymbolFlags ResolvedFlags = Sym.getFlags();
+      // Resolved symbols can not be weak: discard the weak flag.
+      JITSymbolFlags ResolvedFlags = ResolvedSym.getFlags();
       ResolvedFlags &= ~JITSymbolFlags::Weak;
-      I->second.setAddress(Sym.getAddress());
-      I->second.setFlags(ResolvedFlags);
-      I->second.setState(SymbolState::Resolved);
+      SymI->second.setAddress(ResolvedSym.getAddress());
+      SymI->second.setFlags(ResolvedFlags);
+      SymI->second.setState(SymbolState::Resolved);
 
       auto &MI = MaterializingInfos[Name];
       for (auto &Q : MI.takeQueriesMeeting(SymbolState::Resolved)) {
-        Q->notifySymbolMetRequiredState(Name, Sym);
+        Q->notifySymbolMetRequiredState(Name, ResolvedSym);
         if (Q->isComplete())
           CompletedQueries.insert(std::move(Q));
       }
     }
-
-    return CompletedQueries;
   });
 
+  assert((SymbolsInErrorState.empty() || CompletedQueries.empty()) &&
+         "Can't fail symbols and completed queries at the same time");
+
+  // If we failed any symbols then return an error.
+  if (!SymbolsInErrorState.empty()) {
+    auto FailedSymbolsDepMap = std::make_shared<SymbolDependenceMap>();
+    (*FailedSymbolsDepMap)[this] = std::move(SymbolsInErrorState);
+    return make_error<FailedToMaterialize>(std::move(FailedSymbolsDepMap));
+  }
+
+  // Otherwise notify all the completed queries.
   for (auto &Q : CompletedQueries) {
     assert(Q->isComplete() && "Q not completed");
     Q->handleComplete();
   }
+
+  return Error::success();
 }
 
-void JITDylib::emit(const SymbolFlagsMap &Emitted) {
-  auto CompletedQueries = ES.runSessionLocked([&, this]() {
-    AsynchronousSymbolQuerySet CompletedQueries;
+Error JITDylib::emit(const SymbolFlagsMap &Emitted) {
+  AsynchronousSymbolQuerySet CompletedQueries;
+  SymbolNameSet SymbolsInErrorState;
+
+  ES.runSessionLocked([&, this]() {
+    std::vector<SymbolTable::iterator> Worklist;
 
+    // Scan to build worklist, record any symbols in the erorr state.
     for (const auto &KV : Emitted) {
-      const auto &Name = KV.first;
+      auto &Name = KV.first;
+
+      auto SymI = Symbols.find(Name);
+      assert(SymI != Symbols.end() && "No symbol table entry for Name");
+
+      if (SymI->second.getFlags().hasError())
+        SymbolsInErrorState.insert(Name);
+      else
+        Worklist.push_back(SymI);
+    }
+
+    // If any symbols were in the error state then bail out.
+    if (!SymbolsInErrorState.empty())
+      return;
+
+    // Otherwise update dependencies and move to the emitted state.
+    while (!Worklist.empty()) {
+      auto SymI = Worklist.back();
+      Worklist.pop_back();
+
+      auto &Name = SymI->first;
 
       auto MII = MaterializingInfos.find(Name);
       assert(MII != MaterializingInfos.end() &&
              "Missing MaterializingInfo entry");
-
       auto &MI = MII->second;
 
       // For each dependant, transfer this node's emitted dependencies to
@@ -939,8 +1106,12 @@ void JITDylib::emit(const SymbolFlagsMap
           auto &DependantMI = DependantMII->second;
 
           // Remove the dependant's dependency on this node.
+          assert(DependantMI.UnemittedDependencies.count(this) &&
+                 "Dependant does not have an unemitted dependencies record for "
+                 "this JITDylib");
           assert(DependantMI.UnemittedDependencies[this].count(Name) &&
                  "Dependant does not count this symbol as a dependency?");
+
           DependantMI.UnemittedDependencies[this].erase(Name);
           if (DependantMI.UnemittedDependencies[this].empty())
             DependantMI.UnemittedDependencies.erase(this);
@@ -980,8 +1151,6 @@ void JITDylib::emit(const SymbolFlagsMap
       MI.IsEmitted = true;
 
       if (MI.UnemittedDependencies.empty()) {
-        auto SymI = Symbols.find(Name);
-        assert(SymI != Symbols.end() && "Symbol has no entry in Symbols table");
         SymI->second.setState(SymbolState::Ready);
         for (auto &Q : MI.takeQueriesMeeting(SymbolState::Ready)) {
           Q->notifySymbolMetRequiredState(Name, SymI->second.getSymbol());
@@ -992,61 +1161,98 @@ void JITDylib::emit(const SymbolFlagsMap
         MaterializingInfos.erase(MII);
       }
     }
-
-    return CompletedQueries;
   });
 
+  assert((SymbolsInErrorState.empty() || CompletedQueries.empty()) &&
+         "Can't fail symbols and completed queries at the same time");
+
+  // If we failed any symbols then return an error.
+  if (!SymbolsInErrorState.empty()) {
+    auto FailedSymbolsDepMap = std::make_shared<SymbolDependenceMap>();
+    (*FailedSymbolsDepMap)[this] = std::move(SymbolsInErrorState);
+    return make_error<FailedToMaterialize>(std::move(FailedSymbolsDepMap));
+  }
+
+  // Otherwise notify all the completed queries.
   for (auto &Q : CompletedQueries) {
     assert(Q->isComplete() && "Q is not complete");
     Q->handleComplete();
   }
-}
 
-void JITDylib::notifyFailed(const SymbolNameSet &FailedSymbols) {
+  return Error::success();
+}
 
-  // FIXME: This should fail any transitively dependant symbols too.
+void JITDylib::notifyFailed(const SymbolFlagsMap &FailedSymbols) {
+  AsynchronousSymbolQuerySet FailedQueries;
 
-  auto FailedQueriesToNotify = ES.runSessionLocked([&, this]() {
-    AsynchronousSymbolQuerySet FailedQueries;
-    std::vector<MaterializingInfosMap::iterator> MIIsToRemove;
+  ES.runSessionLocked([&]() {
+    for (auto &KV : FailedSymbols) {
+      auto &Name = KV.first;
 
-    for (auto &Name : FailedSymbols) {
-      auto I = Symbols.find(Name);
-      assert(I != Symbols.end() && "Symbol not present in this JITDylib");
-      Symbols.erase(I);
+      assert(Symbols.count(Name) && "No symbol table entry for Name");
+      auto &Sym = Symbols[Name];
 
+      // Move the symbol into the error state.
+      // Note that this may be redundant: The symbol might already have been
+      // moved to this state in response to the failure of a dependence.
+      Sym.setFlags(Sym.getFlags() | JITSymbolFlags::HasError);
+
+      // FIXME: Come up with a sane mapping of state to
+      // presence-of-MaterializingInfo so that we can assert presence / absence
+      // here, rather than testing it.
       auto MII = MaterializingInfos.find(Name);
 
-      // If we have not created a MaterializingInfo for this symbol yet then
-      // there is nobody to notify.
       if (MII == MaterializingInfos.end())
         continue;
 
-      // Remove this symbol from the dependants list of any dependencies.
-      for (auto &KV : MII->second.UnemittedDependencies) {
-        auto *DependencyJD = KV.first;
-        auto &Dependencies = KV.second;
-        for (auto &DependencyName : Dependencies) {
-          auto DependencyMII =
-              DependencyJD->MaterializingInfos.find(DependencyName);
-          assert(DependencyMII != DependencyJD->MaterializingInfos.end() &&
-                 "Unemitted dependency must have a MaterializingInfo entry");
-          assert(DependencyMII->second.Dependants.count(this) &&
-                 "Dependency's dependants list does not contain this JITDylib");
-          assert(DependencyMII->second.Dependants[this].count(Name) &&
-                 "Dependency's dependants list does not contain dependant");
-          DependencyMII->second.Dependants[this].erase(Name);
+      auto &MI = MII->second;
+
+      // Move all dependants to the error state and disconnect from them.
+      for (auto &KV : MI.Dependants) {
+        auto &DependantJD = *KV.first;
+        for (auto &DependantName : KV.second) {
+          assert(DependantJD.Symbols.count(DependantName) &&
+                 "No symbol table entry for DependantName");
+          auto &DependantSym = DependantJD.Symbols[DependantName];
+          DependantSym.setFlags(DependantSym.getFlags() |
+                                JITSymbolFlags::HasError);
+
+          assert(DependantJD.MaterializingInfos.count(DependantName) &&
+                 "No MaterializingInfo for dependant");
+          auto &DependantMI = DependantJD.MaterializingInfos[DependantName];
+
+          auto UnemittedDepI = DependantMI.UnemittedDependencies.find(this);
+          assert(UnemittedDepI != DependantMI.UnemittedDependencies.end() &&
+                 "No UnemittedDependencies entry for this JITDylib");
+          assert(UnemittedDepI->second.count(Name) &&
+                 "No UnemittedDependencies entry for this symbol");
+          UnemittedDepI->second.erase(Name);
+          if (UnemittedDepI->second.empty())
+            DependantMI.UnemittedDependencies.erase(UnemittedDepI);
+        }
+      }
+
+      // Disconnect from all unemitted depenencies.
+      for (auto &KV : MI.UnemittedDependencies) {
+        auto &UnemittedDepJD = *KV.first;
+        for (auto &UnemittedDepName : KV.second) {
+          auto UnemittedDepMII =
+              UnemittedDepJD.MaterializingInfos.find(UnemittedDepName);
+          assert(UnemittedDepMII != UnemittedDepJD.MaterializingInfos.end() &&
+                 "Missing MII for unemitted dependency");
+          assert(UnemittedDepMII->second.Dependants.count(this) &&
+                 "JD not listed as a dependant of unemitted dependency");
+          assert(UnemittedDepMII->second.Dependants[this].count(Name) &&
+                 "Name is not listed as a dependant of unemitted dependency");
+          UnemittedDepMII->second.Dependants[this].erase(Name);
+          if (UnemittedDepMII->second.Dependants[this].empty())
+            UnemittedDepMII->second.Dependants.erase(this);
         }
       }
 
-      // Copy all the queries to the FailedQueries list, then abandon them.
-      // This has to be a copy, and the copy has to come before the abandon
-      // operation: Each Q.detach() call will reach back into this
-      // PendingQueries list to remove Q.
+      // Collect queries to be failed for this MII.
       for (auto &Q : MII->second.pendingQueries())
         FailedQueries.insert(Q);
-
-      MIIsToRemove.push_back(std::move(MII));
     }
 
     // Detach failed queries.
@@ -1054,18 +1260,17 @@ void JITDylib::notifyFailed(const Symbol
       Q->detach();
 
     // Remove the MaterializingInfos.
-    for (auto &MII : MIIsToRemove) {
-      assert(!MII->second.hasQueriesPending() &&
-             "Queries remain after symbol was failed");
-
-      MaterializingInfos.erase(MII);
+    for (auto &KV : FailedSymbols) {
+      assert(MaterializingInfos.count(KV.first) && "Expected MI for Name");
+      MaterializingInfos.erase(KV.first);
     }
-
-    return FailedQueries;
   });
 
-  for (auto &Q : FailedQueriesToNotify)
-    Q->handleFailed(make_error<FailedToMaterialize>(FailedSymbols));
+  auto FailedSymbolsMap = std::make_shared<SymbolDependenceMap>();
+  for (auto &KV : FailedSymbols)
+    (*FailedSymbolsMap)[this].insert(KV.first);
+  for (auto &Q : FailedQueries)
+    Q->handleFailed(make_error<FailedToMaterialize>(FailedSymbolsMap));
 }
 
 void JITDylib::setSearchOrder(JITDylibSearchList NewSearchOrder,
@@ -1221,7 +1426,8 @@ Error JITDylib::lodgeQuery(std::shared_p
                            MaterializationUnitList &MUs) {
   assert(Q && "Query can not be null");
 
-  lodgeQueryImpl(Q, Unresolved, MatchNonExported, MUs);
+  if (auto Err = lodgeQueryImpl(Q, Unresolved, MatchNonExported, MUs))
+    return Err;
 
   // Run any definition generators.
   for (auto &DG : DefGenerators) {
@@ -1233,13 +1439,21 @@ Error JITDylib::lodgeQuery(std::shared_p
     // Run the generator.
     auto NewDefs = DG->tryToGenerate(*this, Unresolved);
 
+    // If the generator returns an error then bail out.
     if (!NewDefs)
       return NewDefs.takeError();
 
+    // If the generator was able to generate new definitions for any of the
+    // unresolved symbols then lodge the query against them.
     if (!NewDefs->empty()) {
       for (auto &D : *NewDefs)
         Unresolved.erase(D);
-      lodgeQueryImpl(Q, *NewDefs, MatchNonExported, MUs);
+
+      // Lodge query. This can not fail as any new definitions were added
+      // by the generator under the session locked. Since they can't have
+      // started materializing yet the can not have failed.
+      cantFail(lodgeQueryImpl(Q, *NewDefs, MatchNonExported, MUs));
+
       assert(NewDefs->empty() &&
              "All fallback defs should have been found by lookupImpl");
     }
@@ -1248,7 +1462,7 @@ Error JITDylib::lodgeQuery(std::shared_p
   return Error::success();
 }
 
-void JITDylib::lodgeQueryImpl(
+Error JITDylib::lodgeQueryImpl(
     std::shared_ptr<AsynchronousSymbolQuery> &Q, SymbolNameSet &Unresolved,
     bool MatchNonExported,
     std::vector<std::unique_ptr<MaterializationUnit>> &MUs) {
@@ -1269,6 +1483,14 @@ void JITDylib::lodgeQueryImpl(
     // Unresolved set.
     ToRemove.push_back(Name);
 
+    // If we matched against this symbol but it is in the error state then
+    // bail out and treat it as a failure to materialize.
+    if (SymI->second.getFlags().hasError()) {
+      auto FailedSymbolsMap = std::make_shared<SymbolDependenceMap>();
+      (*FailedSymbolsMap)[this] = {Name};
+      return make_error<FailedToMaterialize>(std::move(FailedSymbolsMap));
+    }
+
     // If this symbol already meets the required state for then notify the
     // query and continue.
     if (SymI->second.getState() >= Q->getRequiredState()) {
@@ -1311,6 +1533,8 @@ void JITDylib::lodgeQueryImpl(
   // Remove any symbols that we found.
   for (auto &Name : ToRemove)
     Unresolved.erase(Name);
+
+  return Error::success();
 }
 
 Expected<SymbolNameSet>

Modified: llvm/trunk/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/IndirectionUtils.cpp?rev=369808&r1=369807&r2=369808&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/IndirectionUtils.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/IndirectionUtils.cpp Fri Aug 23 13:37:31 2019
@@ -37,8 +37,9 @@ private:
   void materialize(MaterializationResponsibility R) override {
     SymbolMap Result;
     Result[Name] = JITEvaluatedSymbol(Compile(), JITSymbolFlags::Exported);
-    R.notifyResolved(Result);
-    R.notifyEmitted();
+    // No dependencies, so these calls cannot fail.
+    cantFail(R.notifyResolved(Result));
+    cantFail(R.notifyEmitted());
   }
 
   void discard(const JITDylib &JD, const SymbolStringPtr &Name) override {

Modified: llvm/trunk/lib/ExecutionEngine/Orc/LazyReexports.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/LazyReexports.cpp?rev=369808&r1=369807&r2=369808&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/LazyReexports.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/LazyReexports.cpp Fri Aug 23 13:37:31 2019
@@ -182,8 +182,9 @@ void LazyReexportsMaterializationUnit::m
   for (auto &Alias : RequestedAliases)
     Stubs[Alias.first] = ISManager.findStub(*Alias.first, false);
 
-  R.notifyResolved(Stubs);
-  R.notifyEmitted();
+  // No registered dependencies, so these calls cannot fail.
+  cantFail(R.notifyResolved(Stubs));
+  cantFail(R.notifyEmitted());
 }
 
 void LazyReexportsMaterializationUnit::discard(const JITDylib &JD,

Modified: llvm/trunk/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp?rev=369808&r1=369807&r2=369808&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp Fri Aug 23 13:37:31 2019
@@ -127,7 +127,11 @@ public:
       if (auto Err = MR.defineMaterializing(ExtraSymbolsToClaim))
         return notifyFailed(std::move(Err));
 
-    MR.notifyResolved(InternedResult);
+    if (auto Err = MR.notifyResolved(InternedResult)) {
+      Layer.getExecutionSession().reportError(std::move(Err));
+      MR.failMaterialization();
+      return;
+    }
 
     Layer.notifyLoaded(MR);
   }
@@ -138,10 +142,12 @@ public:
     if (auto Err = Layer.notifyEmitted(MR, std::move(A))) {
       Layer.getExecutionSession().reportError(std::move(Err));
       MR.failMaterialization();
-
       return;
     }
-    MR.notifyEmitted();
+    if (auto Err = MR.notifyEmitted()) {
+      Layer.getExecutionSession().reportError(std::move(Err));
+      MR.failMaterialization();
+    }
   }
 
   AtomGraphPassFunction getMarkLivePass(const Triple &TT) const override {

Modified: llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp?rev=369808&r1=369807&r2=369808&view=diff
==============================================================================
--- llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp (original)
+++ llvm/trunk/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp Fri Aug 23 13:37:31 2019
@@ -184,7 +184,10 @@ Error RTDyldObjectLinkingLayer::onObjLoa
     if (auto Err = R.defineMaterializing(ExtraSymbolsToClaim))
       return Err;
 
-  R.notifyResolved(Symbols);
+  if (auto Err = R.notifyResolved(Symbols)) {
+    R.failMaterialization();
+    return Err;
+  }
 
   if (NotifyLoaded)
     NotifyLoaded(K, Obj, *LoadedObjInfo);
@@ -201,7 +204,11 @@ void RTDyldObjectLinkingLayer::onObjEmit
     return;
   }
 
-  R.notifyEmitted();
+  if (auto Err = R.notifyEmitted()) {
+    getExecutionSession().reportError(std::move(Err));
+    R.failMaterialization();
+    return;
+  }
 
   if (NotifyEmitted)
     NotifyEmitted(K, std::move(ObjBuffer));

Modified: llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp?rev=369808&r1=369807&r2=369808&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp Fri Aug 23 13:37:31 2019
@@ -48,11 +48,11 @@ TEST_F(CoreAPIsStandardTest, BasicSucces
 
   EXPECT_FALSE(OnCompletionRun) << "Should not have been resolved yet";
 
-  FooMR->notifyResolved({{Foo, FooSym}});
+  cantFail(FooMR->notifyResolved({{Foo, FooSym}}));
 
   EXPECT_FALSE(OnCompletionRun) << "Should not be ready yet";
 
-  FooMR->notifyEmitted();
+  cantFail(FooMR->notifyEmitted());
 
   EXPECT_TRUE(OnCompletionRun) << "Should have been marked ready";
 }
@@ -109,8 +109,8 @@ TEST_F(CoreAPIsStandardTest, RemoveSymbo
       SymbolFlagsMap({{Bar, BarSym.getFlags()}}),
       [this](MaterializationResponsibility R) {
         ADD_FAILURE() << "Unexpected materialization of \"Bar\"";
-        R.notifyResolved({{Bar, BarSym}});
-        R.notifyEmitted();
+        cantFail(R.notifyResolved({{Bar, BarSym}}));
+        cantFail(R.notifyEmitted());
       },
       [&](const JITDylib &JD, const SymbolStringPtr &Name) {
         EXPECT_EQ(Name, Bar) << "Expected \"Bar\" to be discarded";
@@ -156,8 +156,8 @@ TEST_F(CoreAPIsStandardTest, RemoveSymbo
     consumeError(std::move(Err));
   }
 
-  BazR->notifyResolved({{Baz, BazSym}});
-  BazR->notifyEmitted();
+  cantFail(BazR->notifyResolved({{Baz, BazSym}}));
+  cantFail(BazR->notifyEmitted());
   {
     // Attempt 3: Search now that all symbols are fully materialized
     // (Foo, Baz), or not yet materialized (Bar).
@@ -318,8 +318,8 @@ TEST_F(CoreAPIsStandardTest, TestThatReE
       SymbolFlagsMap({{Bar, BarSym.getFlags()}}),
       [&](MaterializationResponsibility R) {
         BarMaterialized = true;
-        R.notifyResolved({{Bar, BarSym}});
-        R.notifyEmitted();
+        cantFail(R.notifyResolved({{Bar, BarSym}}));
+        cantFail(R.notifyEmitted());
       });
 
   cantFail(JD.define(BarMU));
@@ -374,8 +374,10 @@ TEST_F(CoreAPIsStandardTest, TestTrivial
             OnCompletion, NoDependenciesToRegister);
 
   FooR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}});
-  FooR->notifyResolved({{Foo, FooSym}});
-  FooR->notifyEmitted();
+  EXPECT_THAT_ERROR(FooR->notifyResolved({{Foo, FooSym}}), Succeeded())
+      << "No symbols marked failed, but Foo failed to resolve";
+  EXPECT_THAT_ERROR(FooR->notifyEmitted(), Succeeded())
+      << "No symbols marked failed, but Foo failed to emit";
 
   EXPECT_TRUE(FooReady)
     << "Self-dependency prevented symbol from being marked ready";
@@ -488,9 +490,12 @@ TEST_F(CoreAPIsStandardTest, TestCircula
   EXPECT_FALSE(BazResolved) << "\"Baz\" should not be resolved yet";
 
   // Resolve the symbols (but do not emit them).
-  FooR->notifyResolved({{Foo, FooSym}});
-  BarR->notifyResolved({{Bar, BarSym}});
-  BazR->notifyResolved({{Baz, BazSym}});
+  EXPECT_THAT_ERROR(FooR->notifyResolved({{Foo, FooSym}}), Succeeded())
+      << "No symbols failed, but Foo failed to resolve";
+  EXPECT_THAT_ERROR(BarR->notifyResolved({{Bar, BarSym}}), Succeeded())
+      << "No symbols failed, but Bar failed to resolve";
+  EXPECT_THAT_ERROR(BazR->notifyResolved({{Baz, BazSym}}), Succeeded())
+      << "No symbols failed, but Baz failed to resolve";
 
   // Verify that the symbols have been resolved, but are not ready yet.
   EXPECT_TRUE(FooResolved) << "\"Foo\" should be resolved now";
@@ -502,8 +507,10 @@ TEST_F(CoreAPIsStandardTest, TestCircula
   EXPECT_FALSE(BazReady) << "\"Baz\" should not be ready yet";
 
   // Emit two of the symbols.
-  FooR->notifyEmitted();
-  BarR->notifyEmitted();
+  EXPECT_THAT_ERROR(FooR->notifyEmitted(), Succeeded())
+      << "No symbols failed, but Foo failed to emit";
+  EXPECT_THAT_ERROR(BarR->notifyEmitted(), Succeeded())
+      << "No symbols failed, but Bar failed to emit";
 
   // Verify that nothing is ready until the circular dependence is resolved.
   EXPECT_FALSE(FooReady) << "\"Foo\" still should not be ready";
@@ -511,7 +518,8 @@ TEST_F(CoreAPIsStandardTest, TestCircula
   EXPECT_FALSE(BazReady) << "\"Baz\" still should not be ready";
 
   // Emit the last symbol.
-  BazR->notifyEmitted();
+  EXPECT_THAT_ERROR(BazR->notifyEmitted(), Succeeded())
+      << "No symbols failed, but Baz failed to emit";
 
   // Verify that everything becomes ready once the circular dependence resolved.
   EXPECT_TRUE(FooReady) << "\"Foo\" should be ready now";
@@ -519,6 +527,197 @@ TEST_F(CoreAPIsStandardTest, TestCircula
   EXPECT_TRUE(BazReady) << "\"Baz\" should be ready now";
 }
 
+TEST_F(CoreAPIsStandardTest, FailureInDependency) {
+  Optional<MaterializationResponsibility> FooR;
+  Optional<MaterializationResponsibility> BarR;
+
+  // Create a MaterializationUnit for each symbol that moves the
+  // MaterializationResponsibility into one of the locals above.
+  auto FooMU = std::make_unique<SimpleMaterializationUnit>(
+      SymbolFlagsMap({{Foo, FooSym.getFlags()}}),
+      [&](MaterializationResponsibility R) { FooR.emplace(std::move(R)); });
+
+  auto BarMU = std::make_unique<SimpleMaterializationUnit>(
+      SymbolFlagsMap({{Bar, BarSym.getFlags()}}),
+      [&](MaterializationResponsibility R) { BarR.emplace(std::move(R)); });
+
+  // Define the symbols.
+  cantFail(JD.define(FooMU));
+  cantFail(JD.define(BarMU));
+
+  bool OnFooReadyRun = false;
+  auto OnFooReady = [&](Expected<SymbolMap> Result) {
+    EXPECT_THAT_EXPECTED(std::move(Result), Failed());
+    OnFooReadyRun = true;
+  };
+
+  ES.lookup(JITDylibSearchList({{&JD, false}}), {Foo}, SymbolState::Ready,
+            std::move(OnFooReady), NoDependenciesToRegister);
+
+  bool OnBarReadyRun = false;
+  auto OnBarReady = [&](Expected<SymbolMap> Result) {
+    EXPECT_THAT_EXPECTED(std::move(Result), Failed());
+    OnBarReadyRun = true;
+  };
+
+  ES.lookup(JITDylibSearchList({{&JD, false}}), {Bar}, SymbolState::Ready,
+            std::move(OnBarReady), NoDependenciesToRegister);
+
+  // Add a dependency by Foo on Bar.
+  FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}});
+
+  // Fail bar.
+  BarR->failMaterialization();
+
+  // Verify that queries on Bar failed, but queries on Foo have not yet.
+  EXPECT_TRUE(OnBarReadyRun) << "Query for \"Bar\" was not run";
+  EXPECT_FALSE(OnFooReadyRun) << "Query for \"Foo\" was run unexpectedly";
+
+  // Check that we can still resolve Foo (even though it has been failed).
+  EXPECT_THAT_ERROR(FooR->notifyResolved({{Foo, FooSym}}), Failed())
+      << "Expected resolution for \"Foo\" to fail.";
+
+  FooR->failMaterialization();
+
+  // Verify that queries on Foo have now failed.
+  EXPECT_TRUE(OnFooReadyRun) << "Query for \"Foo\" was not run";
+
+  // Verify that subsequent lookups on Bar and Foo fail.
+  EXPECT_THAT_EXPECTED(ES.lookup({&JD}, {Bar}), Failed())
+      << "Lookup on failed symbol should fail";
+
+  EXPECT_THAT_EXPECTED(ES.lookup({&JD}, {Foo}), Failed())
+      << "Lookup on failed symbol should fail";
+}
+
+TEST_F(CoreAPIsStandardTest, FailureInCircularDependency) {
+  Optional<MaterializationResponsibility> FooR;
+  Optional<MaterializationResponsibility> BarR;
+
+  // Create a MaterializationUnit for each symbol that moves the
+  // MaterializationResponsibility into one of the locals above.
+  auto FooMU = std::make_unique<SimpleMaterializationUnit>(
+      SymbolFlagsMap({{Foo, FooSym.getFlags()}}),
+      [&](MaterializationResponsibility R) { FooR.emplace(std::move(R)); });
+
+  auto BarMU = std::make_unique<SimpleMaterializationUnit>(
+      SymbolFlagsMap({{Bar, BarSym.getFlags()}}),
+      [&](MaterializationResponsibility R) { BarR.emplace(std::move(R)); });
+
+  // Define the symbols.
+  cantFail(JD.define(FooMU));
+  cantFail(JD.define(BarMU));
+
+  bool OnFooReadyRun = false;
+  auto OnFooReady = [&](Expected<SymbolMap> Result) {
+    EXPECT_THAT_EXPECTED(std::move(Result), Failed());
+    OnFooReadyRun = true;
+  };
+
+  ES.lookup(JITDylibSearchList({{&JD, false}}), {Foo}, SymbolState::Ready,
+            std::move(OnFooReady), NoDependenciesToRegister);
+
+  bool OnBarReadyRun = false;
+  auto OnBarReady = [&](Expected<SymbolMap> Result) {
+    EXPECT_THAT_EXPECTED(std::move(Result), Failed());
+    OnBarReadyRun = true;
+  };
+
+  ES.lookup(JITDylibSearchList({{&JD, false}}), {Bar}, SymbolState::Ready,
+            std::move(OnBarReady), NoDependenciesToRegister);
+
+  // Add a dependency by Foo on Bar and vice-versa.
+  FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}});
+  BarR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}});
+
+  // Fail bar.
+  BarR->failMaterialization();
+
+  // Verify that queries on Bar failed, but queries on Foo have not yet.
+  EXPECT_TRUE(OnBarReadyRun) << "Query for \"Bar\" was not run";
+  EXPECT_FALSE(OnFooReadyRun) << "Query for \"Foo\" was run unexpectedly";
+
+  // Verify that trying to resolve Foo fails.
+  EXPECT_THAT_ERROR(FooR->notifyResolved({{Foo, FooSym}}), Failed())
+      << "Expected resolution for \"Foo\" to fail.";
+
+  FooR->failMaterialization();
+
+  // Verify that queries on Foo have now failed.
+  EXPECT_TRUE(OnFooReadyRun) << "Query for \"Foo\" was not run";
+
+  // Verify that subsequent lookups on Bar and Foo fail.
+  EXPECT_THAT_EXPECTED(ES.lookup({&JD}, {Bar}), Failed())
+      << "Lookup on failed symbol should fail";
+
+  EXPECT_THAT_EXPECTED(ES.lookup({&JD}, {Foo}), Failed())
+      << "Lookup on failed symbol should fail";
+}
+
+TEST_F(CoreAPIsStandardTest, AddDependencyOnFailedSymbol) {
+  Optional<MaterializationResponsibility> FooR;
+  Optional<MaterializationResponsibility> BarR;
+
+  // Create a MaterializationUnit for each symbol that moves the
+  // MaterializationResponsibility into one of the locals above.
+  auto FooMU = std::make_unique<SimpleMaterializationUnit>(
+      SymbolFlagsMap({{Foo, FooSym.getFlags()}}),
+      [&](MaterializationResponsibility R) { FooR.emplace(std::move(R)); });
+
+  auto BarMU = std::make_unique<SimpleMaterializationUnit>(
+      SymbolFlagsMap({{Bar, BarSym.getFlags()}}),
+      [&](MaterializationResponsibility R) { BarR.emplace(std::move(R)); });
+
+  // Define the symbols.
+  cantFail(JD.define(FooMU));
+  cantFail(JD.define(BarMU));
+
+  bool OnFooReadyRun = false;
+  auto OnFooReady = [&](Expected<SymbolMap> Result) {
+    EXPECT_THAT_EXPECTED(std::move(Result), Failed());
+    OnFooReadyRun = true;
+  };
+
+  ES.lookup(JITDylibSearchList({{&JD, false}}), {Foo}, SymbolState::Ready,
+            std::move(OnFooReady), NoDependenciesToRegister);
+
+  bool OnBarReadyRun = false;
+  auto OnBarReady = [&](Expected<SymbolMap> Result) {
+    EXPECT_THAT_EXPECTED(std::move(Result), Failed());
+    OnBarReadyRun = true;
+  };
+
+  ES.lookup(JITDylibSearchList({{&JD, false}}), {Bar}, SymbolState::Ready,
+            std::move(OnBarReady), NoDependenciesToRegister);
+
+  // Fail bar.
+  BarR->failMaterialization();
+
+  // We expect Bar's query to fail immediately, but Foo's query not to have run
+  // yet.
+  EXPECT_TRUE(OnBarReadyRun) << "Query for \"Bar\" was not run";
+  EXPECT_FALSE(OnFooReadyRun) << "Query for \"Foo\" should not have run yet";
+
+  // Add dependency of Foo on Bar.
+  FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}});
+
+  // Check that we can still resolve Foo (even though it has been failed).
+  EXPECT_THAT_ERROR(FooR->notifyResolved({{Foo, FooSym}}), Failed())
+      << "Expected resolution for \"Foo\" to fail.";
+
+  FooR->failMaterialization();
+
+  // Foo's query should have failed before we return from addDependencies.
+  EXPECT_TRUE(OnFooReadyRun) << "Query for \"Foo\" was not run";
+
+  // Verify that subsequent lookups on Bar and Foo fail.
+  EXPECT_THAT_EXPECTED(ES.lookup({&JD}, {Bar}), Failed())
+      << "Lookup on failed symbol should fail";
+
+  EXPECT_THAT_EXPECTED(ES.lookup({&JD}, {Foo}), Failed())
+      << "Lookup on failed symbol should fail";
+}
+
 TEST_F(CoreAPIsStandardTest, DropMaterializerWhenEmpty) {
   bool DestructorRun = false;
 
@@ -560,8 +759,8 @@ TEST_F(CoreAPIsStandardTest, AddAndMater
       SymbolFlagsMap({{Foo, JITSymbolFlags::Exported}, {Bar, WeakExported}}),
       [&](MaterializationResponsibility R) {
         assert(BarDiscarded && "Bar should have been discarded by this point");
-        R.notifyResolved(SymbolMap({{Foo, FooSym}}));
-        R.notifyEmitted();
+        cantFail(R.notifyResolved(SymbolMap({{Foo, FooSym}})));
+        cantFail(R.notifyEmitted());
         FooMaterialized = true;
       },
       [&](const JITDylib &JD, SymbolStringPtr Name) {
@@ -601,7 +800,8 @@ TEST_F(CoreAPIsStandardTest, TestBasicWe
   auto MU1 = std::make_unique<SimpleMaterializationUnit>(
       SymbolFlagsMap({{Foo, FooSym.getFlags()}, {Bar, BarSym.getFlags()}}),
       [&](MaterializationResponsibility R) {
-        R.notifyResolved(SymbolMap({{Foo, FooSym}, {Bar, BarSym}})), R.notifyEmitted();
+        cantFail(R.notifyResolved(SymbolMap({{Foo, FooSym}, {Bar, BarSym}})));
+        cantFail(R.notifyEmitted());
         BarMaterialized = true;
       });
 
@@ -650,8 +850,8 @@ TEST_F(CoreAPIsStandardTest, DefineMater
       [&](MaterializationResponsibility R) {
         cantFail(
             R.defineMaterializing(SymbolFlagsMap({{Bar, BarSym.getFlags()}})));
-        R.notifyResolved(SymbolMap({{Foo, FooSym}, {Bar, BarSym}}));
-        R.notifyEmitted();
+        cantFail(R.notifyResolved(SymbolMap({{Foo, FooSym}, {Bar, BarSym}})));
+        cantFail(R.notifyEmitted());
       });
 
   cantFail(JD.define(MU));
@@ -716,21 +916,22 @@ TEST_F(CoreAPIsStandardTest, FailResolut
 
   EXPECT_FALSE(!!Result) << "Expected failure";
   if (!Result) {
-    handleAllErrors(Result.takeError(),
-                    [&](FailedToMaterialize &F) {
-                      EXPECT_EQ(F.getSymbols(), Names)
-                          << "Expected to fail on symbols in Names";
-                    },
-                    [](ErrorInfoBase &EIB) {
-                      std::string ErrMsg;
-                      {
-                        raw_string_ostream ErrOut(ErrMsg);
-                        EIB.log(ErrOut);
-                      }
-                      ADD_FAILURE()
-                          << "Expected a FailedToResolve error. Got:\n"
-                          << ErrMsg;
-                    });
+    handleAllErrors(
+        Result.takeError(),
+        [&](FailedToMaterialize &F) {
+          EXPECT_TRUE(F.getSymbols().count(&JD))
+              << "Expected to fail on JITDylib JD";
+          EXPECT_EQ(F.getSymbols().find(&JD)->second, Names)
+              << "Expected to fail on symbols in Names";
+        },
+        [](ErrorInfoBase &EIB) {
+          std::string ErrMsg;
+          {
+            raw_string_ostream ErrOut(ErrMsg);
+            EIB.log(ErrOut);
+          }
+          ADD_FAILURE() << "Expected a FailedToResolve error. Got:\n" << ErrMsg;
+        });
   }
 }
 
@@ -741,7 +942,7 @@ TEST_F(CoreAPIsStandardTest, FailEmissio
   auto MU = std::make_unique<SimpleMaterializationUnit>(
       SymbolFlagsMap({{Foo, FooSym.getFlags()}, {Bar, BarSym.getFlags()}}),
       [&](MaterializationResponsibility R) {
-        R.notifyResolved(SymbolMap({{Foo, FooSym}, {Bar, BarSym}}));
+        cantFail(R.notifyResolved(SymbolMap({{Foo, FooSym}, {Bar, BarSym}})));
 
         ES.lookup(
             JITDylibSearchList({{&JD, false}}), SymbolNameSet({Baz}),
@@ -773,8 +974,8 @@ TEST_F(CoreAPIsStandardTest, TestLookupW
   auto MU = std::make_unique<SimpleMaterializationUnit>(
       SymbolFlagsMap({{Foo, JITSymbolFlags::Exported}}),
       [&](MaterializationResponsibility R) {
-        R.notifyResolved({{Foo, FooSym}});
-        R.notifyEmitted();
+        cantFail(R.notifyResolved({{Foo, FooSym}}));
+        cantFail(R.notifyEmitted());
       });
 
   cantFail(JD.define(MU));
@@ -832,15 +1033,15 @@ TEST_F(CoreAPIsStandardTest, TestGetRequ
         auto NewMU = std::make_unique<SimpleMaterializationUnit>(
             SymbolFlagsMap({{Bar, BarSym.getFlags()}}),
             [&](MaterializationResponsibility R2) {
-              R2.notifyResolved(SymbolMap({{Bar, BarSym}}));
-              R2.notifyEmitted();
+              cantFail(R2.notifyResolved(SymbolMap({{Bar, BarSym}})));
+              cantFail(R2.notifyEmitted());
               BarMaterialized = true;
             });
 
         R.replace(std::move(NewMU));
 
-        R.notifyResolved(SymbolMap({{Foo, FooSym}}));
-        R.notifyEmitted();
+        cantFail(R.notifyResolved(SymbolMap({{Foo, FooSym}})));
+        cantFail(R.notifyEmitted());
 
         FooMaterialized = true;
       });
@@ -871,10 +1072,10 @@ TEST_F(CoreAPIsStandardTest, TestMateria
       [&](MaterializationResponsibility R) {
         auto R2 = R.delegate({Bar});
 
-        R.notifyResolved({{Foo, FooSym}});
-        R.notifyEmitted();
-        R2.notifyResolved({{Bar, BarSym}});
-        R2.notifyEmitted();
+        cantFail(R.notifyResolved({{Foo, FooSym}}));
+        cantFail(R.notifyEmitted());
+        cantFail(R2.notifyResolved({{Bar, BarSym}}));
+        cantFail(R2.notifyEmitted());
       });
 
   cantFail(JD.define(MU));
@@ -924,8 +1125,9 @@ TEST_F(CoreAPIsStandardTest, TestMateria
       << "Expected a duplicate definition error";
   consumeError(std::move(Err));
 
-  FooResponsibility->notifyResolved(SymbolMap({{Foo, FooSym}}));
-  FooResponsibility->notifyEmitted();
+  // No dependencies registered, can't fail:
+  cantFail(FooResponsibility->notifyResolved(SymbolMap({{Foo, FooSym}})));
+  cantFail(FooResponsibility->notifyEmitted());
 }
 
 } // namespace

Modified: llvm/trunk/unittests/ExecutionEngine/Orc/LazyCallThroughAndReexportsTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/LazyCallThroughAndReexportsTest.cpp?rev=369808&r1=369807&r2=369808&view=diff
==============================================================================
--- llvm/trunk/unittests/ExecutionEngine/Orc/LazyCallThroughAndReexportsTest.cpp (original)
+++ llvm/trunk/unittests/ExecutionEngine/Orc/LazyCallThroughAndReexportsTest.cpp Fri Aug 23 13:37:31 2019
@@ -41,12 +41,13 @@ TEST_F(LazyReexportsTest, BasicLocalCall
       SymbolFlagsMap({{DummyTarget, JITSymbolFlags::Exported}}),
       [&](MaterializationResponsibility R) {
         DummyTargetMaterialized = true;
-        R.notifyResolved(
+        // No dependencies registered, can't fail.
+        cantFail(R.notifyResolved(
             {{DummyTarget,
               JITEvaluatedSymbol(static_cast<JITTargetAddress>(
                                      reinterpret_cast<uintptr_t>(&dummyTarget)),
-                                 JITSymbolFlags::Exported)}});
-        R.notifyEmitted();
+                                 JITSymbolFlags::Exported)}}));
+        cantFail(R.notifyEmitted());
       })));
 
   unsigned NotifyResolvedCount = 0;




More information about the llvm-commits mailing list