[llvm] r257263 - [Orc][RuntimeDyld] Prevent duplicate calls to finalizeMemory on shared memory
Lang Hames via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 19 13:15:21 PST 2016
I've attempted to fix this in r258185 by not loading objects (via
RuntimeDyld::loadObject) until they're about to be finalized.
- Lang.
On Sat, Jan 16, 2016 at 1:12 PM, Lang Hames <lhames at gmail.com> wrote:
> Hi Dave,
>
> > Can, but don't have to?
>
> Yep. It's a matter of symbol dependence: If R1 contains a relocation to a
> symbol defined in R2, then relocating R1 requires relocating R2. If there
> is no symbol dependence, then there will be no recursive relocate calls.
>
> > 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 finalised?
>
> Ahh - I see your concern now. You've identified a new bug in this scheme.
> In practice this won't come up often because higher layers try to avoid
> pushing objects down (or compiling them at all) until they're needed, so
> you usually only have RuntimeDyld instances for the code you're asking for,
> plus any dependent code that's needed. The interface shouldn't require that
> though: You should be able to add unrelated objects with the same memory
> manager and not get premature finalisation.
>
> > 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.
>
> Where does the ref count live? The user may pass the memory manager in in
> more than one place. E.g.:
>
> ObjectLayer.addObjectSet(JITStdLib, M1, NullResolver);
> CompileLayer.addModuleSet(UserCode, M1, CustomResolver);
>
> And I don't want to count on them doing the wrapping. If you add the
> ref-count to the MemoryManager itself it doesn't look much different to
> what I've got.
>
> I think the right way forward is to take my existing scheme and adapt the
> ObjectLinkingLayer so that it doesn't even create a RuntimeDyld instance
> until an object is actually needed. That way the only memory allocated by
> the memory manager will be the memory that needs to be finalised before we
> return from a symbol lookup. I'll see if I can work that up.
>
> - Lang.
>
> On Sat, Jan 16, 2016 at 11:45 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
>
>>
>>
>> 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/20160119/74229f01/attachment.html>
More information about the llvm-commits
mailing list