<div dir="ltr">I can't see how a decorator would work here unfortunately.<div><br></div><div>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:</div><div><br></div><div><font face="monospace, monospace">R1 -> R2 -> R3 -> R4 -> R5</font></div><div><font face="monospace, monospace"> ^ ^</font></div><div><font face="monospace, monospace"> | \ M2 should be finalised once we return here.</font></div><div><font face="monospace, monospace"> \ M1 should be finalised when we return here. </font></div><div><br></div><div>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.</div><div><br></div><div>Hope that makes sense?</div><div><br></div><div>- Lang.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 14, 2016 at 3:25 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Sat, Jan 9, 2016 at 11:50 AM, Lang Hames via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: lhames<br>
Date: Sat Jan 9 13:50:40 2016<br>
New Revision: 257263<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=257263&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=257263&view=rev</a><br>
Log:<br>
[Orc][RuntimeDyld] Prevent duplicate calls to finalizeMemory on shared memory<br>
managers.<br>
<br>
Prior to this patch, recursive finalization (where finalization of one<br>
RuntimeDyld instance triggers finalization of another instance on which the<br>
first depends) could trigger memory access failures: When the inner (dependent)<br>
RuntimeDyld instance and its memory manager are finalized, memory allocated<br>
(but not yet relocated) by the outer instance is locked, and relocation in the<br>
outer instance fails with a memory access error.<br>
<br>
This patch adds a latch to the RuntimeDyld::MemoryManager base class that is<br>
checked by a new method: RuntimeDyld::finalizeWithMemoryManagerLocking, ensuring<br>
that shared memory managers are only finalized by the outermost RuntimeDyld<br>
instance.<br>
<br>
This allows ORC clients to supply the same memory manager to multiple calls to<br>
addModuleSet. In particular it enables the use of user-supplied memory managers<br>
with the CompileOnDemandLayer which must reuse the supplied memory manager for<br>
each function that is lazily compiled.<br></blockquote><div><br></div></span><div>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?)<br><br>- Dave</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h<br>
llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h<br>
llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp<br>
llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp<br>
llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h<br>
<br>
Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h?rev=257263&r1=257262&r2=257263&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h?rev=257263&r1=257262&r2=257263&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h (original)<br>
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/ObjectLinkingLayer.h Sat Jan 9 13:50:40 2016<br>
@@ -108,9 +108,7 @@ private:<br>
<br>
void Finalize() override {<br>
State = Finalizing;<br>
- RTDyld->resolveRelocations();<br>
- RTDyld->registerEHFrames();<br>
- MemMgr->finalizeMemory();<br>
+ RTDyld->finalizeWithMemoryManagerLocking();<br>
State = Finalized;<br>
}<br>
<br>
<br>
Modified: llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h?rev=257263&r1=257262&r2=257263&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h?rev=257263&r1=257262&r2=257263&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h (original)<br>
+++ llvm/trunk/include/llvm/ExecutionEngine/RuntimeDyld.h Sat Jan 9 13:50:40 2016<br>
@@ -95,7 +95,9 @@ public:<br>
<br>
/// \brief Memory Management.<br>
class MemoryManager {<br>
+ friend class RuntimeDyld;<br>
public:<br>
+ MemoryManager() : FinalizationLocked(false) {}<br>
virtual ~MemoryManager() {}<br>
<br>
/// Allocate a memory block of (at least) the given size suitable for<br>
@@ -153,6 +155,7 @@ public:<br>
<br>
private:<br>
virtual void anchor();<br>
+ bool FinalizationLocked;<br>
};<br>
<br>
/// \brief Symbol resolution.<br>
@@ -241,6 +244,25 @@ public:<br>
this->ProcessAllSections = ProcessAllSections;<br>
}<br>
<br>
+ /// Perform all actions needed to make the code owned by this RuntimeDyld<br>
+ /// instance executable:<br>
+ ///<br>
+ /// 1) Apply relocations.<br>
+ /// 2) Register EH frames.<br>
+ /// 3) Update memory permissions*.<br>
+ ///<br>
+ /// * Finalization is potentially recursive**, and the 3rd step will only be<br>
+ /// applied by the outermost call to finalize. This allows different<br>
+ /// RuntimeDyld instances to share a memory manager without the innermost<br>
+ /// finalization locking the memory and causing relocation fixup errors in<br>
+ /// outer instances.<br>
+ ///<br>
+ /// ** Recursive finalization occurs when one RuntimeDyld instances needs the<br>
+ /// address of a symbol owned by some other instance in order to apply<br>
+ /// relocations.<br>
+ ///<br>
+ void finalizeWithMemoryManagerLocking();<br>
+<br>
private:<br>
// RuntimeDyldImpl is the actual class. RuntimeDyld is just the public<br>
// interface.<br>
<br>
Modified: llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=257263&r1=257262&r2=257263&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp?rev=257263&r1=257262&r2=257263&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp (original)<br>
+++ llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp Sat Jan 9 13:50:40 2016<br>
@@ -967,6 +967,17 @@ bool RuntimeDyld::hasError() { return Dy<br>
<br>
StringRef RuntimeDyld::getErrorString() { return Dyld->getErrorString(); }<br>
<br>
+void RuntimeDyld::finalizeWithMemoryManagerLocking() {<br>
+ bool MemoryFinalizationLocked = MemMgr.FinalizationLocked;<br>
+ MemMgr.FinalizationLocked = true;<br>
+ resolveRelocations();<br>
+ registerEHFrames();<br>
+ if (!MemoryFinalizationLocked) {<br>
+ MemMgr.finalizeMemory();<br>
+ MemMgr.FinalizationLocked = false;<br>
+ }<br>
+}<br>
+<br>
void RuntimeDyld::registerEHFrames() {<br>
if (Dyld)<br>
Dyld->registerEHFrames();<br>
<br>
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp?rev=257263&r1=257262&r2=257263&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp?rev=257263&r1=257262&r2=257263&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp (original)<br>
+++ llvm/trunk/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp Sat Jan 9 13:50:40 2016<br>
@@ -7,6 +7,7 @@<br>
//<br>
//===----------------------------------------------------------------------===//<br>
<br>
+#include "OrcTestCommon.h"<br>
#include "llvm/ExecutionEngine/ExecutionEngine.h"<br>
#include "llvm/ExecutionEngine/SectionMemoryManager.h"<br>
#include "llvm/ExecutionEngine/Orc/CompileUtils.h"<br>
@@ -21,6 +22,10 @@ using namespace llvm::orc;<br>
<br>
namespace {<br>
<br>
+class ObjectLinkingLayerExecutionTest : public testing::Test,<br>
+ public OrcExecutionTest {<br>
+};<br>
+<br>
TEST(ObjectLinkingLayerTest, TestSetProcessAllSections) {<br>
<br>
class SectionMemoryManagerWrapper : public SectionMemoryManager {<br>
@@ -91,4 +96,81 @@ TEST(ObjectLinkingLayerTest, TestSetProc<br>
}<br>
}<br>
<br>
+<br>
+TEST_F(ObjectLinkingLayerExecutionTest, NoDuplicateFinalization) {<br>
+<br>
+ if (!TM)<br>
+ return;<br>
+<br>
+ class SectionMemoryManagerWrapper : public SectionMemoryManager {<br>
+ public:<br>
+ int FinalizationCount = 0;<br>
+ bool finalizeMemory(std::string *ErrMsg = 0) override {<br>
+ ++FinalizationCount;<br>
+ return SectionMemoryManager::finalizeMemory(ErrMsg);<br>
+ }<br>
+ };<br>
+<br>
+ ObjectLinkingLayer<> ObjLayer;<br>
+ SimpleCompiler Compile(*TM);<br>
+<br>
+ // Create a pair of modules that will trigger recursive finalization:<br>
+ // Module 1:<br>
+ // int bar() { return 42; }<br>
+ // Module 2:<br>
+ // int bar();<br>
+ // int foo() { return bar(); }<br>
+<br>
+ ModuleBuilder MB1(getGlobalContext(), "", "dummy");<br>
+ {<br>
+ MB1.getModule()->setDataLayout(TM->createDataLayout());<br>
+ Function *BarImpl = MB1.createFunctionDecl<int32_t(void)>("bar");<br>
+ BasicBlock *BarEntry = BasicBlock::Create(getGlobalContext(), "entry",<br>
+ BarImpl);<br>
+ IRBuilder<> Builder(BarEntry);<br>
+ IntegerType *Int32Ty = IntegerType::get(getGlobalContext(), 32);<br>
+ Value *FourtyTwo = ConstantInt::getSigned(Int32Ty, 42);<br>
+ Builder.CreateRet(FourtyTwo);<br>
+ }<br>
+<br>
+ auto Obj1 = Compile(*MB1.getModule());<br>
+ std::vector<object::ObjectFile*> Obj1Set;<br>
+ Obj1Set.push_back(Obj1.getBinary());<br>
+<br>
+ ModuleBuilder MB2(getGlobalContext(), "", "dummy");<br>
+ {<br>
+ MB2.getModule()->setDataLayout(TM->createDataLayout());<br>
+ Function *BarDecl = MB2.createFunctionDecl<int32_t(void)>("bar");<br>
+ Function *FooImpl = MB2.createFunctionDecl<int32_t(void)>("foo");<br>
+ BasicBlock *FooEntry = BasicBlock::Create(getGlobalContext(), "entry",<br>
+ FooImpl);<br>
+ IRBuilder<> Builder(FooEntry);<br>
+ Builder.CreateRet(Builder.CreateCall(BarDecl));<br>
+ }<br>
+ auto Obj2 = Compile(*MB2.getModule());<br>
+ std::vector<object::ObjectFile*> Obj2Set;<br>
+ Obj2Set.push_back(Obj2.getBinary());<br>
+<br>
+ auto Resolver =<br>
+ createLambdaResolver(<br>
+ [&](const std::string &Name) {<br>
+ if (auto Sym = ObjLayer.findSymbol(Name, true))<br>
+ return RuntimeDyld::SymbolInfo(Sym.getAddress(), Sym.getFlags());<br>
+ return RuntimeDyld::SymbolInfo(nullptr);<br>
+ },<br>
+ [](const std::string &Name) {<br>
+ return RuntimeDyld::SymbolInfo(nullptr);<br>
+ });<br>
+<br>
+ SectionMemoryManagerWrapper SMMW;<br>
+ ObjLayer.addObjectSet(std::move(Obj1Set), &SMMW, &*Resolver);<br>
+ auto H = ObjLayer.addObjectSet(std::move(Obj2Set), &SMMW, &*Resolver);<br>
+ ObjLayer.emitAndFinalize(H);<br>
+<br>
+ // Finalization of module 2 should trigger finalization of module 1.<br>
+ // Verify that finalize on SMMW is only called once.<br>
+ EXPECT_EQ(SMMW.FinalizationCount, 1)<br>
+ << "Extra call to finalize";<br>
+}<br>
+<br>
}<br>
<br>
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp?rev=257263&r1=257262&r2=257263&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp?rev=257263&r1=257262&r2=257263&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp (original)<br>
+++ llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.cpp Sat Jan 9 13:50:40 2016<br>
@@ -19,7 +19,7 @@ bool OrcExecutionTest::NativeTargetIniti<br>
<br>
ModuleBuilder::ModuleBuilder(LLVMContext &Context, StringRef Triple,<br>
StringRef Name)<br>
- : M(new Module(Name, Context)),<br>
- Builder(Context) {<br>
- M->setTargetTriple(Triple);<br>
+ : M(new Module(Name, Context)) {<br>
+ if (Triple != "")<br>
+ M->setTargetTriple(Triple);<br>
}<br>
<br>
Modified: llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h?rev=257263&r1=257262&r2=257263&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h?rev=257263&r1=257262&r2=257263&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h (original)<br>
+++ llvm/trunk/unittests/ExecutionEngine/Orc/OrcTestCommon.h Sat Jan 9 13:50:40 2016<br>
@@ -20,6 +20,7 @@<br>
#include "llvm/IR/LLVMContext.h"<br>
#include "llvm/IR/Module.h"<br>
#include "llvm/IR/TypeBuilder.h"<br>
+#include "llvm/Object/ObjectFile.h"<br>
#include "llvm/ExecutionEngine/ExecutionEngine.h"<br>
#include "llvm/ExecutionEngine/Orc/JITSymbol.h"<br>
#include "llvm/Support/TargetSelect.h"<br>
@@ -74,7 +75,6 @@ public:<br>
<br>
private:<br>
std::unique_ptr<Module> M;<br>
- IRBuilder<> Builder;<br>
};<br>
<br>
// Dummy struct type.<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>