[Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
jingham at apple.com
jingham at apple.com
Thu Mar 6 10:10:30 PST 2014
Andrew,
I'll have to check out this patch when I get back home. You very nicely turned off the JITLoaderGDB for MacOS X, but now that I've updated, that means I can't tell whether your patch fixes the crash :-( I'm in the middle of something else so I don't want to have to mess with this right now, but my checkout at home is still in the state it was last night, so I'll check it out there.
We don't currently have a pluggable mechanism for watching shared library loads, but there is a place where code belonging to Target & Process can observe these changes synchronously. That place is Target::ModulesDidLoad. Currently it handles the breakpoint resetting, and some work that Jason needed to do for the "SystemRuntime plugin". Since your JITLoader is also internal to the process, it would be fine for you to hook in there and look for your symbol. I don't thing we need to come up with some generic mechanism for this yet.
However, when we had only one process thing doing a job in Target::ModulesDidLoad it was kinda okay to check for m_process_sp and do what is arguably process related work there:
void
Target::ModulesDidLoad (ModuleList &module_list)
{
if (module_list.GetSize())
{
m_breakpoint_list.UpdateBreakpoints (module_list, true, false);
if (m_process_sp)
{
SystemRuntime *sys_runtime = m_process_sp->GetSystemRuntime();
if (sys_runtime)
{
sys_runtime->ModulesDidLoad (module_list);
}
}
// TODO: make event data that packages up the module_list
BroadcastEvent (eBroadcastBitModulesLoaded, NULL);
}
}
but now that there are two process related jobs, we should break this out and make a Process::ModulesDidLoad and do the process related stuff there. If you don't mind splitting this off, then feel free to do so and put your symbol search there rather than in the stop hook.
If the work you have to do is applicable to generic processes there's no reason to make Process::ModulesDidLoad virtual, but that might arguably be useful. Since there is Process generic work that is done here as well (the SystemRuntime call) you'll have to deal with how partition the generic & overridable behaviors.
We have two patterns for structuring a task that requires some generic work and some subclass, related work in lldb. In some places we just document in the base class method that you have to call the base class method in your derived method. Generally that's in code that aren't going to have a lot of folks touching it, so you can trust that whoever changes it will read the headers first. In other places we have a non-virtual Task method, and a virtual DoTask method, then the Task method does the generic stuff and calls DoTask for the virtual part of the task. The latter is cleaner though it adds another method to the internal API. Anyway, if it makes sense to have this Process::ModulesDidLoad be virtual, feel free to do this either way...
You might also want to hook into SymbolsDidLoad, which gets called when a symbol file gets added to a binary already loaded in the process. That would catch the case where your symbol was stripped from the binary, but somebody added a symbol file that contained the symbol mid-way through the debug run.
Jim
On Mar 6, 2014, at 1:52 AM, Andrew MacPherson <andrew.macp at gmail.com> wrote:
> Hi Jim,
>
> I agree that this part of the patch isn't pretty but I wasn't able to find a way to be notified when a shared library is loaded (exactly as you suggest). We need to check on launch or attach and then whenever a new library is loaded, if there's a better way to do that it would be great to clean this up.
>
> I can't reproduce the crash on Linux but it looks like we need to unregister the notification callback that's set, that's the only reason I can think of that you would end up in the ProcessStateChangedCallback with an invalid baton pointer. I've attached a patch that I'm hoping will resolve the issue here, it's definitely a bug that it wasn't being unregistered.
>
> I hadn't intended for the JITLoaderGDB to be enabled on OSX as I wasn't able to test it there, sorry about that. It's likely not useful there right now as it would need modifications to the ObjectFileMachO to do some relocations, and additionally MCJIT in LLVM would need to be modified to call the GDB hook on OSX (though other JIT hosts may work). That said it shouldn't be causing crashes if enabled.
>
> Thanks for looking into this.
>
> Cheers,
> Andrew
>
>
>
> On Thu, Mar 6, 2014 at 4:28 AM, Jim Ingham <jingham at apple.com> wrote:
> This part of the patch worries me. If I am debugging a process that doesn’t have this JIT loader symbol, this bit means every time that the process stops for any reason you will search the whole world for some symbol that won’t be found. That’s something we really avoid doing if we can, programs get pretty big and this is not the sort of thing you want to do.
>
> I don’t know how this symbol comes about, is there no event (shared library load or something) that you can hook into to find this symbol?
>
> This patch is also causing a crash on Mac OS X just running a program. The crash looks like:
>
> (lldb) bt
> * thread #8: tid = 0xf68e5, name = <lldb.process.internal-state(pid=40372)>, function: lldb_private::Process::GetTarget() , stop reason = EXC_BAD_ACCESS (code=1, address=0x100)
> frame #0: 0x0000000106f8072c LLDB`lldb_private::Process::GetTarget() at Process.h:2516
> frame #1: 0x0000000108e7aa5a LLDB`JITLoaderGDB::GetSymbolAddress(lldb_private::ConstString const&, lldb::SymbolType) const at JITLoaderGDB.cpp:368
> frame #2: 0x0000000108e7a8bf LLDB`JITLoaderGDB::SetJITBreakpoint() at JITLoaderGDB.cpp:99
> frame #3: 0x0000000108e7a6d8 LLDB`JITLoaderGDB::ProcessStateChangedCallback(void*, lldb_private::Process*, lldb::StateType) at JITLoaderGDB.cpp:354
> frame #4: 0x0000000108b7b29b LLDB`lldb_private::Process::SynchronouslyNotifyStateChanged(lldb::StateType) at Process.cpp:1223
> frame #5: 0x0000000108b89762 LLDB`lldb_private::Process::ShouldBroadcastEvent(lldb_private::Event*) at Process.cpp:3846
> frame #6: 0x0000000108b8454d LLDB`lldb_private::Process::HandlePrivateEvent(std::__1::shared_ptr<lldb_private::Event>&) at Process.cpp:4141
> frame #7: 0x0000000108b8a755 LLDB`lldb_private::Process::RunPrivateStateThread() at Process.cpp:4290
> frame #8: 0x0000000108b89bfd LLDB`lldb_private::Process::PrivateStateThread(void*) at Process.cpp:4221
> frame #9: 0x00000001087d811a LLDB`ThreadCreateTrampoline(void*) at Host.cpp:629
> frame #10: 0x00007fff815df899 libsystem_pthread.dylib`_pthread_body
> frame #11: 0x00007fff815df72a libsystem_pthread.dylib`_pthread_start
> frame #12: 0x00007fff815e3fc9 libsystem_pthread.dylib`thread_start
> (lldb) f 2
> frame #2: 0x0000000108e7a8bf LLDB`JITLoaderGDB::SetJITBreakpoint() at JITLoaderGDB.cpp:99
> 96 log->Printf("JITLoaderGDB::%s looking for JIT register hook",
> 97 __FUNCTION__);
> 98
> -> 99 addr_t jit_addr = GetSymbolAddress(ConstString("__jit_debug_register_code"),
> 100 eSymbolTypeAny);
> 101 if (jit_addr == LLDB_INVALID_ADDRESS)
> 102 return;
> (lldb) expr *this
> (JITLoaderGDB) $13 = {
> lldb_private::JITLoader = {
> m_process = 0x0000000000000000 Public: lldb_private::ThreadSafeValue<lldb::StateType> @ Private: lldb_private::ThreadSafeValue<lldb::StateType> @
> }
> m_jit_objects = size=160215376 {
> [0] = {
> first = <parent is NULL>
> second = <parent is NULL>
> }
> ...
> }
> m_jit_break_id = 0
> m_notification_callbacks = {
> baton = 0x0000000000000001
> initialize = 0x00007fc54b00f3b0
> process_state_changed = 0x00000001098cb150 (vtable for std::__1::__shared_ptr_pointer<lldb_private::Section*, std::__1::default_delete<lldb_private::Section>, std::__1::allocator<lldb_private::Section> > + 16)
> }
> }
>
> Looks like the JIT instance that is getting passed in is not good for some reason.
>
> Jim
>
>
> On Mar 5, 2014, at 2:12 AM, Andrew MacPherson <andrew.macp at gmail.com> wrote:
>
>> +void
>> +JITLoaderGDB::ProcessStateChangedCallback(void *baton,
>> + lldb_private::Process *process,
>> + lldb::StateType state)
>> +{
>> + JITLoaderGDB* instance = static_cast<JITLoaderGDB*>(baton);
>> +
>> + switch (state)
>> + {
>> + case eStateConnected:
>> + case eStateAttaching:
>> + case eStateLaunching:
>> + case eStateInvalid:
>> + case eStateUnloaded:
>> + case eStateExited:
>> + case eStateDetached:
>> + // instance->Clear(false);
>> + break;
>> +
>> + case eStateRunning:
>> + case eStateStopped:
>> + // Keep trying to set our JIT breakpoint each time we stop until we
>> + // succeed
>> + if (!instance->DidSetJITBreakpoint() && process->IsAlive())
>> + instance->SetJITBreakpoint();
>> + break;
>> +
>> + case eStateStepping:
>> + case eStateCrashed:
>> + case eStateSuspended:
>> + break;
>> + }
>> +}
>> +
>
>
> <jit-unregister-notification.patch>
More information about the lldb-commits
mailing list