[llvm] r257263 - [Orc][RuntimeDyld] Prevent duplicate calls to finalizeMemory on shared memory
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 14 15:25:36 PST 2016
On Sat, Jan 9, 2016 at 11:50 AM, Lang Hames via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: lhames
> Date: Sat Jan 9 13:50:40 2016
> New Revision: 257263
>
> URL: http://llvm.org/viewvc/llvm-project?rev=257263&view=rev
> Log:
> [Orc][RuntimeDyld] Prevent duplicate calls to finalizeMemory on shared
> memory
> managers.
>
> Prior to this patch, recursive finalization (where finalization of one
> RuntimeDyld instance triggers finalization of another instance on which the
> first depends) could trigger memory access failures: When the inner
> (dependent)
> RuntimeDyld instance and its memory manager are finalized, memory allocated
> (but not yet relocated) by the outer instance is locked, and relocation in
> the
> outer instance fails with a memory access error.
>
> This patch adds a latch to the RuntimeDyld::MemoryManager base class that
> is
> checked by a new method: RuntimeDyld::finalizeWithMemoryManagerLocking,
> ensuring
> that shared memory managers are only finalized by the outermost RuntimeDyld
> instance.
>
> This allows ORC clients to supply the same memory manager to multiple
> calls to
> addModuleSet. In particular it enables the use of user-supplied memory
> managers
> with the CompileOnDemandLayer which must reuse the supplied memory manager
> for
> each function that is lazily compiled.
>
Rather than having MemoryManager befriending RuntimeDyld, would it make
sense to have a MemoryManager ref counting decorator? If there are multiple
simultaneous users of a MemoryManager, is it a constraint that all clients
of the MemoryManager must finalize it before it can be truly finalized?
(it's not clear to me from this change if that is a constraint - I guess it
probably /isn't/? But then what happens to the other clients that never
participated in this finalize-with-locking - wouldn't they try to finalize
(or even do pre-finalization tasks) at some later point?)
- Dave
>
>
> Modified:
> llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
> llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h
> llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
> llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
> llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp
> llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h
>
> Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h?rev=257263&r1=257262&r2=257263&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h
> (original)
> +++ llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h Sat
> Jan 9 13:50:40 2016
> @@ -108,9 +108,7 @@ private:
>
> void Finalize() override {
> State = Finalizing;
> - RTDyld->resolveRelocations();
> - RTDyld->registerEHFrames();
> - MemMgr->finalizeMemory();
> + RTDyld->finalizeWithMemoryManagerLocking();
> State = Finalized;
> }
>
>
> Modified: llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h?rev=257263&r1=257262&r2=257263&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h (original)
> +++ llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h Sat Jan 9
> 13:50:40 2016
> @@ -95,7 +95,9 @@ public:
>
> /// \brief Memory Management.
> class MemoryManager {
> + friend class RuntimeDyld;
> public:
> + MemoryManager() : FinalizationLocked(false) {}
> virtual ~MemoryManager() {}
>
> /// Allocate a memory block of (at least) the given size suitable for
> @@ -153,6 +155,7 @@ public:
>
> private:
> virtual void anchor();
> + bool FinalizationLocked;
> };
>
> /// \brief Symbol resolution.
> @@ -241,6 +244,25 @@ public:
> this->ProcessAllSections = ProcessAllSections;
> }
>
> + /// Perform all actions needed to make the code owned by this
> RuntimeDyld
> + /// instance executable:
> + ///
> + /// 1) Apply relocations.
> + /// 2) Register EH frames.
> + /// 3) Update memory permissions*.
> + ///
> + /// * Finalization is potentially recursive**, and the 3rd step will
> only be
> + /// applied by the outermost call to finalize. This allows different
> + /// RuntimeDyld instances to share a memory manager without the
> innermost
> + /// finalization locking the memory and causing relocation fixup
> errors in
> + /// outer instances.
> + ///
> + /// ** Recursive finalization occurs when one RuntimeDyld instances
> needs the
> + /// address of a symbol owned by some other instance in order to apply
> + /// relocations.
> + ///
> + void finalizeWithMemoryManagerLocking();
> +
> private:
> // RuntimeDyldImpl is the actual class. RuntimeDyld is just the public
> // interface.
>
> Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=257263&r1=257262&r2=257263&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp (original)
> +++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp Sat Jan 9
> 13:50:40 2016
> @@ -967,6 +967,17 @@ bool RuntimeDyld::hasError() { return Dy
>
> StringRef RuntimeDyld::getErrorString() { return Dyld->getErrorString(); }
>
> +void RuntimeDyld::finalizeWithMemoryManagerLocking() {
> + bool MemoryFinalizationLocked = MemMgr.FinalizationLocked;
> + MemMgr.FinalizationLocked = true;
> + resolveRelocations();
> + registerEHFrames();
> + if (!MemoryFinalizationLocked) {
> + MemMgr.finalizeMemory();
> + MemMgr.FinalizationLocked = false;
> + }
> +}
> +
> void RuntimeDyld::registerEHFrames() {
> if (Dyld)
> Dyld->registerEHFrames();
>
> Modified:
> llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp?rev=257263&r1=257262&r2=257263&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
> (original)
> +++ llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
> Sat Jan 9 13:50:40 2016
> @@ -7,6 +7,7 @@
> //
>
> //===----------------------------------------------------------------------===//
>
> +#include "OrcTestCommon.h"
> #include "llvm/ExecutionEngine/ExecutionEngine.h"
> #include "llvm/ExecutionEngine/SectionMemoryManager.h"
> #include "llvm/ExecutionEngine/Orc/CompileUtils.h"
> @@ -21,6 +22,10 @@ using namespace llvm::orc;
>
> namespace {
>
> +class ObjectLinkingLayerExecutionTest : public testing::Test,
> + public OrcExecutionTest {
> +};
> +
> TEST(ObjectLinkingLayerTest, TestSetProcessAllSections) {
>
> class SectionMemoryManagerWrapper : public SectionMemoryManager {
> @@ -91,4 +96,81 @@ TEST(ObjectLinkingLayerTest, TestSetProc
> }
> }
>
> +
> +TEST_F(ObjectLinkingLayerExecutionTest, NoDuplicateFinalization) {
> +
> + if (!TM)
> + return;
> +
> + class SectionMemoryManagerWrapper : public SectionMemoryManager {
> + public:
> + int FinalizationCount = 0;
> + bool finalizeMemory(std::string *ErrMsg = 0) override {
> + ++FinalizationCount;
> + return SectionMemoryManager::finalizeMemory(ErrMsg);
> + }
> + };
> +
> + ObjectLinkingLayer<> ObjLayer;
> + SimpleCompiler Compile(*TM);
> +
> + // Create a pair of modules that will trigger recursive finalization:
> + // Module 1:
> + // int bar() { return 42; }
> + // Module 2:
> + // int bar();
> + // int foo() { return bar(); }
> +
> + ModuleBuilder MB1(getGlobalContext(), "", "dummy");
> + {
> + MB1.getModule()->setDataLayout(TM->createDataLayout());
> + Function *BarImpl = MB1.createFunctionDecl<int32_t(void)>("bar");
> + BasicBlock *BarEntry = BasicBlock::Create(getGlobalContext(), "entry",
> + BarImpl);
> + IRBuilder<> Builder(BarEntry);
> + IntegerType *Int32Ty = IntegerType::get(getGlobalContext(), 32);
> + Value *FourtyTwo = ConstantInt::getSigned(Int32Ty, 42);
> + Builder.CreateRet(FourtyTwo);
> + }
> +
> + auto Obj1 = Compile(*MB1.getModule());
> + std::vector<object::ObjectFile*> Obj1Set;
> + Obj1Set.push_back(Obj1.getBinary());
> +
> + ModuleBuilder MB2(getGlobalContext(), "", "dummy");
> + {
> + MB2.getModule()->setDataLayout(TM->createDataLayout());
> + Function *BarDecl = MB2.createFunctionDecl<int32_t(void)>("bar");
> + Function *FooImpl = MB2.createFunctionDecl<int32_t(void)>("foo");
> + BasicBlock *FooEntry = BasicBlock::Create(getGlobalContext(), "entry",
> + FooImpl);
> + IRBuilder<> Builder(FooEntry);
> + Builder.CreateRet(Builder.CreateCall(BarDecl));
> + }
> + auto Obj2 = Compile(*MB2.getModule());
> + std::vector<object::ObjectFile*> Obj2Set;
> + Obj2Set.push_back(Obj2.getBinary());
> +
> + auto Resolver =
> + createLambdaResolver(
> + [&](const std::string &Name) {
> + if (auto Sym = ObjLayer.findSymbol(Name, true))
> + return RuntimeDyld::SymbolInfo(Sym.getAddress(),
> Sym.getFlags());
> + return RuntimeDyld::SymbolInfo(nullptr);
> + },
> + [](const std::string &Name) {
> + return RuntimeDyld::SymbolInfo(nullptr);
> + });
> +
> + SectionMemoryManagerWrapper SMMW;
> + ObjLayer.addObjectSet(std::move(Obj1Set), &SMMW, &*Resolver);
> + auto H = ObjLayer.addObjectSet(std::move(Obj2Set), &SMMW, &*Resolver);
> + ObjLayer.emitAndFinalize(H);
> +
> + // Finalization of module 2 should trigger finalization of module 1.
> + // Verify that finalize on SMMW is only called once.
> + EXPECT_EQ(SMMW.FinalizationCount, 1)
> + << "Extra call to finalize";
> +}
> +
> }
>
> Modified: llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp?rev=257263&r1=257262&r2=257263&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp (original)
> +++ llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp Sat Jan 9
> 13:50:40 2016
> @@ -19,7 +19,7 @@ bool OrcExecutionTest::NativeTargetIniti
>
> ModuleBuilder::ModuleBuilder(LLVMContext &Context, StringRef Triple,
> StringRef Name)
> - : M(new Module(Name, Context)),
> - Builder(Context) {
> - M->setTargetTriple(Triple);
> + : M(new Module(Name, Context)) {
> + if (Triple != "")
> + M->setTargetTriple(Triple);
> }
>
> Modified: llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h?rev=257263&r1=257262&r2=257263&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h (original)
> +++ llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h Sat Jan 9
> 13:50:40 2016
> @@ -20,6 +20,7 @@
> #include "llvm/IR/LLVMContext.h"
> #include "llvm/IR/Module.h"
> #include "llvm/IR/TypeBuilder.h"
> +#include "llvm/Object/ObjectFile.h"
> #include "llvm/ExecutionEngine/ExecutionEngine.h"
> #include "llvm/ExecutionEngine/Orc/JITSymbol.h"
> #include "llvm/Support/TargetSelect.h"
> @@ -74,7 +75,6 @@ public:
>
> private:
> std::unique_ptr<Module> M;
> - IRBuilder<> Builder;
> };
>
> // Dummy struct type.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160114/214cd4da/attachment.html>
More information about the llvm-commits
mailing list