Bug 560235 - Add a way to jump to a location and execute code from there
Add a way to jump to a location and execute code from there
Status: RESOLVED FIXED
Product: nemiver
Classification: Other
Component: general
trunk
Other Linux
: Normal enhancement
: ---
Assigned To: Dodji Seketeli
Nemiver maintainers
:
Depends on: 560377
Blocks:
  Show dependency tree
 
Reported: 2008-11-10 21:47 UTC by Dodji Seketeli
Modified: 2011-05-07 09:09 UTC (History)
1 user (show)

See Also:
GNOME target: ---
GNOME version: ---


Attachments
Initial patch that implements the idea (10.81 KB, patch)
2008-11-10 22:09 UTC, Dodji Seketeli
none Details | Diff | Review
Updated patch (10.83 KB, patch)
2008-11-11 18:00 UTC, Dodji Seketeli
none Details | Diff | Review

Description Dodji Seketeli 2008-11-10 21:47:01 UTC
The title says it all.
Comment 1 Dodji Seketeli 2008-11-10 22:09:53 UTC
Created attachment 122356 [details] [review]
Initial patch that implements the idea

This patch implements the idea.

The problem is that GDB/MI does not have any command to perform the "jump". So I did ressort to the jump command of the GDB/command line interface. This interface does not return any GDB/MI out-of-band output record, so Nemiver as no way to update its interface once the jump happened. So the interface is like the jump did not happen, but it did.

In the mean time, I did open a bug to the GDB bug tracker at http://sourceware.org/cgi-bin/gnatsweb.pl. The bug # is 2551. Once it is fixed, I can change the patch slightly to make it use the GDB/MI command.
Comment 2 Dodji Seketeli 2008-11-11 18:00:31 UTC
Created attachment 122437 [details] [review]
Updated patch

So finally a patch to add a new -exec-jump command to GDB/MI was proposed in GDB bug tracker, attached on bug 2551 at http://sourceware.org/cgi-bin/gnatsweb.pl.

I have applied it to GDB CVS HEAD (you can git clone gdb by doing: git clone git://sourceware.org/git/gdb.git).

For Nemiver to work with GDB CVS HEAD, appeared that I needed to fix bug #560377. A patch is applied to that bug by the way.

So after doing all that, this feature seems to work for me. Now I am waiting for the GDB patch to land so that I can apply this one.
Comment 3 Jonathon Jongsma 2008-11-12 14:40:57 UTC
The big thing that jumps out at me is: Do we need to somehow provide guards to only provide this functionality if the user's version of GDB is new enough?  What happens if the user has an old version of GDB that doesn't provide this MI interface?  (I haven't actually tried to build the patch yet) 

in on_debugger_breakpoints_set_signal(), when you're actually jumping, you might want to check (assert?) that m_priv->location_to_jump_to is not -1 before calling debugger ()->jump_to_location()

Typo: "Jump and stop to cursor and stop there" (extra "and stop")

As I mentioned in a short discussion on IRC: I'm not sure that both of these commands are frequently-used enough to add 2 more items to the context menu (since it's already getting quite full), but we can reorganize the context menu in a different bug.

Also, Dodji mentioned in the IRC conversation that the 'jump and break' command will keep setting duplicate breakpoints if you jump to the same location multiple times, so it needs a bit more intelligence to check if there's a breakpoint set on that line already.

One other thought: right now the jump-and-break command needs to be handled explicitly by the DBGPerspective (e.g. it has to handle the 'set-breakpoint' signal and then send the 'jump' command).  Would it make sense to have that handled in the IDebugger instead?  (e.g. maybe provide a bool parameter to the IDebugger::jump_to_location() function that specifies whether to stop after jumping or not).  This logic could then maybe be encapsulated into the debugger engine instead of the dbgperspective.  What do you think?  I'm actually not quite sure which way I prefer...
Comment 4 Dodji Seketeli 2011-04-10 13:45:09 UTC
I have created a branch named "jump-to" in which I have applied,
refreshed and modified with this patch.  You can browse it at
http://git.gnome.org/browse/nemiver/log/?h=jump-to.  For now the
branch is based on the gtk2-branch to let it have a change to compile
on the broadest possible set of systems.

Please find below my reply to Jonner's comments.

> The big thing that jumps out at me is: Do we need to somehow provide
> guards to only provide this functionality if the user's version of
> GDB is new enough?

After two years, I believe we can reasonably expect the jump-to
feature to be in most of the GDB installs.  Though I agree that we'd
need a generic framework that would allow us to ensure that GDB has
the given needed feature that we rely on.

> What happens if the user has an old version of GDB that doesn't
> provide this MI interface?  (I haven't actually tried to build the
> patch yet)

Nothing happens, I think :-)

> in on_debugger_breakpoints_set_signal(), when you're actually
> jumping, you might want to check (assert?) that
> m_priv->location_to_jump_to is not -1 before calling debugger
> ()->jump_to_location()

The new implementation doesn't use the "global" signal handler
mechanisms as the former implementer.  Rather,
IDebugger::set_breakpoint now takes a callback slot in argument.  That
slot is invoked when the breakpoint is set.  So we pass a particular
slot there to do some "break-before-jump" specific business, thus
doing away with the need for m_priv->location_to_jump_to.

 
> Typo: "Jump and stop to cursor and stop there" (extra "and stop")

Thanks.  Corrected.
 
> As I mentioned in a short discussion on IRC: I'm not sure that both
> of these commands are frequently-used enough to add 2 more items to
> the context menu (since it's already getting quite full), but we can
> reorganize the context menu in a different bug.

OK.  I'll let interested parties deal with the re-organization. :-)

> Also, Dodji mentioned in the IRC conversation that the 'jump and
> break' command will keep setting duplicate breakpoints if you jump
> to the same location multiple times, so it needs a bit more
> intelligence to check if there's a breakpoint set on that line
> already.

Yeah, this check is now done so there should be no more multiple
breakpoint issue here, hopefully.

> One other thought: right now the jump-and-break command needs to be
> handled explicitly by the DBGPerspective (e.g. it has to handle the
> 'set-breakpoint' signal and then send the 'jump' command).  Would it
> make sense to have that handled in the IDebugger instead?
> (e.g. maybe provide a bool parameter to the
> IDebugger::jump_to_location() function that specifies whether to
> stop after jumping or not).  This logic could then maybe be
> encapsulated into the debugger engine instead of the dbgperspective.
> What do you think?  I'm actually not not quite sure which way I
> prefer...

What you say makes sense.  I am fine either way.  I have left it
managed by DBGPerspective for now.  I guess if I ever need to re-use
the jump-and-break command from IDebugger, say, in a command line
interpreter or something like that, I'll move to IDebugger at that
point.
Comment 5 Dodji Seketeli 2011-05-07 09:09:50 UTC
I have merged support for this into master and gtk2-branch.

Note You need to log in before you can comment on or make changes to this bug.