Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
Sorry, got busy.
This looks fine.
Jim
On Mar 12, 2014, at 2:29 AM, Andrew MacPherson wrote:
> Hi Jim, just pinging this again to see if you've had a chance to take a quick
> look at the ModulesDidLoad patch, let me know if you'd prefer I submit this
> as a general patch for someone else to review. Thanks!
>
>
> On Fri, Mar 7, 2014 at 10:41 AM, Andrew MacPherson
> wrote:
> Hi Jim,
>
> I've attached a patch here that does what you suggest, I created a
> ModulesDidLoad() on both the Process and the JITLoader and use this now to
> only search the new modules for necessary JIT symbols. This works great and
> startup time does seem faster on programs with large shared libraries.
>
> I didn't make ModulesDidLoad() on the Process virtual for now as there was no
> need for it in this particular case but if you'd prefer I do it anyway just
> let me know. I also left out SymbolsDidLoad() as I'd prefer to have a use
> case that I can test with it and I don't right now, I will certainly keep it
> in mind for later though.
>
> This new patch also does away with the need for the patch I sent yesterday as
> that code (and the crashing code you hit) has been replaced with this new
> method. I'm not sure what the status of building the JITLoaderGDB on OSX is
> right now, I haven't made any commits since the initial patch commit.
>
> Thanks for the help!
>
> Andrew
>
>
>
> On Thu, Mar 6, 2014 at 7:10 PM, wrote:
> 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 wrote:
>
> > H
Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
Hi Jim, just pinging this again to see if you've had a chance to take a
quick look at the ModulesDidLoad patch, let me know if you'd prefer I
submit this as a general patch for someone else to review. Thanks!
On Fri, Mar 7, 2014 at 10:41 AM, Andrew MacPherson wrote:
> Hi Jim,
>
> I've attached a patch here that does what you suggest, I created a
> ModulesDidLoad() on both the Process and the JITLoader and use this now to
> only search the new modules for necessary JIT symbols. This works great and
> startup time does seem faster on programs with large shared libraries.
>
> I didn't make ModulesDidLoad() on the Process virtual for now as there was
> no need for it in this particular case but if you'd prefer I do it anyway
> just let me know. I also left out SymbolsDidLoad() as I'd prefer to have a
> use case that I can test with it and I don't right now, I will certainly
> keep it in mind for later though.
>
> This new patch also does away with the need for the patch I sent yesterday
> as that code (and the crashing code you hit) has been replaced with this
> new method. I'm not sure what the status of building the JITLoaderGDB on
> OSX is right now, I haven't made any commits since the initial patch commit.
>
> Thanks for the help!
>
> Andrew
>
>
>
> On Thu, Mar 6, 2014 at 7:10 PM, wrote:
>
>> 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
>> wrote:
>>
>> > Hi Jim,
>> >
>> > I agree that this part of the patch isn't pretty but I wasn't able to
>> fi
Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
Hi Jim,
I've attached a patch here that does what you suggest, I created a
ModulesDidLoad() on both the Process and the JITLoader and use this now to
only search the new modules for necessary JIT symbols. This works great and
startup time does seem faster on programs with large shared libraries.
I didn't make ModulesDidLoad() on the Process virtual for now as there was
no need for it in this particular case but if you'd prefer I do it anyway
just let me know. I also left out SymbolsDidLoad() as I'd prefer to have a
use case that I can test with it and I don't right now, I will certainly
keep it in mind for later though.
This new patch also does away with the need for the patch I sent yesterday
as that code (and the crashing code you hit) has been replaced with this
new method. I'm not sure what the status of building the JITLoaderGDB on
OSX is right now, I haven't made any commits since the initial patch commit.
Thanks for the help!
Andrew
On Thu, Mar 6, 2014 at 7:10 PM, wrote:
> 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
> 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
> c
Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
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 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 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
Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
The make files should be changed to be included on all platforms. I don't know why people keep making plug-ins that will work fine remotely, but make them only build natively on the host they want to use them on. LLDB does remote debugging, so we need and all plug-ins that can be built for cross debugging to always be built. On Mar 6, 2014, at 3:26 AM, Ahmed Charles wrote: > This patch seems to break builds on windows, since the JITLoaderGDB class is > only included in cmake on FreeBSD and Linux, but it is used in lldb.cpp on > all platforms. > > > > >> From: andrew.m...@gmail.com >> Date: Thu, 6 Mar 2014 10:52:36 +0100 >> To: jing...@apple.com >> CC: lldb-commits@cs.uiuc.edu >> Subject: Re: [Lldb-commits] [lldb] r202956 - Add support for JIT >> debugging on Linux using the GDB JIT interface. Patch written with Keno >> Fischer. >> >> 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 >> mailto:jing...@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 = >> , function: >> lldb_private::Process::GetTarget() , stop reason = EXC_BAD_ACCESS >> (code=1, address=0x100) >> frame #0: 0x000106f8072c >> LLDB`lldb_private::Process::GetTarget() at Process.h:2516 >> frame #1: 0x000108e7aa5a >> LLDB`JITLoaderGDB::GetSymbolAddress(lldb_private::ConstString const&, >> lldb::SymbolType) const at JITLoaderGDB.cpp:368 >> frame #2: 0x000108e7a8bf LLDB`JITLoaderGDB::SetJITBreakpoint() >> at JITLoaderGDB.cpp:99 >> frame #3: 0x000108e7a6d8 >> LLDB`JITLoaderGDB::ProcessStateChangedCallback(void*, >> lldb_private::Process*, lldb::StateType) at JITLoaderGDB.cpp:354 >> frame #4: 0x000108b7b29b >> LLDB`lldb_private::Process::SynchronouslyNotifyStateChanged(lldb::StateType) >> >> at Process.cpp:1223 >> frame #5: 0x000108b89762 >> LLDB`lldb_private::Process::ShouldBroadcastEvent(lldb_private::Event*) >> at Process.cpp:3846 >> frame #6: 0x000108b8454d >> LLDB`lldb_private::Process::HandlePrivateEvent(std::__1::shared_ptr&) >> >> at Process.cpp:4141 >> frame #7: 0x000108b8a755 >> LLDB`lldb_private::Process::RunPrivateStateThread() at Process.cpp:4290 >> frame #8: 0x000108b89bfd >> LLDB`lldb_private::Process::PrivateStateThread(void*) at >> Process.cpp:4221 >> frame #9: 0x0001087d811a LLDB`ThreadCreateTrampoline(void*) at >> Host.cpp:629 >> frame
Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
This patch seems to break builds on windows, since the JITLoaderGDB class is
only included in cmake on FreeBSD and Linux, but it is used in lldb.cpp on all
platforms.
> From: andrew.m...@gmail.com
> Date: Thu, 6 Mar 2014 10:52:36 +0100
> To: jing...@apple.com
> CC: lldb-commits@cs.uiuc.edu
> Subject: Re: [Lldb-commits] [lldb] r202956 - Add support for JIT
> debugging on Linux using the GDB JIT interface. Patch written with Keno
> Fischer.
>
> 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
> mailto:jing...@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 =
> , function:
> lldb_private::Process::GetTarget() , stop reason = EXC_BAD_ACCESS
> (code=1, address=0x100)
> frame #0: 0x000106f8072c
> LLDB`lldb_private::Process::GetTarget() at Process.h:2516
> frame #1: 0x000108e7aa5a
> LLDB`JITLoaderGDB::GetSymbolAddress(lldb_private::ConstString const&,
> lldb::SymbolType) const at JITLoaderGDB.cpp:368
> frame #2: 0x000108e7a8bf LLDB`JITLoaderGDB::SetJITBreakpoint()
> at JITLoaderGDB.cpp:99
> frame #3: 0x000108e7a6d8
> LLDB`JITLoaderGDB::ProcessStateChangedCallback(void*,
> lldb_private::Process*, lldb::StateType) at JITLoaderGDB.cpp:354
> frame #4: 0x000108b7b29b
> LLDB`lldb_private::Process::SynchronouslyNotifyStateChanged(lldb::StateType)
> at Process.cpp:1223
> frame #5: 0x000108b89762
> LLDB`lldb_private::Process::ShouldBroadcastEvent(lldb_private::Event*)
> at Process.cpp:3846
> frame #6: 0x000108b8454d
> LLDB`lldb_private::Process::HandlePrivateEvent(std::__1::shared_ptr&)
>
> at Process.cpp:4141
> frame #7: 0x000108b8a755
> LLDB`lldb_private::Process::RunPrivateStateThread() at Process.cpp:4290
> frame #8: 0x000108b89bfd
> LLDB`lldb_private::Process::PrivateStateThread(void*) at
> Process.cpp:4221
> frame #9: 0x0001087d811a LLDB`ThreadCreateTrampoline(void*) at
> Host.cpp:629
> frame #10: 0x7fff815df899 libsystem_pthread.dylib`_pthread_body
> frame #11: 0x7fff815df72a libsystem_pthread.dylib`_pthread_start
> frame #12: 0x7fff815e3fc9 libsystem_pthread.dylib`thread_start
> (lldb) f 2
> frame #2: 0x000108e7a8bf 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"),
> 100eSymbolTypeAny);
> 101 if (jit_addr == LLDB_INVALID_A
Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
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 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 =
> , function:
> lldb_private::Process::GetTarget() , stop reason = EXC_BAD_ACCESS (code=1,
> address=0x100)
> frame #0: 0x000106f8072c LLDB`lldb_private::Process::GetTarget()
> at Process.h:2516
> frame #1: 0x000108e7aa5a
> LLDB`JITLoaderGDB::GetSymbolAddress(lldb_private::ConstString const&,
> lldb::SymbolType) const at JITLoaderGDB.cpp:368
> frame #2: 0x000108e7a8bf LLDB`JITLoaderGDB::SetJITBreakpoint() at
> JITLoaderGDB.cpp:99
> frame #3: 0x000108e7a6d8
> LLDB`JITLoaderGDB::ProcessStateChangedCallback(void*,
> lldb_private::Process*, lldb::StateType) at JITLoaderGDB.cpp:354
> frame #4: 0x000108b7b29b
> LLDB`lldb_private::Process::SynchronouslyNotifyStateChanged(lldb::StateType)
> at Process.cpp:1223
> frame #5: 0x000108b89762
> LLDB`lldb_private::Process::ShouldBroadcastEvent(lldb_private::Event*) at
> Process.cpp:3846
> frame #6: 0x000108b8454d
> LLDB`lldb_private::Process::HandlePrivateEvent(std::__1::shared_ptr&)
> at Process.cpp:4141
> frame #7: 0x000108b8a755
> LLDB`lldb_private::Process::RunPrivateStateThread() at Process.cpp:4290
> frame #8: 0x000108b89bfd
> LLDB`lldb_private::Process::PrivateStateThread(void*) at Process.cpp:4221
> frame #9: 0x0001087d811a LLDB`ThreadCreateTrampoline(void*) at
> Host.cpp:629
> frame #10: 0x7fff815df899 libsystem_pthread.dylib`_pthread_body
> frame #11: 0x7fff815df72a libsystem_pthread.dylib`_pthread_start
> frame #12: 0x7fff815e3fc9 libsystem_pthread.dylib`thread_start
> *(lldb) **f 2*
> frame #2: 0x000108e7a8bf 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"),
>100eSymbolTypeAny);
>101 if (jit_addr == LLDB_INVALID_ADDRESS)
>102 return;
> *(lldb) **expr *this*
> (JITLoaderGDB) $13 = {
> lldb_private::JITLoader = {
> m_process = 0x Public:
> lldb_private::ThreadSafeValue @ Private:
> lldb_private::ThreadSafeValue @
> }
> m_jit_objects = size=160215376 {
> [0] = {
> first =
> second =
> }
> ...
> }
> m_jit_break_id = 0
> m_notification_callbacks = {
> baton = 0x0001
> initialize = 0x7fc54b00f3b0
> process_state_changed = 0x0001098cb150 (vtable for
> std::__1::__shared_ptr_pointer std::__1::default_delete,
> std::__1::allocator > + 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
> wrote:
>
> +void
> +JITLoaderGDB::ProcessStateChangedCallback(void *baton,
> + lldb_private::Process *process,
> + lldb::StateType state)
> +{
> +JITLoaderGDB* instance = static_cast(baton);
> +
> +switch (state)
> +{
> +case eStateConnected:
> +case eState
Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
BTW, I realize this was a kind of incoherent message. I was looking at the
patch and wrote the first and last sections, then played with it and saw the
crash, and interleaved the two messages… Sorry ‘bout that, my only defense is
that I was waiting for the Pizza to arrive…
Jim
On Mar 5, 2014, at 7:28 PM, Jim Ingham 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 = ,
> function: lldb_private::Process::GetTarget() , stop reason = EXC_BAD_ACCESS
> (code=1, address=0x100)
> frame #0: 0x000106f8072c LLDB`lldb_private::Process::GetTarget() at
> Process.h:2516
> frame #1: 0x000108e7aa5a
> LLDB`JITLoaderGDB::GetSymbolAddress(lldb_private::ConstString const&,
> lldb::SymbolType) const at JITLoaderGDB.cpp:368
> frame #2: 0x000108e7a8bf LLDB`JITLoaderGDB::SetJITBreakpoint() at
> JITLoaderGDB.cpp:99
> frame #3: 0x000108e7a6d8
> LLDB`JITLoaderGDB::ProcessStateChangedCallback(void*, lldb_private::Process*,
> lldb::StateType) at JITLoaderGDB.cpp:354
> frame #4: 0x000108b7b29b
> LLDB`lldb_private::Process::SynchronouslyNotifyStateChanged(lldb::StateType)
> at Process.cpp:1223
> frame #5: 0x000108b89762
> LLDB`lldb_private::Process::ShouldBroadcastEvent(lldb_private::Event*) at
> Process.cpp:3846
> frame #6: 0x000108b8454d
> LLDB`lldb_private::Process::HandlePrivateEvent(std::__1::shared_ptr&)
> at Process.cpp:4141
> frame #7: 0x000108b8a755
> LLDB`lldb_private::Process::RunPrivateStateThread() at Process.cpp:4290
> frame #8: 0x000108b89bfd
> LLDB`lldb_private::Process::PrivateStateThread(void*) at Process.cpp:4221
> frame #9: 0x0001087d811a LLDB`ThreadCreateTrampoline(void*) at
> Host.cpp:629
> frame #10: 0x7fff815df899 libsystem_pthread.dylib`_pthread_body
> frame #11: 0x7fff815df72a libsystem_pthread.dylib`_pthread_start
> frame #12: 0x7fff815e3fc9 libsystem_pthread.dylib`thread_start
> (lldb) f 2
> frame #2: 0x000108e7a8bf 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);
>101if (jit_addr == LLDB_INVALID_ADDRESS)
>102return;
> (lldb) expr *this
> (JITLoaderGDB) $13 = {
> lldb_private::JITLoader = {
> m_process = 0x Public:
> lldb_private::ThreadSafeValue @ Private:
> lldb_private::ThreadSafeValue @
> }
> m_jit_objects = size=160215376 {
> [0] = {
> first =
> second =
> }
> ...
> }
> m_jit_break_id = 0
> m_notification_callbacks = {
> baton = 0x0001
> initialize = 0x7fc54b00f3b0
> process_state_changed = 0x0001098cb150 (vtable for
> std::__1::__shared_ptr_pointer std::__1::default_delete,
> std::__1::allocator > + 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 wrote:
>
>> +void
>> +JITLoaderGDB::ProcessStateChangedCallback(void *baton,
>> + lldb_private::Process *process,
>> + lldb::StateType state)
>> +{
>> +JITLoaderGDB* instance = static_cast(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;
>> +}
>> +}
>> +
>
> ___
> lldb-commits mailing list
> lldb-commits@cs.uiuc.edu
> http://lists.cs
Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
So the crash is coming (on OS X) only in the case where you use:
(lldb) process launch —shell=/bin/bash —
which we implement (as does gdb) by consing up a command like:
/bin/bash exec PROGNAME arg1 arg2 …
and then running that. So the program exec’s a few times before it gets to the
executable we are actually trying to debug. If I don’t do that but just run
the program directly, then I don’t get this crash.
I temporarily disabled the calling of the GetJITLoader().Did{Attach,Launch}
till we fix the crash.
Jim
On Mar 5, 2014, at 7:28 PM, Jim Ingham 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 = ,
> function: lldb_private::Process::GetTarget() , stop reason = EXC_BAD_ACCESS
> (code=1, address=0x100)
> frame #0: 0x000106f8072c LLDB`lldb_private::Process::GetTarget() at
> Process.h:2516
> frame #1: 0x000108e7aa5a
> LLDB`JITLoaderGDB::GetSymbolAddress(lldb_private::ConstString const&,
> lldb::SymbolType) const at JITLoaderGDB.cpp:368
> frame #2: 0x000108e7a8bf LLDB`JITLoaderGDB::SetJITBreakpoint() at
> JITLoaderGDB.cpp:99
> frame #3: 0x000108e7a6d8
> LLDB`JITLoaderGDB::ProcessStateChangedCallback(void*, lldb_private::Process*,
> lldb::StateType) at JITLoaderGDB.cpp:354
> frame #4: 0x000108b7b29b
> LLDB`lldb_private::Process::SynchronouslyNotifyStateChanged(lldb::StateType)
> at Process.cpp:1223
> frame #5: 0x000108b89762
> LLDB`lldb_private::Process::ShouldBroadcastEvent(lldb_private::Event*) at
> Process.cpp:3846
> frame #6: 0x000108b8454d
> LLDB`lldb_private::Process::HandlePrivateEvent(std::__1::shared_ptr&)
> at Process.cpp:4141
> frame #7: 0x000108b8a755
> LLDB`lldb_private::Process::RunPrivateStateThread() at Process.cpp:4290
> frame #8: 0x000108b89bfd
> LLDB`lldb_private::Process::PrivateStateThread(void*) at Process.cpp:4221
> frame #9: 0x0001087d811a LLDB`ThreadCreateTrampoline(void*) at
> Host.cpp:629
> frame #10: 0x7fff815df899 libsystem_pthread.dylib`_pthread_body
> frame #11: 0x7fff815df72a libsystem_pthread.dylib`_pthread_start
> frame #12: 0x7fff815e3fc9 libsystem_pthread.dylib`thread_start
> (lldb) f 2
> frame #2: 0x000108e7a8bf 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);
>101if (jit_addr == LLDB_INVALID_ADDRESS)
>102return;
> (lldb) expr *this
> (JITLoaderGDB) $13 = {
> lldb_private::JITLoader = {
> m_process = 0x Public:
> lldb_private::ThreadSafeValue @ Private:
> lldb_private::ThreadSafeValue @
> }
> m_jit_objects = size=160215376 {
> [0] = {
> first =
> second =
> }
> ...
> }
> m_jit_break_id = 0
> m_notification_callbacks = {
> baton = 0x0001
> initialize = 0x7fc54b00f3b0
> process_state_changed = 0x0001098cb150 (vtable for
> std::__1::__shared_ptr_pointer std::__1::default_delete,
> std::__1::allocator > + 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 wrote:
>
>> +void
>> +JITLoaderGDB::ProcessStateChangedCallback(void *baton,
>> + lldb_private::Process *process,
>> + lldb::StateType state)
>> +{
>> +JITLoaderGDB* instance = static_cast(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;
>> +
>> +cas
Re: [Lldb-commits] [lldb] r202956 - Add support for JIT debugging on Linux using the GDB JIT interface. Patch written with Keno Fischer.
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 = ,
function: lldb_private::Process::GetTarget() , stop reason = EXC_BAD_ACCESS
(code=1, address=0x100)
frame #0: 0x000106f8072c LLDB`lldb_private::Process::GetTarget() at
Process.h:2516
frame #1: 0x000108e7aa5a
LLDB`JITLoaderGDB::GetSymbolAddress(lldb_private::ConstString const&,
lldb::SymbolType) const at JITLoaderGDB.cpp:368
frame #2: 0x000108e7a8bf LLDB`JITLoaderGDB::SetJITBreakpoint() at
JITLoaderGDB.cpp:99
frame #3: 0x000108e7a6d8
LLDB`JITLoaderGDB::ProcessStateChangedCallback(void*, lldb_private::Process*,
lldb::StateType) at JITLoaderGDB.cpp:354
frame #4: 0x000108b7b29b
LLDB`lldb_private::Process::SynchronouslyNotifyStateChanged(lldb::StateType) at
Process.cpp:1223
frame #5: 0x000108b89762
LLDB`lldb_private::Process::ShouldBroadcastEvent(lldb_private::Event*) at
Process.cpp:3846
frame #6: 0x000108b8454d
LLDB`lldb_private::Process::HandlePrivateEvent(std::__1::shared_ptr&)
at Process.cpp:4141
frame #7: 0x000108b8a755
LLDB`lldb_private::Process::RunPrivateStateThread() at Process.cpp:4290
frame #8: 0x000108b89bfd
LLDB`lldb_private::Process::PrivateStateThread(void*) at Process.cpp:4221
frame #9: 0x0001087d811a LLDB`ThreadCreateTrampoline(void*) at
Host.cpp:629
frame #10: 0x7fff815df899 libsystem_pthread.dylib`_pthread_body
frame #11: 0x7fff815df72a libsystem_pthread.dylib`_pthread_start
frame #12: 0x7fff815e3fc9 libsystem_pthread.dylib`thread_start
(lldb) f 2
frame #2: 0x000108e7a8bf 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 = 0x Public:
lldb_private::ThreadSafeValue @ Private:
lldb_private::ThreadSafeValue @
}
m_jit_objects = size=160215376 {
[0] = {
first =
second =
}
...
}
m_jit_break_id = 0
m_notification_callbacks = {
baton = 0x0001
initialize = 0x7fc54b00f3b0
process_state_changed = 0x0001098cb150 (vtable for
std::__1::__shared_ptr_pointer,
std::__1::allocator > + 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 wrote:
> +void
> +JITLoaderGDB::ProcessStateChangedCallback(void *baton,
> + lldb_private::Process *process,
> + lldb::StateType state)
> +{
> +JITLoaderGDB* instance = static_cast(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;
> +}
> +}
> +
___
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
