[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