Stefan Gränitz via llvm-dev
2017-Aug-24 12:29 UTC
[llvm-dev] Invalid Signature of orc::RTDyldObjectLinkingLayer::NotifyLoadedFtor
Hi all, hi Lang
It's a little late to report issues for release_50, but I just found
that thing while porting my JitFromScratch examples to 5.0.
This is a really nifty detail, but (if I'm not mistaken) the function
signature of RTDyldObjectLinkingLayer::NotifyLoadedFtor is incorrect:
$ grep -h -r -A 1 "using NotifyLoadedFtor"
./include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h
using NotifyLoadedFtor = std::function<void(ObjHandleT, const
ObjectPtr &Obj,
const LoadedObjectInfo &)>;
^^^^^^^^^^^^^^^^
It refers to llvm::LoadedObjectInfo base class in llvm/DebugInfo/DIContext.h
instead of llvm::RuntimeDyld::LoadedObjectInfo in
llvm/ExecutionEngine/RuntimeDyld.h
I think it should actually use the derived
RuntimeDyld::LoadedObjectInfo, as this is what listeners expect:
$ grep -h -r -A 1 "NotifyObjectEmitted("
./include/llvm/ExecutionEngine/JITEventListener.h
virtual void NotifyObjectEmitted(const object::ObjectFile &Obj,
const RuntimeDyld::LoadedObjectInfo
&L) {}
It doesn't break OrcMCJITReplacement as it duplicates the issue and the
Info parameter remains unused:
$ grep -h -r -A 4 -B 2 "const LoadedObjectInfo &Info"
./lib/ExecutionEngine/Orc/OrcMCJITReplacement.h
void operator()(RTDyldObjectLinkingLayerBase::ObjHandleT H,
const RTDyldObjectLinkingLayer::ObjectPtr &Obj,
const LoadedObjectInfo &Info) const {
M.UnfinalizedSections[H] std::move(M.SectionsAllocatedSinceLastLoad);
M.SectionsAllocatedSinceLastLoad = SectionAddrSet();
M.MemMgr->notifyObjectLoaded(&M, *Obj->getBinary());
}
OrcLazyJIT in lli on the other hand doesn't make use of the
notification.It can however be reproducedthere too:
diff --git a/tools/lli/OrcLazyJIT.h b/tools/lli/OrcLazyJIT.h
index 47a2acc4d7e..41a7c99413b 100644
--- a/tools/lli/OrcLazyJIT.h
+++ b/tools/lli/OrcLazyJIT.h
@@ -62,7 +62,11 @@ public:
bool InlineStubs)
: TM(std::move(TM)), DL(this->TM->createDataLayout()),
CCMgr(std::move(CCMgr)),
- ObjectLayer([]() { return
std::make_shared<SectionMemoryManager>(); }),
+ ObjectLayer([]() { return
std::make_shared<SectionMemoryManager>(); },
+ [this](llvm::orc::RTDyldObjectLinkingLayerBase::ObjHandleT,
+ const
llvm::orc::RTDyldObjectLinkingLayerBase::ObjectPtr &obj,
+ const llvm::RuntimeDyld::LoadedObjectInfo &info) {
+ }),
CompileLayer(ObjectLayer, orc::SimpleCompiler(*this->TM)),
IRDumpLayer(CompileLayer, createDebugDumper()),
CODLayer(IRDumpLayer, extractSingleFunction, *this->CCMgr,
Diagnostic message:
error: no matching constructor for initialization of 'ObjLayerT' (aka
'llvm::orc::RTDyldObjectLinkingLayer')
ObjectLayer([]() { return
std::make_shared<SectionMemoryManager>(); },
^
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /llvm50/llvm/tools/lli/OrcLazyJIT.cpp:10:
In file included from /llvm50/llvm/tools/lli/OrcLazyJIT.h:28:
/llvm50/llvm/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h:237:3:
note: candidate constructor not viable: no known conversion from
'(lambda at /llvm50/llvm/tools/lli/OrcLazyJIT.h:66:15)' to
'NotifyLoadedFtor' (aka 'function<void ([...], [...], const
llvm::LoadedObjectInfo &)>') for 2nd argument
Can it still be fixed? The patch is a 13 character one-liner:
diff --git a/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h
b/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h
index e1016ef95f0..fcf00fbc462 100644
--- a/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h
+++ b/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h
@@ -100,7 +100,7 @@ public:
/// @brief Functor for receiving object-loaded notifications.
using NotifyLoadedFtor = std::function<void(ObjHandleT, const
ObjectPtr &Obj,
- const LoadedObjectInfo
&)>;
+ const
RuntimeDyld::LoadedObjectInfo &)>;
/// @brief Functor for receiving finalization notifications.
using NotifyFinalizedFtor = std::function<void(ObjHandleT)>;
diff --git a/tools/lli/OrcLazyJIT.h b/tools/lli/OrcLazyJIT.h
index 47a2acc4d7e..41a7c99413b 100644
Opinions?
Cheers,
Stefan
--
https://weliveindetail.github.io/blog/
https://cryptup.org/pub/stefan.graenitz at gmail.com
Lang Hames via llvm-dev
2017-Sep-28 17:45 UTC
[llvm-dev] Invalid Signature of orc::RTDyldObjectLinkingLayer::NotifyLoadedFtor
Hi Stefan, You're right! Fixed in r314436 -- sorry for the delay! Cheers, Lang. On Thu, Aug 24, 2017 at 5:29 AM, Stefan Gränitz <stefan.graenitz at gmail.com> wrote:> Hi all, hi Lang > > It's a little late to report issues for release_50, but I just found > that thing while porting my JitFromScratch examples to 5.0. > This is a really nifty detail, but (if I'm not mistaken) the function > signature of RTDyldObjectLinkingLayer::NotifyLoadedFtor is incorrect: > > $ grep -h -r -A 1 "using NotifyLoadedFtor" > ./include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h > using NotifyLoadedFtor = std::function<void(ObjHandleT, const > ObjectPtr &Obj, > const LoadedObjectInfo &)>; > ^^^^^^^^^^^^^^^^ > It refers to llvm::LoadedObjectInfo base class in > llvm/DebugInfo/DIContext.h > instead of llvm::RuntimeDyld::LoadedObjectInfo in > llvm/ExecutionEngine/RuntimeDyld.h > > I think it should actually use the derived > RuntimeDyld::LoadedObjectInfo, as this is what listeners expect: > > $ grep -h -r -A 1 "NotifyObjectEmitted(" > ./include/llvm/ExecutionEngine/JITEventListener.h > virtual void NotifyObjectEmitted(const object::ObjectFile &Obj, > const RuntimeDyld::LoadedObjectInfo > &L) {} > > It doesn't break OrcMCJITReplacement as it duplicates the issue and the > Info parameter remains unused: > > $ grep -h -r -A 4 -B 2 "const LoadedObjectInfo &Info" > ./lib/ExecutionEngine/Orc/OrcMCJITReplacement.h > void operator()(RTDyldObjectLinkingLayerBase::ObjHandleT H, > const RTDyldObjectLinkingLayer::ObjectPtr &Obj, > const LoadedObjectInfo &Info) const { > M.UnfinalizedSections[H] > std::move(M.SectionsAllocatedSinceLastLoad); > M.SectionsAllocatedSinceLastLoad = SectionAddrSet(); > M.MemMgr->notifyObjectLoaded(&M, *Obj->getBinary()); > } > > OrcLazyJIT in lli on the other hand doesn't make use of the > notification.It can however be reproducedthere too: > > diff --git a/tools/lli/OrcLazyJIT.h b/tools/lli/OrcLazyJIT.h > index 47a2acc4d7e..41a7c99413b 100644 > --- a/tools/lli/OrcLazyJIT.h > +++ b/tools/lli/OrcLazyJIT.h > @@ -62,7 +62,11 @@ public: > bool InlineStubs) > : TM(std::move(TM)), DL(this->TM->createDataLayout()), > CCMgr(std::move(CCMgr)), > - ObjectLayer([]() { return > std::make_shared<SectionMemoryManager>(); }), > + ObjectLayer([]() { return > std::make_shared<SectionMemoryManager>(); }, > + [this](llvm::orc::RTDyldObjectLinkingLayerBase::ObjHandleT, > + const > llvm::orc::RTDyldObjectLinkingLayerBase::ObjectPtr &obj, > + const llvm::RuntimeDyld::LoadedObjectInfo &info) { > + }), > CompileLayer(ObjectLayer, orc::SimpleCompiler(*this->TM)), > IRDumpLayer(CompileLayer, createDebugDumper()), > CODLayer(IRDumpLayer, extractSingleFunction, *this->CCMgr, > > Diagnostic message: > error: no matching constructor for initialization of 'ObjLayerT' (aka > 'llvm::orc::RTDyldObjectLinkingLayer') > ObjectLayer([]() { return > std::make_shared<SectionMemoryManager>(); }, > ^ > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > In file included from /llvm50/llvm/tools/lli/OrcLazyJIT.cpp:10: > In file included from /llvm50/llvm/tools/lli/OrcLazyJIT.h:28: > /llvm50/llvm/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h: > 237:3: > note: candidate constructor not viable: no known conversion from > '(lambda at /llvm50/llvm/tools/lli/OrcLazyJIT.h:66:15)' to > 'NotifyLoadedFtor' (aka 'function<void ([...], [...], const > llvm::LoadedObjectInfo &)>') for 2nd argument > > > Can it still be fixed? The patch is a 13 character one-liner: > > diff --git a/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h > b/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h > index e1016ef95f0..fcf00fbc462 100644 > --- a/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h > +++ b/include/llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h > @@ -100,7 +100,7 @@ public: > > /// @brief Functor for receiving object-loaded notifications. > using NotifyLoadedFtor = std::function<void(ObjHandleT, const > ObjectPtr &Obj, > - const LoadedObjectInfo &)>; > + const > RuntimeDyld::LoadedObjectInfo &)>; > > /// @brief Functor for receiving finalization notifications. > using NotifyFinalizedFtor = std::function<void(ObjHandleT)>; > diff --git a/tools/lli/OrcLazyJIT.h b/tools/lli/OrcLazyJIT.h > index 47a2acc4d7e..41a7c99413b 100644 > > Opinions? > > Cheers, > Stefan > > -- > https://weliveindetail.github.io/blog/ > https://cryptup.org/pub/stefan.graenitz at gmail.com > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170928/e5d0e0cd/attachment.html>