From 7565b20b50b254a72efa9d505e92be65c664b1b2 Mon Sep 17 00:00:00 2001 From: Lang Hames Date: Tue, 30 Apr 2024 22:55:29 -0930 Subject: [PATCH] [ORC] Switch ObjectLinkingLayer::Plugins to shared ownership, copy pipeline. Previously ObjectLinkingLayer held unique ownership of Plugins, and links always used the Layer's plugin list at each step. This can cause problems if plugins are added while links are in progress however, as the newly added plugin may receive only some of the callbacks for links that are already running. In this patch each link gets its own copy of the pipeline that remains consistent throughout the link's lifetime, and it is guaranteed that Plugin objects (now with shared ownership) will remain valid until the link completes. Coding my way home: 9.80469S, 139.03167W --- .../ExecutionEngine/Orc/ObjectLinkingLayer.h | 11 +-- .../Orc/ObjectLinkingLayer.cpp | 73 ++++++++++--------- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h b/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h index 34f2e0789462c..e452b90598a0a 100644 --- a/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h +++ b/llvm/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h @@ -122,7 +122,7 @@ class ObjectLinkingLayer : public RTTIExtends, } /// Add a pass-config modifier. - ObjectLinkingLayer &addPlugin(std::unique_ptr P) { + ObjectLinkingLayer &addPlugin(std::shared_ptr P) { std::lock_guard Lock(LayerMutex); Plugins.push_back(std::move(P)); return *this; @@ -181,11 +181,8 @@ class ObjectLinkingLayer : public RTTIExtends, private: using FinalizedAlloc = jitlink::JITLinkMemoryManager::FinalizedAlloc; - void modifyPassConfig(MaterializationResponsibility &MR, - jitlink::LinkGraph &G, - jitlink::PassConfiguration &PassConfig); - void notifyLoaded(MaterializationResponsibility &MR); - Error notifyEmitted(MaterializationResponsibility &MR, FinalizedAlloc FA); + Error recordFinalizedAlloc(MaterializationResponsibility &MR, + FinalizedAlloc FA); Error handleRemoveResources(JITDylib &JD, ResourceKey K) override; void handleTransferResources(JITDylib &JD, ResourceKey DstKey, @@ -198,7 +195,7 @@ class ObjectLinkingLayer : public RTTIExtends, bool AutoClaimObjectSymbols = false; ReturnObjectBufferFunction ReturnObjectBuffer; DenseMap> Allocs; - std::vector> Plugins; + std::vector> Plugins; }; class EHFrameRegistrationPlugin : public ObjectLinkingLayer::Plugin { diff --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp index 131728fd7e7e4..57ade94767865 100644 --- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp +++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp @@ -156,7 +156,10 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext { std::unique_ptr MR, std::unique_ptr ObjBuffer) : JITLinkContext(&MR->getTargetJITDylib()), Layer(Layer), - MR(std::move(MR)), ObjBuffer(std::move(ObjBuffer)) {} + MR(std::move(MR)), ObjBuffer(std::move(ObjBuffer)) { + std::lock_guard Lock(Layer.LayerMutex); + Plugins = Layer.Plugins; + } ~ObjectLinkingLayerJITLinkContext() { // If there is an object buffer return function then use it to @@ -168,14 +171,14 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext { JITLinkMemoryManager &getMemoryManager() override { return Layer.MemMgr; } void notifyMaterializing(LinkGraph &G) { - for (auto &P : Layer.Plugins) + for (auto &P : Plugins) P->notifyMaterializing(*MR, G, *this, ObjBuffer ? ObjBuffer->getMemBufferRef() : MemoryBufferRef()); } void notifyFailed(Error Err) override { - for (auto &P : Layer.Plugins) + for (auto &P : Plugins) Err = joinErrors(std::move(Err), P->notifyFailed(*MR)); Layer.getExecutionSession().reportError(std::move(Err)); MR->failMaterialization(); @@ -317,12 +320,12 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext { if (auto Err = MR->notifyResolved(InternedResult)) return Err; - Layer.notifyLoaded(*MR); + notifyLoaded(); return Error::success(); } void notifyFinalized(JITLinkMemoryManager::FinalizedAlloc A) override { - if (auto Err = Layer.notifyEmitted(*MR, std::move(A))) { + if (auto Err = notifyEmitted(std::move(A))) { Layer.getExecutionSession().reportError(std::move(Err)); MR->failMaterialization(); return; @@ -344,7 +347,8 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext { return claimOrExternalizeWeakAndCommonSymbols(G); }); - Layer.modifyPassConfig(*MR, LG, Config); + for (auto &P : Plugins) + P->modifyPassConfig(*MR, LG, Config); Config.PreFixupPasses.push_back( [this](LinkGraph &G) { return registerDependencies(G); }); @@ -352,6 +356,29 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext { return Error::success(); } + void notifyLoaded() { + for (auto &P : Plugins) + P->notifyLoaded(*MR); + } + + Error notifyEmitted(jitlink::JITLinkMemoryManager::FinalizedAlloc FA) { + Error Err = Error::success(); + for (auto &P : Plugins) + Err = joinErrors(std::move(Err), P->notifyEmitted(*MR)); + + if (Err) { + if (FA) + Err = + joinErrors(std::move(Err), Layer.MemMgr.deallocate(std::move(FA))); + return Err; + } + + if (FA) + return Layer.recordFinalizedAlloc(*MR, std::move(FA)); + + return Error::success(); + } + private: // Symbol name dependencies: // Internal: Defined in this graph. @@ -522,7 +549,7 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext { SymbolDependenceGroup SynthSDG; - for (auto &P : Layer.Plugins) { + for (auto &P : Plugins) { auto SynthDeps = P->getSyntheticSymbolDependencies(*MR); if (SynthDeps.empty()) continue; @@ -636,6 +663,7 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext { } ObjectLinkingLayer &Layer; + std::vector> Plugins; std::unique_ptr MR; std::unique_ptr ObjBuffer; DenseMap ExternalBlockDeps; @@ -702,34 +730,9 @@ void ObjectLinkingLayer::emit(std::unique_ptr R, link(std::move(G), std::move(Ctx)); } -void ObjectLinkingLayer::modifyPassConfig(MaterializationResponsibility &MR, - LinkGraph &G, - PassConfiguration &PassConfig) { - for (auto &P : Plugins) - P->modifyPassConfig(MR, G, PassConfig); -} - -void ObjectLinkingLayer::notifyLoaded(MaterializationResponsibility &MR) { - for (auto &P : Plugins) - P->notifyLoaded(MR); -} - -Error ObjectLinkingLayer::notifyEmitted(MaterializationResponsibility &MR, - FinalizedAlloc FA) { - Error Err = Error::success(); - for (auto &P : Plugins) - Err = joinErrors(std::move(Err), P->notifyEmitted(MR)); - - if (Err) { - if (FA) - Err = joinErrors(std::move(Err), MemMgr.deallocate(std::move(FA))); - return Err; - } - - if (!FA) - return Error::success(); - - Err = MR.withResourceKeyDo( +Error ObjectLinkingLayer::recordFinalizedAlloc( + MaterializationResponsibility &MR, FinalizedAlloc FA) { + auto Err = MR.withResourceKeyDo( [&](ResourceKey K) { Allocs[K].push_back(std::move(FA)); }); if (Err)