[llvm] r257263 - [Orc][RuntimeDyld] Prevent duplicate calls to finalizeMemory on shared memory

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 16 11:25:21 PST 2016


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). 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?

- 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/07ac7121/attachment.html>


More information about the llvm-commits mailing list