[llvm-c] Expose debug support for LLJIT in Orc C-API bindings #73257
Conversation
weliveindetail
force-pushed
the
capi-orc-debug-plugin
branch
from
83cebca to
786088e
Compare
weliveindetail
force-pushed
the
capi-orc-debug-plugin
branch
2 times, most recently
from
db37381 to
74340b3
Compare
| @@ -39,7 +39,7 @@ Error enableDebuggerSupport(LLJIT &J) { | |||
| if (!Registrar) | |||
| return Registrar.takeError(); | |||
| ObjLinkingLayer->addPlugin(std::make_unique<DebugObjectManagerPlugin>( | |||
| ES, std::move(*Registrar), true, true)); | |||
| ES, std::move(*Registrar), false, true)); | |||
There was a problem hiding this comment.
This is the RequireDebugSections flag. Without this change the test won't pass, because the bitcode for the sum() example has no debug info.
This change may cause some overhead, but this shouldn't be a surprise in the context of debugging. And it may avoid headaches for users: Not remembering to generate debug info is a common problem for JITed code. With this change users get at least function names, like in release builds. Without it, the debugger has zero info.
Alternatively, we could expose it in the C-API and wire it up in the internal function here. See my second comment for details.
| } | ||
| } | ||
|
|
||
| #if defined(_AIX) or not defined(__ELF__) |
There was a problem hiding this comment.
Right now the test doesn't pass for MachO, because the sum() example has no debug info and it's a hard-coded requirement in the MachO plugin:
https://github.com/llvm/llvm-project/blob/0424546ed4a75708/llvm/lib/ExecutionEngine/Orc/Debugging/DebuggerSupportPlugin.cpp#L401
@lhames For the reasons described above, it might be interesting to expose the RequireDebugSections flag in the C-API. What do you think? Would you wire it up in the MachO plugin?
There was a problem hiding this comment.
The MachO plugin just bails out early if there's no debug info, right? I think that's reasonable behavior. Could we add some debug info to the test case? Hopefully the IR-level debug info is the same for both platforms?
There was a problem hiding this comment.
The MachO plugin just bails out early if there's no debug info, right? I think that's reasonable behavior.
Not sure. I think it's confusing for users. People are used to have symbol names in non-debug builds, so they can at least set breakpoints on their functions. With the bail out behavior here they have nothing at all and it's not immediately obvious why. It's one of too many things that can go wrong in JITed code debugging.
Could we add some debug info to the test case? Hopefully the IR-level debug info is the same for both platforms?
Anyway, yes that's probably the best solution. I guess minimal debug info will be pretty portable.
There was a problem hiding this comment.
The MachO plugin just bails out early if there's no debug info, right? I think that's reasonable behavior.
Not sure. I think it's confusing for users. People are used to have symbol names in non-debug builds, so they can at least set breakpoints on their functions. With the bail out behavior here they have nothing at all and it's not immediately obvious why. It's one of too many things that can go wrong in JITed code debugging.
Regarding that: I'm working on some lighter-weight symbolication solutions that we could use to enable breakpoints (and symbolicated backtraces) for JIT'd code, even in the case of a crashed executor. So hopefully this situation will be temporary. :)
weliveindetail
changed the title
weliveindetail
force-pushed
the
capi-orc-debug-plugin
branch
from
bf47a4b to
589fc25
Compare
weliveindetail
force-pushed
the
capi-orc-debug-plugin
branch
from
589fc25 to
f7ae608
Compare
|
weliveindetail
force-pushed
the
capi-orc-debug-plugin
branch
from
f7ae608 to
09de016
Compare
…o RuntimeDyld Fix test failure reported from buildbot clang-s390x-linux-lnt after #73257 OrcCAPITest.cpp:562: Debugger support requires JITLink
Allow C-API users to debug their JITed code. The new API function
LLVMOrcObjectLayerRegisterPluginJITLoaderGDB()adds the required plugin to provide GDB JIT Interface support. This initial patch covers the ELF use-case by installing the DebugObjectManagerPlugin. It should be easy to add support for other object formats later on.