[llvm] r257263 - [Orc][RuntimeDyld] Prevent duplicate calls to finalizeMemory on shared memory
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Sat Jan 16 11:45:30 PST 2016
On Sat, Jan 16, 2016 at 11:25 AM, Lang Hames <lhames at gmail.com> wrote:
> I can't see how a decorator would work here unfortunately.
>
> The issue is this: We want to finalise the memory before returning to the
> user, so that they can use whatever pointer is returned. However, we can't
> finalise memory until all RuntimeDyld instances that are sharing a given
> memory manager have applied their relocations, and relocation of one
> RuntimeDyld instance can trigger the relocation of another (and
> transitively another, and so on).
>
Can, but don't have to?
What happens if one RuntimeDyld hasn't finalized its memory, but another
that shares the same MM has? Could the first one end up trying to use its
MM in a non-finalized when it had already been finalized?
> Taking a concrete example: Imagine that you had five RuntimeDyld
> instances: R1..R5, and two memory managers: M1 and M2. R1, R2, and R4 use
> M1. R3 and R5 use M2. Each RuntimeDyld is chained such that relocating it
> will trigger relocation of the next one. When you relocate R1 you have the
> following situation:
>
> R1 -> R2 -> R3 -> R4 -> R5
> ^ ^
> | \ M2 should be finalised once we return here.
> \ M1 should be finalised when we return here.
>
> The scheme that this patch adopts is that RuntimeDyld instance checks its
> associated memory manager to see whether anyone else has claimed
> responsibility for finalising it yet. If nobody has, this instance claims
> responsibility and sets the flag. So before relocating, R1 will check M1
> and find that nobody else has claimed responsibility for finalising it yet,
> so R1 will claim that responsibility. R2 then checks M1 and finds that
> someone else has already claimed responsibility, so it does nothing. R3
> checks M2, finds that nobody has claimed responsibility for finalising it
> yet and so takes responsibility for that. R4 and R5 check M1 and M2
> respectively, both finding that responsibility has already been claimed, so
> they do nothing.
>
> Hope that makes sense?
>
I think it does, but I'm still thinking a ref counted finalization using
decorators may still apply here.
To take your same scenario but instead of claiming the right to finalize,
each RuntimeDyld instance gets what appears to be a separate MemoryManager,
yet they are all reference-counting-finalizing decorators around the two
real memory managers M1 and M2.
When R5 finalizes its MM (a wrapper around M2) the ref count is decreased
(or increased, whichever way you look at it/implement it) by one, but
that's still not at zero (because there were two wrappers created, one for
R5 and one for R3) so it doesn't actually do anything (as is the case in
your current implementation). Then when R3 finalizes its MM (the other
wrapper around M2) the ref count is decremented again, dropping to zero and
causing the actual finalization to occur.
Similarly for 4 (refcount 3->2), 2 (refcount 2->1), and 1 (refcount 1->0,
triggering finalization).
>
> - Lang.
>
>
> On Thu, Jan 14, 2016 at 3:25 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> 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/20160116/47752b91/attachment.html>
More information about the llvm-commits
mailing list