diff --git a/llvm/include/llvm/ExecutionEngine/Orc/Core.h b/llvm/include/llvm/ExecutionEngine/Orc/Core.h --- a/llvm/include/llvm/ExecutionEngine/Orc/Core.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/Core.h @@ -400,6 +400,16 @@ SymbolNameVector Symbols; }; +/// Returned from operations performed on closed JITDylibs and +/// MaterializationResponsibilities. +class JITDylibDestroyed : public ErrorInfo { +public: + static char ID; + + std::error_code convertToErrorCode() const override; + void log(raw_ostream &OS) const override; +}; + /// Tracks responsibility for materialization, and mediates interactions between /// MaterializationUnits and JDs. /// @@ -419,6 +429,17 @@ /// emitted or notified of an error. ~MaterializationResponsibility(); + /// Consume an Error from a MaterializationResponsibility operation and apply + /// the standard error handling rules (see below). Clients should abandon + /// materialization if this method returns true. + /// + /// (1) If the error is a MaterializationAbandoned error then return true (in + /// which case the client should bail out). + /// (2) If the error is any other failure value then call failMaterialization + /// then return true (in which case the client should bail out). + /// (3) If the error is a success value then return false. + bool shouldBailOut(Error Err); + /// Returns the target JITDylib that these symbols are being materialized /// into. JITDylib &getTargetJITDylib() const { return *JD; } @@ -439,10 +460,12 @@ /// information can be used to return responsibility for unrequested symbols /// back to the JITDylib via the delegate method. /// - /// If this method returns None then this MaterializationResponsibility has - /// been discarded and materialization should be abandoned. There is no - /// need to call failMaterialization in this case. - LLVM_NODISCARD Optional getRequestedSymbols() const; + /// This method may return the following error type(s): + /// + /// - JITDylibDestroyed: If the target JITDylib has been destroyed already. + /// in this case the client need not call failMaterialization (if called + /// it will be a no-op), but should abandon materialization. + Expected getRequestedSymbols() const; /// Notifies the target JITDylib that the given symbols have been resolved. /// This will update the given symbols' addresses in the JITDylib, and notify @@ -451,20 +474,33 @@ /// instance. Individual calls to this method may resolve a subset of the /// symbols, but all symbols must have been resolved prior to calling emit. /// - /// If this method returns None then this MaterializationResponsibility has - /// been discarded and materialization should be abandoned. There is no - /// need to call failMaterialization in this case. If this method returns an - /// Error value other than Error success. - LLVM_NODISCARD Optional notifyResolved(const SymbolMap &Symbols); + /// This method may return the following error type(s): + /// + /// - JITDylibDestroyed: If the target JITDylib has been destroyed already. + /// in this case the client need not call failMaterialization (if called + /// it will be a no-op), but should abandon materialization. + /// + /// - FailedToMaterialize: If any of the resolved symbols have been moved + /// to the error state due to dependence on some other failed symbol. + /// The client should report the error (via ExecutionSession::reportError), + /// call failMaterialization, and then abandon materialization. + 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. /// - /// If this method returns None then this MaterializationResponsibility has - /// been discarded and materialization should be abandoned. There is no - /// need to call failMaterialization in this case. - LLVM_NODISCARD Optional notifyEmitted(); + /// This method may return the following error type(s): + /// + /// - JITDylibDestroyed: If the target JITDylib has been destroyed already. + /// in this case the client need not call failMaterialization (if called + /// it will be a no-op), but should abandon materialization. + /// + /// - FailedToMaterialize: If any of the emitted symbols have been moved + /// to the error state due to dependence on some other failed symbol. + /// The client should report the error (via ExecutionSession::reportError), + /// call failMaterialization, and then abandon materialization. + Error notifyEmitted(); /// Attempt to claim responsibility for new definitions. This method can be /// used to claim responsibility for symbols that are added to a @@ -478,10 +514,17 @@ /// additional symbols at materialization time (e.g. stubs, compile /// callbacks, metadata). /// - /// If this method returns None then this MaterializationResponsibility has - /// been discarded and materialization should be abandoned. There is no - /// need to call failMaterialization in this case. - LLVM_NODISCARD Optional defineMaterializing(SymbolFlagsMap SymbolFlags); + /// This method may return the following error type(s): + /// + /// - JITDylibDestroyed: If the target JITDylib has been destroyed already. + /// in this case the client need not call failMaterialization (if called + /// it will be a no-op), but should abandon materialization. + /// + /// - DuplicateDefinition: If any of the new definitions clashes with an + /// existing definition in the target JITDylib. The client should report + /// the error (via ExecutionSession::reportError), call failMaterialization, + /// and then abandon materialization. + Error defineMaterializing(SymbolFlagsMap SymbolFlags); /// Notify all not-yet-emitted covered by this MaterializationResponsibility /// instance that an error has occurred. @@ -496,10 +539,12 @@ /// by introspecting which symbols have actually been looked up and /// materializing only those). /// - /// If this method returns false then this MaterializationResponsibility has - /// been discarded and materialization should be abandoned. There is no - /// need to call failMaterialization in this case. - LLVM_NODISCARD bool replace(std::unique_ptr MU); + /// This method may return the following error type(s): + /// + /// - JITDylibDestroyed: If the target JITDylib has been destroyed already. + /// in this case the client need not call failMaterialization (if called + /// it will be a no-op), but should abandon materialization. + Error replace(std::unique_ptr MU); /// Delegates responsibility for the given symbols to the returned /// materialization responsibility. Useful for breaking up work between @@ -508,18 +553,18 @@ /// Add dependencies that apply to symbol Name. /// - /// If this method returns false then this MaterializationResponsibility has - /// been discarded and materialization should be abandoned. There is no - /// need to call failMaterialization in this case. - LLVM_NODISCARD bool addDependencies(const SymbolStringPtr &Name, - const SymbolDependenceMap &Dependencies); + /// - JITDylibDestroyed: If the target JITDylib has been destroyed already. + /// in this case the client need not call failMaterialization (if called + /// it will be a no-op), but should abandon materialization. + Error addDependencies(const SymbolStringPtr &Name, + const SymbolDependenceMap &Dependencies); /// Add dependencies that apply to all symbols covered by this instance. /// - /// If this method returns false then this MaterializationResponsibility has - /// been discarded and materialization should be abandoned. There is no - /// need to call failMaterialization in this case. - LLVM_NODISCARD bool addDependenciesForAll(const SymbolDependenceMap &Dependencies); + /// - JITDylibDestroyed: If the target JITDylib has been destroyed already. + /// in this case the client need not call failMaterialization (if called + /// it will be a no-op), but should abandon materialization. + Error addDependenciesForAll(const SymbolDependenceMap &Dependencies); private: /// Create a MaterializationResponsibility for the given JITDylib and @@ -1027,15 +1072,15 @@ Error replace(std::unique_ptr MU); - Optional + Expected getRequestedSymbols(const SymbolFlagsMap &SymbolFlags) const; - bool addDependencies(const SymbolStringPtr &Name, - const SymbolDependenceMap &Dependants); + Error addDependencies(const SymbolStringPtr &Name, + const SymbolDependenceMap &Dependants); - bool resolve(const SymbolMap &Resolved); + Error resolve(const SymbolMap &Resolved); - bool emit(const SymbolFlagsMap &Emitted); + Error emit(const SymbolFlagsMap &Emitted); using FailedSymbolsWorklist = std::vector>; @@ -1045,7 +1090,7 @@ }; static NotifyFailedResult notifyFailed(FailedSymbolsWorklist FailedSymbols); - void prepareForRemoval(); + NotifyFailedResult prepareForRemoval(); ExecutionSession &ES; std::string JITDylibName; @@ -1137,6 +1182,9 @@ runSessionLocked([&]() { JITDylibDestructionListeners.push_back(&L); }); } + /// Deregister a JITDylibDestructionListener. + void deregisterJITDylibDestructionListener(JITDylibDestructionListener &L); + /// Return a pointer to the "name" JITDylib. /// Ownership of JITDylib remains within Execution Session JITDylib *getJITDylibByName(StringRef Name); @@ -1164,11 +1212,19 @@ /// Close a JITDylib. /// - /// This will delete the JITDylib and release any resources associated with - /// it. This method does *not* run static destructors: If supported they must - /// be run by the Platform prior to calling this method. + /// This will delete the JITDylib, release any resources associated with + /// it, and remove it from the link order of any JITDylibs that depend on it. + /// This method does not run static destructors, nor attempt to remove any + /// other JITDylibs Error destroyJITDylib(JITDylib &JD); + /// Returns a vector of current JITDylib pointers in reverse-preorder + /// according to their linkage relationships. + /// + /// This list is invalidated if any JITDylibs are created or removed, or + /// any new linakge relationships are added or removed from any JITDylib. + std::vector getJITDylibsInReverseLinkOrder(); + /// Allocate a module key for a new module to add to the JIT. VModuleKey allocateVModule() { return runSessionLocked([this]() { return ++LastKey; }); @@ -1213,9 +1269,9 @@ /// Search the given JITDylib list for the given symbols. /// /// SearchOrder lists the JITDylibs to search. For each dylib, the associated - /// boolean indicates whether the search should match against non-exported - /// (hidden visibility) symbols in that dylib (true means match against - /// non-exported symbols, false means do not match). + /// flags indicate whether the search should match against non-exported + /// (hidden visibility) symbols in that dylib, or all symbols. All JITDylibs + /// in the order must be valid (i.e. not destroyed yet). /// /// The NotifyComplete callback will be called once all requested symbols /// reach the required state. diff --git a/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h b/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h --- a/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h @@ -93,6 +93,9 @@ ObjectLinkingLayer(ExecutionSession &ES, std::unique_ptr MemMgr); + /// Destroy an ObjectLinkingLayer. + ~ObjectLinkingLayer(); + /// Set an object buffer return function. By default object buffers are /// deleted once the JIT has linked them. If a return function is set then /// it will be called to transfer ownership of the buffer instead. diff --git a/llvm/include/llvm/ExecutionEngine/Orc/OrcError.h b/llvm/include/llvm/ExecutionEngine/Orc/OrcError.h --- a/llvm/include/llvm/ExecutionEngine/Orc/OrcError.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/OrcError.h @@ -40,6 +40,7 @@ UnknownResourceHandle, MissingSymbolDefinitions, UnexpectedSymbolDefinitions, + JITDylibDestroyed }; std::error_code orcError(OrcErrorCode ErrCode); diff --git a/llvm/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h b/llvm/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h --- a/llvm/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h @@ -36,7 +36,8 @@ namespace llvm { namespace orc { -class RTDyldObjectLinkingLayer : public ObjectLayer { +class RTDyldObjectLinkingLayer : public ObjectLayer, + public JITDylibDestructionListener { public: /// Functor for receiving object-loaded notifications. using NotifyLoadedFunction = std::function O) override; + /// Handle JITDylib teardown notifications. + Error notifyDestroyingJITDylib(JITDylib &JD) override; + /// Set the NotifyLoaded callback. RTDyldObjectLinkingLayer &setNotifyLoaded(NotifyLoadedFunction NotifyLoaded) { this->NotifyLoaded = std::move(NotifyLoaded); @@ -140,7 +144,8 @@ bool ProcessAllSections = false; bool OverrideObjectFlags = false; bool AutoClaimObjectSymbols = false; - std::vector> MemMgrs; + DenseMap>> + MemMgrs; std::vector EventListeners; DenseMap> diff --git a/llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp b/llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp --- a/llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp +++ b/llvm/lib/ExecutionEngine/Orc/CompileOnDemandLayer.cpp @@ -163,12 +163,18 @@ return; } - if (!NonCallables.empty()) - R.replace(reexports(PDR.getImplDylib(), std::move(NonCallables), - JITDylibLookupFlags::MatchAllSymbols)); - if (!Callables.empty()) - R.replace(lazyReexports(LCTMgr, PDR.getISManager(), PDR.getImplDylib(), - std::move(Callables), AliaseeImpls)); + if (!NonCallables.empty()) { + if (R.shouldBailOut( + R.replace(reexports(PDR.getImplDylib(), std::move(NonCallables), + JITDylibLookupFlags::MatchAllSymbols)))) + return; + } + if (!Callables.empty()) { + if (R.shouldBailOut(R.replace( + lazyReexports(LCTMgr, PDR.getISManager(), PDR.getImplDylib(), + std::move(Callables), AliaseeImpls)))) + return; + } } CompileOnDemandLayer::PerDylibResources & @@ -255,7 +261,12 @@ auto &ES = getExecutionSession(); GlobalValueSet RequestedGVs; - for (auto &Name : R.getRequestedSymbols()) { + + auto RequestedSymbols = R.getRequestedSymbols(); + if (R.shouldBailOut(RequestedSymbols.takeError())) + return; + + for (auto &Name : *RequestedSymbols) { if (Name == R.getInitializerSymbol()) TSM.withModuleDo([&](Module &M) { for (auto &GV : getStaticInitGVs(M)) @@ -283,9 +294,12 @@ // If the partition is empty, return the whole module to the symbol table. if (GVsToExtract->empty()) { - R.replace(std::make_unique( - std::move(TSM), R.getSymbols(), R.getInitializerSymbol(), - std::move(Defs), *this)); + // We're going to bail out either way here, but we use shouldBailOut for + // the standard error handling. + R.shouldBailOut( + R.replace(std::make_unique( + std::move(TSM), R.getSymbols(), R.getInitializerSymbol(), + std::move(Defs), *this))); return; } @@ -350,8 +364,10 @@ return; } - R.replace(std::make_unique( - ES, *getManglingOptions(), std::move(TSM), *this)); + if (R.shouldBailOut( + R.replace(std::make_unique( + ES, *getManglingOptions(), std::move(TSM), *this)))) + return; BaseLayer.emit(std::move(R), std::move(*ExtractedTSM)); } diff --git a/llvm/lib/ExecutionEngine/Orc/Core.cpp b/llvm/lib/ExecutionEngine/Orc/Core.cpp --- a/llvm/lib/ExecutionEngine/Orc/Core.cpp +++ b/llvm/lib/ExecutionEngine/Orc/Core.cpp @@ -28,6 +28,7 @@ char SymbolsCouldNotBeRemoved::ID = 0; char MissingSymbolDefinitions::ID = 0; char UnexpectedSymbolDefinitions::ID = 0; +char JITDylibDestroyed::ID = 0; RegisterDependenciesFunction NoDependenciesToRegister = RegisterDependenciesFunction(); @@ -98,6 +99,14 @@ << ": " << Symbols; } +std::error_code JITDylibDestroyed::convertToErrorCode() const { + return orcError(OrcErrorCode::JITDylibDestroyed); +} + +void JITDylibDestroyed::log(raw_ostream &OS) const { + OS << "JITDylib already destroyed"; +} + AsynchronousSymbolQuery::AsynchronousSymbolQuery( const SymbolLookupSet &Symbols, SymbolState RequiredState, SymbolsResolvedCallback NotifyComplete) @@ -186,12 +195,30 @@ "All symbols should have been explicitly materialized or failed"); } - Optional +bool MaterializationResponsibility::shouldBailOut(Error Err) { + if (!Err) + return false; + + handleAllErrors( + std::move(Err), + [](const JITDylibDestroyed &) { + // If this is a JITDylibDestroyed error then we don't need to fail + // materialization. + }, + [&](std::unique_ptr EIB) -> Error { + JD->ES.reportError(Error(std::move(EIB))); + failMaterialization(); + return Error::success(); + }); + return true; +} + +Expected MaterializationResponsibility::getRequestedSymbols() const { return JD->getRequestedSymbols(SymbolFlags); } -bool MaterializationResponsibility::notifyResolved(const SymbolMap &Symbols) { +Error MaterializationResponsibility::notifyResolved(const SymbolMap &Symbols) { LLVM_DEBUG({ dbgs() << "In " << JD->getName() << " resolving " << Symbols << "\n"; }); @@ -401,11 +428,13 @@ if (!Aliases.empty()) { if (SourceJD) { - if (!R.replace(reexports(*SourceJD, std::move(Aliases), SourceJDLookupFlags))) + if (R.shouldBailOut(R.replace( + reexports(*SourceJD, std::move(Aliases), SourceJDLookupFlags)))) return; } else { - if (!R.replace(symbolAliases(std::move(Aliases)))) + if (R.shouldBailOut(R.replace(symbolAliases(std::move(Aliases))))) return; + } } // The OnResolveInfo struct will hold the aliases and responsibilty for each @@ -484,7 +513,9 @@ for (auto &KV : QueryInfo->Aliases) if (SrcJDDeps.count(KV.second.Aliasee)) { PerAliasDeps = {KV.second.Aliasee}; - QueryInfo->R.addDependencies(KV.first, PerAliasDepsMap); + if (QueryInfo->R.shouldBailOut( + QueryInfo->R.addDependencies(KV.first, PerAliasDepsMap))) + return; } }; @@ -614,7 +645,7 @@ // If this JITDylib has been closed then error out immediately. if (!Open) - return make_error(); + return make_error(); std::vector AddedSyms; std::vector RejectedWeakDefs; @@ -664,11 +695,10 @@ Error JITDylib::replace(std::unique_ptr MU) { assert(MU != nullptr && "Can not replace with a null MaterializationUnit"); - auto MustRunMU = - ES.runSessionLocked([&, this]() -> Expected> { - + auto MustRunMU = ES.runSessionLocked( + [&, this]() -> Expected> { if (!Open) - return make_error(); + return make_error(); #ifndef NDEBUG for (auto &KV : MU->getSymbols()) { @@ -721,18 +751,17 @@ if (*MustRunMU) { auto MR = (*MustRunMU)->createMaterializationResponsibility(shared_from_this()); - ES.dispatchMaterialization(std::move(MustRunMU), std::move(MR)); + ES.dispatchMaterialization(std::move(*MustRunMU), std::move(MR)); } return Error::success(); } -Optional +Expected JITDylib::getRequestedSymbols(const SymbolFlagsMap &SymbolFlags) const { - return ES.runSessionLocked([&]() { - + return ES.runSessionLocked([&]() -> Expected { if (!Open) - return None; + return make_error(); SymbolNameSet RequestedSymbols; @@ -751,106 +780,107 @@ RequestedSymbols.insert(KV.first); } - return RequestedSymbols; + return std::move(RequestedSymbols); }); } Error JITDylib::addDependencies(const SymbolStringPtr &Name, const SymbolDependenceMap &Dependencies) { - return runSessionLocked([&]() -> Error { - - if (!Open) - return make_error(); + return ES.runSessionLocked([&]() -> Error { + if (!Open) + return make_error(); - assert(Symbols.count(Name) && "Name not in symbol table"); - assert(Symbols[Name].getState() < SymbolState::Emitted && - "Can not add dependencies for a symbol that is not materializing"); + assert(Symbols.count(Name) && "Name not in symbol table"); + assert(Symbols[Name].getState() < SymbolState::Emitted && + "Can not add dependencies for a symbol that is not materializing"); - LLVM_DEBUG({ - dbgs() << "In " << getName() << " adding dependencies for " - << *Name << ": " << Dependencies << "\n"; - }); + LLVM_DEBUG({ + dbgs() << "In " << getName() << " adding dependencies for " << *Name + << ": " << Dependencies << "\n"; + }); - // If Name is already in an error state then just bail out. - if (Symbols[Name].getFlags().hasError()) - return; + // If Name is already in an error state then just bail out. + if (Symbols[Name].getFlags().hasError()) + return Error::success(); - auto &MI = MaterializingInfos[Name]; - assert(Symbols[Name].getState() != SymbolState::Emitted && - "Can not add dependencies to an emitted symbol"); + auto &MI = MaterializingInfos[Name]; + assert(Symbols[Name].getState() != SymbolState::Emitted && + "Can not add dependencies to an emitted symbol"); - bool DependsOnSymbolInErrorState = false; + 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]; + // 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) { + for (auto &OtherSymbol : KV.second) { - // Check the sym entry for the dependency. - auto OtherSymI = OtherJITDylib.Symbols.find(OtherSymbol); + // Check the sym entry for the dependency. + auto OtherSymI = OtherJITDylib.Symbols.find(OtherSymbol); - // Assert that this symbol exists and has not reached the ready state - // already. - assert(OtherSymI != OtherJITDylib.Symbols.end() && - "Dependency on unknown symbol"); + // Assert that this symbol exists and has not reached the ready state + // already. + assert(OtherSymI != OtherJITDylib.Symbols.end() && + "Dependency on unknown symbol"); - auto &OtherSymEntry = OtherSymI->second; + auto &OtherSymEntry = OtherSymI->second; - // If the other symbol is already in the Ready state then there's no - // dependency to add. - if (OtherSymEntry.getState() == SymbolState::Ready) - continue; + // If the other symbol is already in the Ready state then there's no + // dependency to add. + if (OtherSymEntry.getState() == SymbolState::Ready) + continue; - // If the dependency is in an error state then note this and continue, - // we will move this symbol to the error state below. - if (OtherSymEntry.getFlags().hasError()) { - DependsOnSymbolInErrorState = true; - continue; - } + // If the dependency is in an error state then note this and continue, + // we will move this symbol to the error state below. + if (OtherSymEntry.getFlags().hasError()) { + DependsOnSymbolInErrorState = true; + continue; + } - // If the dependency was not in the error state then add it to - // our list of dependencies. - auto &OtherMI = OtherJITDylib.MaterializingInfos[OtherSymbol]; + // If the dependency was not in the error state then add it to + // our list of dependencies. + auto &OtherMI = OtherJITDylib.MaterializingInfos[OtherSymbol]; - if (OtherSymEntry.getState() == SymbolState::Emitted) - transferEmittedNodeDependencies(MI, Name, OtherMI); - else if (&OtherJITDylib != this || OtherSymbol != Name) { - OtherMI.Dependants[this].insert(Name); - DepsOnOtherJITDylib.insert(OtherSymbol); - } + if (OtherSymEntry.getState() == SymbolState::Emitted) + transferEmittedNodeDependencies(MI, Name, OtherMI); + else if (&OtherJITDylib != this || OtherSymbol != Name) { + OtherMI.Dependants[this].insert(Name); + DepsOnOtherJITDylib.insert(OtherSymbol); } - - 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); + if (DepsOnOtherJITDylib.empty()) + MI.UnemittedDependencies.erase(&OtherJITDylib); + } - return Error::success(); - }); + // 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); + + return Error::success(); + }); } -bool JITDylib::resolve(const SymbolMap &Resolved) { - SymbolNameSet SymbolsInErrorState; +Error JITDylib::resolve(const SymbolMap &Resolved) { AsynchronousSymbolQuerySet CompletedQueries; - ES.runSessionLocked([&, this]() { - + auto SymbolsInErrorState = ES.runSessionLocked([&, this]() + -> Expected< + SymbolNameSet> { if (!Open) - false; + return make_error(); struct WorklistEntry { SymbolTable::iterator SymI; JITEvaluatedSymbol ResolvedSym; }; + SymbolNameSet SymbolsInErrorState; std::vector Worklist; Worklist.reserve(Resolved.size()); @@ -886,7 +916,7 @@ // If any symbols were in the error state then bail out. if (!SymbolsInErrorState.empty()) - return; + return SymbolsInErrorState; while (!Worklist.empty()) { auto SymI = Worklist.back().SymI; @@ -913,15 +943,20 @@ CompletedQueries.insert(std::move(Q)); } } + + return SymbolNameSet(); }); - assert((SymbolsInErrorState.empty() || CompletedQueries.empty()) && + if (!SymbolsInErrorState) + return SymbolsInErrorState.takeError(); + + 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()) { + if (!SymbolsInErrorState->empty()) { auto FailedSymbolsDepMap = std::make_shared(); - (*FailedSymbolsDepMap)[this] = std::move(SymbolsInErrorState); + (*FailedSymbolsDepMap)[this] = std::move(*SymbolsInErrorState); return make_error(std::move(FailedSymbolsDepMap)); } @@ -936,14 +971,15 @@ Error JITDylib::emit(const SymbolFlagsMap &Emitted) { AsynchronousSymbolQuerySet CompletedQueries; - SymbolNameSet SymbolsInErrorState; DenseMap ReadySymbols; - ES.runSessionLocked([&, this]() { - + auto SymbolsInErrorState = ES.runSessionLocked([&, this]() + -> Expected< + SymbolNameSet> { if (!Open) - return make_error(); + return make_error(); + SymbolNameSet SymbolsInErrorState; std::vector Worklist; // Scan to build worklist, record any symbols in the erorr state. @@ -961,7 +997,7 @@ // If any symbols were in the error state then bail out. if (!SymbolsInErrorState.empty()) - return; + return SymbolsInErrorState; // Otherwise update dependencies and move to the emitted state. while (!Worklist.empty()) { @@ -1060,15 +1096,20 @@ } } } + + return SymbolNameSet(); }); - assert((SymbolsInErrorState.empty() || CompletedQueries.empty()) && + if (!SymbolsInErrorState) + return SymbolsInErrorState.takeError(); + + 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()) { + if (!SymbolsInErrorState->empty()) { auto FailedSymbolsDepMap = std::make_shared(); - (*FailedSymbolsDepMap)[this] = std::move(SymbolsInErrorState); + (*FailedSymbolsDepMap)[this] = std::move(*SymbolsInErrorState); return make_error(std::move(FailedSymbolsDepMap)); } @@ -1081,13 +1122,15 @@ return Error::success(); } -NotifyFailedResult JITDylib::notifyFailed(FailedSymbolsWorklist Worklist) { - NotifyFailedResult NFR; - NFR.FailedSymbolsMap = std::make_shared(); +JITDylib::NotifyFailedResult +JITDylib::notifyFailed(FailedSymbolsWorklist Worklist) { // Failing no symbols is a no-op. if (Worklist.empty()) - return; + return NotifyFailedResult(); + + NotifyFailedResult NFR; + NFR.FailedSymbolsMap = std::make_shared(); auto &ES = Worklist.front().first->getExecutionSession(); @@ -1202,15 +1245,15 @@ return NFR; } -NotifyFailedResult JITDylib::prepareForRemoval() { +JITDylib::NotifyFailedResult JITDylib::prepareForRemoval() { // Order is important here: // 1. Notify materializing symbols that they've failed (collect queries to // fail but don't execute them). // 2. Move the JITDylib to a closed state. // 3. Clean up other members (except the JITDylib name). - FailSymbolsWorklist FSW; - for (auto &KV : MaterializingInfosMap) + FailedSymbolsWorklist FSW; + for (auto &KV : MaterializingInfos) FSW.push_back(std::make_pair(this, KV.first)); auto NFR = notifyFailed(std::move(FSW)); Open = false; @@ -1221,6 +1264,8 @@ UnmaterializedInfos.clear(); MaterializingInfos.clear(); LinkOrder.clear(); + + return NFR; } void JITDylib::setLinkOrder(JITDylibSearchOrder NewLinkOrder, @@ -1841,6 +1886,21 @@ : SSP(SSP ? std::move(SSP) : std::make_shared()) { } +void ExecutionSession::deregisterJITDylibDestructionListener( + JITDylibDestructionListener &L) { + runSessionLocked([&]() { + auto JDI = JITDylibDestructionListeners.rbegin(); + auto JDE = JITDylibDestructionListeners.rend(); + for (; JDI != JDE; ++JDI) { + if (*JDI == &L) { + JITDylibDestructionListeners.erase(JDI.base() - 1); + break; + } + } + assert(JDI != JDE && "No such listener registered"); + }); +} + JITDylib *ExecutionSession::getJITDylibByName(StringRef Name) { return runSessionLocked([&, this]() -> JITDylib * { for (auto &JD : JDs) @@ -1853,6 +1913,7 @@ JITDylib &ExecutionSession::createBareJITDylib(std::string Name) { assert(!getJITDylibByName(Name) && "JITDylib with that name already exists"); return runSessionLocked([&, this]() -> JITDylib & { + LLVM_DEBUG(dbgs() << "Creating JITDylib \"" << Name << "\"\n"); JDs.push_back( std::shared_ptr(new JITDylib(*this, std::move(Name)))); return *JDs.back(); @@ -1872,12 +1933,16 @@ auto NFR = runSessionLocked([&]() { // Remove JD from JDs list, and unlink it from all other JITDylibs. + + assert(JD.Open && "Duplicate call to destroyJITDylib?"); + + LLVM_DEBUG(dbgs() << "Destroying JITDylib \"" << JD.getName() << "\"\n"); for (auto JDI = JDs.begin(); JDI != JDs.end();) { if (JDI->get() == &JD) { OwnedJD = std::move(*JDI); JDI = JDs.erase(JDI); } else { - (*JDI)->removeFromSearchOrder(JD); + (*JDI)->removeFromLinkOrder(JD); ++JDI; } } @@ -1898,6 +1963,39 @@ return Err; } +std::vector ExecutionSession::getJITDylibsInReverseLinkOrder() { + return runSessionLocked([&]() { + std::vector Result; + Result.reserve(JDs.size()); + + std::vector Worklist; + Worklist.reserve(JDs.size()); + DenseSet Visited; + + for (auto &TopLevelJD : JDs) { + + Worklist.push_back(TopLevelJD.get()); + + while (!Worklist.empty()) { + auto &JD = *Worklist.back(); + Worklist.pop_back(); + if (Visited.count(&JD)) + continue; + Visited.insert(&JD); + + JD.withLinkOrderDo([&](const JITDylibSearchOrder &LinkOrder) { + for (auto &KV : reverse(LinkOrder)) + Worklist.push_back(KV.first); + }); + Result.push_back(&JD); + } + } + + std::reverse(Result.begin(), Result.end()); + return Result; + }); +} + void ExecutionSession::legacyFailQuery(AsynchronousSymbolQuery &Q, Error Err) { assert(!!Err && "Error should be in failure state"); @@ -2041,9 +2139,14 @@ Q->detach(); // Replace the MUs. + // + // We can 'cantFail' this replace operation because lookup's contract + // requires all JITDylibs in the search order to be valid, and the + // only way replace can fail is if the target has been destroyed + // already. for (auto &KV : CollectedMUsMap) for (auto &MU : KV.second) - KV.first->replace(std::move(MU)); + cantFail(KV.first->replace(std::move(MU))); return Err; } diff --git a/llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp b/llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp --- a/llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp +++ b/llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp @@ -128,8 +128,11 @@ MaterializationResponsibility R) { auto RequestedSymbols = R.getRequestedSymbols(); + if (R.shouldBailOut(RequestedSymbols.takeError())) + return; + SymbolAliasMap RequestedAliases; - for (auto &RequestedSymbol : RequestedSymbols) { + for (auto &RequestedSymbol : *RequestedSymbols) { auto I = CallableAliases.find(RequestedSymbol); assert(I != CallableAliases.end() && "Symbol not found in alias map?"); RequestedAliases[I->first] = std::move(I->second); @@ -137,8 +140,10 @@ } if (!CallableAliases.empty()) - R.replace(lazyReexports(LCTManager, ISManager, SourceJD, - std::move(CallableAliases), AliaseeTable)); + if (R.shouldBailOut( + R.replace(lazyReexports(LCTManager, ISManager, SourceJD, + std::move(CallableAliases), AliaseeTable)))) + return; IndirectStubsManager::StubInitsMap StubInits; for (auto &Alias : RequestedAliases) { diff --git a/llvm/lib/ExecutionEngine/Orc/Legacy.cpp b/llvm/lib/ExecutionEngine/Orc/Legacy.cpp --- a/llvm/lib/ExecutionEngine/Orc/Legacy.cpp +++ b/llvm/lib/ExecutionEngine/Orc/Legacy.cpp @@ -43,7 +43,7 @@ auto Unresolved = R.lookup(Q, InternedSymbols); if (Unresolved.empty()) { if (MR) - MR->addDependenciesForAll(Q->QueryRegistrations); + consumeError(MR->addDependenciesForAll(Q->QueryRegistrations)); } else ES.legacyFailQuery(*Q, make_error(std::move(Unresolved))); } diff --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp --- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp +++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp @@ -70,6 +70,15 @@ LookupSet.add(ES.intern(KV.first), LookupFlags); } + for (auto &KV : InternalNamedSymbolDeps) { + SymbolDependenceMap InternalDeps; + InternalDeps[&MR.getTargetJITDylib()] = std::move(KV.second); + if (auto Err = MR.addDependencies(KV.first, InternalDeps)) { + LC->run(std::move(Err)); + return; + } + } + // OnResolve -- De-intern the symbols and pass the result to the linker. auto OnResolve = [this, LookupContinuation = std::move(LC)]( Expected Result) mutable { @@ -84,12 +93,6 @@ } }; - for (auto &KV : InternalNamedSymbolDeps) { - SymbolDependenceMap InternalDeps; - InternalDeps[&MR.getTargetJITDylib()] = std::move(KV.second); - MR.addDependencies(KV.first, InternalDeps); - } - ES.lookup(LookupKind::Static, LinkOrder, std::move(LookupSet), SymbolState::Resolved, std::move(OnResolve), [this](const SymbolDependenceMap &Deps) { @@ -434,7 +437,8 @@ SymbolDeps.erase(&SourceJD); } - MR.addDependencies(Name, SymbolDeps); + if (MR.shouldBailOut(MR.addDependencies(Name, SymbolDeps))) + return; } } @@ -453,6 +457,10 @@ ES.registerJITDylibDestructionListener(*this); } +ObjectLinkingLayer::~ObjectLinkingLayer() { + getExecutionSession().deregisterJITDylibDestructionListener(*this); +} + void ObjectLinkingLayer::emit(MaterializationResponsibility R, std::unique_ptr O) { assert(O && "Object must not be null"); diff --git a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp --- a/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp +++ b/llvm/lib/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.cpp @@ -44,7 +44,7 @@ // Register dependencies for all symbols contained in this set. auto RegisterDependencies = [&](const SymbolDependenceMap &Deps) { - MR.addDependenciesForAll(Deps); + consumeError(MR.addDependenciesForAll(Deps)); }; JITDylibSearchOrder LinkOrder; @@ -77,16 +77,24 @@ RTDyldObjectLinkingLayer::RTDyldObjectLinkingLayer( ExecutionSession &ES, GetMemoryManagerFunction GetMemoryManager) - : ObjectLayer(ES), GetMemoryManager(GetMemoryManager) {} + : ObjectLayer(ES), GetMemoryManager(GetMemoryManager) { + ES.registerJITDylibDestructionListener(*this); +} RTDyldObjectLinkingLayer::~RTDyldObjectLinkingLayer() { - std::lock_guard Lock(RTDyldLayerMutex); - for (auto &MemMgr : MemMgrs) { - for (auto *L : EventListeners) - L->notifyFreeingObject( - static_cast(reinterpret_cast(MemMgr.get()))); - MemMgr->deregisterEHFrames(); + { + std::lock_guard Lock(RTDyldLayerMutex); + assert(LoadedObjInfos.empty() && "LoadedObjInfos still attached"); + for (auto &KV : MemMgrs) { + for (auto &MemMgr : KV.second) { + for (auto *L : EventListeners) + L->notifyFreeingObject( + static_cast(reinterpret_cast(MemMgr.get()))); + MemMgr->deregisterEHFrames(); + } + } } + getExecutionSession().deregisterJITDylibDestructionListener(*this); } void RTDyldObjectLinkingLayer::emit(MaterializationResponsibility R, @@ -121,7 +129,7 @@ continue; } else { ES.reportError(SymType.takeError()); - R.failMaterialization(); + SharedR->failMaterialization(); return; } @@ -129,7 +137,7 @@ if (!SymFlagsOrErr) { // TODO: Test this error. ES.reportError(SymFlagsOrErr.takeError()); - R.failMaterialization(); + SharedR->failMaterialization(); return; } @@ -139,7 +147,7 @@ InternalSymbols->insert(*SymName); else { ES.reportError(SymName.takeError()); - R.failMaterialization(); + SharedR->failMaterialization(); return; } } @@ -151,8 +159,9 @@ { auto Tmp = GetMemoryManager(); std::lock_guard Lock(RTDyldLayerMutex); - MemMgrs.push_back(std::move(Tmp)); - MemMgr = MemMgrs.back().get(); + auto &JDMemMgrs = MemMgrs[&SharedR->getTargetJITDylib()]; + JDMemMgrs.push_back(std::move(Tmp)); + MemMgr = JDMemMgrs.back().get(); } JITDylibSearchOrderResolver Resolver(*SharedR); @@ -173,6 +182,22 @@ }); } +Error RTDyldObjectLinkingLayer::notifyDestroyingJITDylib(JITDylib &JD) { + std::lock_guard Lock(RTDyldLayerMutex); + auto JDI = MemMgrs.find(&JD); + if (JDI != MemMgrs.end()) { + auto &JDMemMgrs = JDI->second; + for (auto &MemMgr : JDMemMgrs) { + for (auto *L : EventListeners) + L->notifyFreeingObject( + static_cast(reinterpret_cast(MemMgr.get()))); + MemMgr->deregisterEHFrames(); + } + MemMgrs.erase(JDI); + } + return Error::success(); +} + void RTDyldObjectLinkingLayer::registerJITEventListener(JITEventListener &L) { std::lock_guard Lock(RTDyldLayerMutex); assert(llvm::none_of(EventListeners, diff --git a/llvm/lib/ExecutionEngine/OrcError/OrcError.cpp b/llvm/lib/ExecutionEngine/OrcError/OrcError.cpp --- a/llvm/lib/ExecutionEngine/OrcError/OrcError.cpp +++ b/llvm/lib/ExecutionEngine/OrcError/OrcError.cpp @@ -65,6 +65,8 @@ return "MissingSymbolsDefinitions"; case OrcErrorCode::UnexpectedSymbolDefinitions: return "UnexpectedSymbolDefinitions"; + case OrcErrorCode::JITDylibDestroyed: + return "JITDylib already destroyed"; } llvm_unreachable("Unhandled error code"); } diff --git a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp --- a/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp +++ b/llvm/unittests/ExecutionEngine/Orc/CoreAPIsTest.cpp @@ -443,7 +443,7 @@ SymbolLookupSet({Foo}), SymbolState::Ready, OnCompletion, NoDependenciesToRegister); - FooR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}}); + cantFail(FooR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}})); EXPECT_THAT_ERROR(FooR->notifyResolved({{Foo, FooSym}}), Succeeded()) << "No symbols marked failed, but Foo failed to resolve"; EXPECT_THAT_ERROR(FooR->notifyEmitted(), Succeeded()) @@ -550,15 +550,15 @@ NoDependenciesToRegister); // Add a circular dependency: Foo -> Bar, Bar -> Baz, Baz -> Foo. - FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}}); - BarR->addDependenciesForAll({{&JD, SymbolNameSet({Baz})}}); - BazR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}}); + cantFail(FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}})); + cantFail(BarR->addDependenciesForAll({{&JD, SymbolNameSet({Baz})}})); + cantFail(BazR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}})); // Add self-dependencies for good measure. This tests that the implementation // of addDependencies filters these out. - FooR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}}); - BarR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}}); - BazR->addDependenciesForAll({{&JD, SymbolNameSet({Baz})}}); + cantFail(FooR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}})); + cantFail(BarR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}})); + cantFail(BazR->addDependenciesForAll({{&JD, SymbolNameSet({Baz})}})); // Check that nothing has been resolved yet. EXPECT_FALSE(FooResolved) << "\"Foo\" should not be resolved yet"; @@ -642,7 +642,7 @@ NoDependenciesToRegister); // Add a dependency by Foo on Bar. - FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}}); + cantFail(FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}})); // Fail bar. BarR->failMaterialization(); @@ -707,8 +707,8 @@ NoDependenciesToRegister); // Add a dependency by Foo on Bar and vice-versa. - FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}}); - BarR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}}); + cantFail(FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}})); + cantFail(BarR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}})); // Fail bar. BarR->failMaterialization(); @@ -781,7 +781,7 @@ EXPECT_FALSE(OnFooReadyRun) << "Query for \"Foo\" should not have run yet"; // Add dependency of Foo on Bar. - FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}}); + cantFail(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()) @@ -839,8 +839,8 @@ NoDependenciesToRegister); // Add a dependency by Foo on Bar and vice-versa. - FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}}); - BarR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}}); + cantFail(FooR->addDependenciesForAll({{&JD, SymbolNameSet({Bar})}})); + cantFail(BarR->addDependenciesForAll({{&JD, SymbolNameSet({Foo})}})); // Materialize Foo. EXPECT_THAT_ERROR(FooR->notifyResolved({{Foo, FooSym}}), Succeeded()) @@ -1127,7 +1127,7 @@ R.failMaterialization(); }, [&](const SymbolDependenceMap &Deps) { - R.addDependenciesForAll(Deps); + cantFail(R.addDependenciesForAll(Deps)); }); }); @@ -1221,7 +1221,7 @@ auto MU = std::make_unique( SymbolFlagsMap({{Foo, FooSym.getFlags()}, {Bar, BarSym.getFlags()}}), [&](MaterializationResponsibility R) { - auto Requested = R.getRequestedSymbols(); + auto Requested = cantFail(R.getRequestedSymbols()); EXPECT_EQ(Requested.size(), 1U) << "Expected one symbol requested"; EXPECT_EQ(*Requested.begin(), Foo) << "Expected \"Foo\" requested"; @@ -1233,7 +1233,7 @@ BarMaterialized = true; }); - R.replace(std::move(NewMU)); + cantFail(R.replace(std::move(NewMU))); cantFail(R.notifyResolved(SymbolMap({{Foo, FooSym}}))); cantFail(R.notifyEmitted());