<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">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><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><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">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><br></div></div>