[0/9] Breakpoints at multiple locations

[0/9] Breakpoints at multiple locations

[Vladimir Prus]

At the moment, gdb assumes that a code breakpoint has
a single PC value. One case where it does not work
is C++ constructors -- GCC generates several function
bodies, and GDB sets breakpoint on just one of them,
so breakpoints in constructors don't work reliably.
Another case is C++ templates -- if I set a breakpoint
on a line in template function, there's unbounded number
of template instantiations that have this line, but GDB
cannot set breakpoints on all of them.

I'm about to post a set of patches that fix it. Essentially,
the patches make breakpoint have a list of breakpoint locations.
When a breakpoint is creates, GDB automatically figures out the
set of locations to use. User can than disable locations that seem
uninteresting to him.

GDB can also handle locations in shared libraries. Suppose you
have breakpoint at foo.hpp:10, which is a template function.
Then, when new shared library is loaded, gdb will figure out
if there is any code in the library corresponding to foo.hpp:4.
If now, the list of locations for the breakpoint will be updated.

As a side effect, this set of patches simplifies internal handling
of pending breakpoints. Since we need the code to recompute all
locations when a shared library is loaded, there's no need to special-case
pending breakpoints, so now pending breakpoint is a breakpoint without
any locations.  Importantly, when pending breakpoint is resolved, it
no longer gets a new number, which should make MI frontends a little
bit happier.

The patches has direct effect on two user commands. The "info break"
now outputs multiple locations, if they are present, like this:

        Num Type           Disp Enb  Address    What
        1   breakpoint     keep y    <MULTIPLE>
        1.1                     y    0xb7fa756d in int bar<int>(int) at helper.hpp:4
        1.2                     y    0xb7fa7588 in double bar<double>(double) at helper.hpp:4


The output for a breakpoint with single location is not changed. 

The display of pending breakpoints is improved. In CVS HEAD as it is now, 
breakpoint internally has three important states:

	- enabled
	- disabled
	- shlib_disabled

Breakpoint enters the last state when the library it is in is unloaded. 
User can never explicitly specify this state. Unfortunately, user can 
explicitly leave this state, using the 'enable' command. The result will be
an error on next resume -- as gdb will try to insert this breakpoint, in
an unloaded library. My patches explicitly separate "enabled/disabled" state,
which can be changed by user, and the "pending" state -- which means that
this breakpoint is not in a loaded code yet, and won't fire yet. The pending
state is maintained by gdb. The display is adjusted accordingly -- 
for pending breakpoints we show "(p)" after "y" or "n", for example:

        1   breakpoint     keep y    <MULTIPLE>
                breakpoint already hit 2 times
        1.1                     y(p) 0xb7f9856d helper.hpp:4
        1.2                     n(p) 0xb7f98588 helper.hpp:4

means that user wants location 1.1 to be enabled, and wants location 1.2
to be disabled. The "(p)" means that both are in unloaded shared library,
so naturally 1.1 won't be hit until the library is loaded.

In addition to "info break" changes, the "enable" and "disable" commands
were modified to accept locations, so:

	disable 1.2 

will disable second location of breakpoint 1.

Since the original patch was about 100K and non-intelligible, it was
split in 9 separate patches. Each leaves gdb without regressions and
some patches have a value on their own.

	1. Remove stale code in update_breakpoints_after_exec
	2. Remove the 'silent' parameter in disable_breakpoints_in_shlibs.
	3. Introduce bpstat_free
	4. Change bpstat->breakpoint_at to point at bp_location.
	5. Move the 'cond' field from breakpoint to bp_location.
	6. Extract some code into separate functions
	7. Handle pending breakpoints in breakpoint_re_set_one
	8. Allow breakpoint to have multiple locations.
	9. Create multiple locations for breakpoints in constructors
	and template functions

Patches 1-3 are rather trivial, and already committed. The remaining patches
were already reviewed by Jim Blandy, and revised, and re-reviewed 
several times, and are actually all approved now. However, breakpoint.c is 
scary, and the patches are big, so I'll appreciate all possible review. If I 
don't get any objections within 2 weeks, I'm gonna commit those patches as is.

- Volodya

RE: [0/9] Breakpoints at multiple locations

[Pierre Muller]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Vladimir Prus
> Sent: Friday, September 07, 2007 10:11 PM
> To: gdb-patches@sources.redhat.com
> Subject: [0/9] Breakpoints at multiple locations
> 
> 
> At the moment, gdb assumes that a code breakpoint has
> a single PC value. One case where it does not work
> is C++ constructors -- GCC generates several function
> bodies, and GDB sets breakpoint on just one of them,
> so breakpoints in constructors don't work reliably.
> Another case is C++ templates -- if I set a breakpoint
> on a line in template function, there's unbounded number
> of template instantiations that have this line, but GDB
> cannot set breakpoints on all of them.

  Another case is pascal (and probably other languages) that
allow overloading of functions, i.e. to implement several flavors
of a same name function, differing by their parameters.

  So I will be most happy about this change inside gdb.

Pierre Muller
Pascal language maintainer.

Re: [0/9] Breakpoints at multiple locations

[Eli Zaretskii]

> From: Vladimir Prus <vladimir@codesourcery.com>
> Date: Sat, 8 Sep 2007 00:11:13 +0400
> 
> At the moment, gdb assumes that a code breakpoint has
> a single PC value. One case where it does not work
> is C++ constructors -- GCC generates several function
> bodies, and GDB sets breakpoint on just one of them,
> so breakpoints in constructors don't work reliably.
> Another case is C++ templates -- if I set a breakpoint
> on a line in template function, there's unbounded number
> of template instantiations that have this line, but GDB
> cannot set breakpoints on all of them.
> 
> I'm about to post a set of patches that fix it. Essentially,
> the patches make breakpoint have a list of breakpoint locations.
> When a breakpoint is creates, GDB automatically figures out the
> set of locations to use. User can than disable locations that seem
> uninteresting to him.

Thanks, this is a great feature to have in GDB.

I hope there will be a suitable patch for the user manual, before all
this is committed.  I tried to indicate what new features need to be
reflected in the manual, when I saw them in your patches (but I'm not
sure I saw all of them).

Also, I think this features should be mentioned in NEWS.

> The patches has direct effect on two user commands. The "info break"
> now outputs multiple locations, if they are present, like this:
> 
>         Num Type           Disp Enb  Address    What
>         1   breakpoint     keep y    <MULTIPLE>
>         1.1                     y    0xb7fa756d in int bar<int>(int) at helper.hpp:4
>         1.2                     y    0xb7fa7588 in double bar<double>(double) at helper.hpp:4

As I wrote in one of my messages, the "1.2" thing could potentially
break alignment of the display.  I think we should handle that
somehow.

Also, I'm not convinced that 1.2 is the best possible way to express
what you want, because it looks like a floating-point number.  Maybe
we should explicitly have an additional "Loc" column, and format the
list like this:

        Num Loc  Type           Disp Enb  Address    What
        1        breakpoint     keep y    <MULTIPLE>
        1   1                        y    0xb7fa756d in int bar<int>(int) at helper.hpp:4
        1   2                        y    0xb7fa7588 in double bar<double>(double) at helper.hpp:4

>         1   breakpoint     keep y    <MULTIPLE>
>                 breakpoint already hit 2 times
>         1.1                     y(p) 0xb7f9856d helper.hpp:4
>         1.2                     n(p) 0xb7f98588 helper.hpp:4
> 
> means that user wants location 1.1 to be enabled, and wants location 1.2
> to be disabled. The "(p)" means that both are in unloaded shared library,
> so naturally 1.1 won't be hit until the library is loaded.

I think the old <PENDING> was much more clear than this enigmatic
"(p)".

Thanks again for working on this.

Re: [0/9] Breakpoints at multiple locations

[Vladimir Prus]

Eli Zaretskii wrote:

>> I'm about to post a set of patches that fix it. Essentially,
>> the patches make breakpoint have a list of breakpoint locations.
>> When a breakpoint is creates, GDB automatically figures out the
>> set of locations to use. User can than disable locations that seem
>> uninteresting to him.
> 
> Thanks, this is a great feature to have in GDB.
> 
> I hope there will be a suitable patch for the user manual, before all
> this is committed.  I tried to indicate what new features need to be
> reflected in the manual, when I saw them in your patches (but I'm not
> sure I saw all of them).
> 
> Also, I think this features should be mentioned in NEWS.

I'll take care of this after code details are sorted out.

>> The patches has direct effect on two user commands. The "info break"
>> now outputs multiple locations, if they are present, like this:
>> 
>>         Num Type           Disp Enb  Address    What
>>         1   breakpoint     keep y    <MULTIPLE>
>>         1.1                     y    0xb7fa756d in int bar<int>(int) at
>>         helper.hpp:4
>>         1.2                     y    0xb7fa7588 in double
>>         bar<double>(double) at helper.hpp:4
> 
> As I wrote in one of my messages, the "1.2" thing could potentially
> break alignment of the display.  I think we should handle that
> somehow.
> 
> Also, I'm not convinced that 1.2 is the best possible way to express
> what you want, because it looks like a floating-point number.  

I don't find that confusing, but it's me. 

> Maybe 
> we should explicitly have an additional "Loc" column, and format the
> list like this:
> 
>         Num Loc  Type           Disp Enb  Address    What
>         1        breakpoint     keep y    <MULTIPLE>
>         1   1                        y    0xb7fa756d in int bar<int>(int)
>         at helper.hpp:4
>         1   2                        y    0xb7fa7588 in double
>         bar<double>(double) at helper.hpp:4


This way, you'll have "Loc" column even if there are no multi-location
breakpoints. I'm not sure I like this.

>>         1   breakpoint     keep y    <MULTIPLE>
>>                 breakpoint already hit 2 times
>>         1.1                     y(p) 0xb7f9856d helper.hpp:4
>>         1.2                     n(p) 0xb7f98588 helper.hpp:4
>> 
>> means that user wants location 1.1 to be enabled, and wants location 1.2
>> to be disabled. The "(p)" means that both are in unloaded shared library,
>> so naturally 1.1 won't be hit until the library is loaded.
> 
> I think the old <PENDING> was much more clear than this enigmatic
> "(p)".

I believe you're comparing different things. The <PENDING> is the output
in address field, when no address at all was ever established for the breakpoint.
With my patches, you'll still see this in address field for a freshly created
pending breakpoint.

The "(p)" comes to play when you already have a breakpoint, and the library it's
in is unloaded. Then, you have old address, but the breakpoint won't actually be hit.
Old code has three breakpoint enablement states -- "y", "n", and "p".
New code has "y", "n", "y(p)", and "n(p)". It seems to me that (p) is no more
enigmatic than "p". And the benefit is that we don't mess with "enabled/disabled" state
that the user can set. 

To clarify, below is the examples of "info break" output for various cases:

For a freshly added pending breakpoint (zero locations yet):

        Num Type           Disp Enb  Address    What
        1   breakpoint     keep y(p) <PENDING>  f

When that breakpoint is resolved, we naturally get this:

        Num Type           Disp Enb  Address    What
        1   breakpoint     keep y    0xb7ee5412 in f at bp3b.c:7

When the library is unloaded, we'll get:

        Num Type           Disp Enb  Address    What
        1   breakpoint     keep y(p) 0xb7fb6412 bp3b.c:7

For a multiple breakpoint location, once it's resolved, we'll get this:

        Num Type           Disp Enb  Address    What
        1   breakpoint     keep y    <MULTIPLE>
        1.1                     y    0xb7fa756d in int bar<int>(int) at helper.hpp:4
        1.2                     y    0xb7fa7588 in double bar<double>(double) at helper.hpp:4

We can disable one location, to get this:

        Num Type           Disp Enb  Address    What
        1   breakpoint     keep y    <MULTIPLE>
        1.1                     y    0xb7fa756d in int bar<int>(int) at helper.hpp:4
        1.2                     n    0xb7fa7588 in double bar<double>(double) at helper.hpp:4

Once the library is unloaded, we get:

        Num Type           Disp Enb  Address    What
        1   breakpoint     keep y    <MULTIPLE>
                breakpoint already hit 2 times
        1.1                     y(p) 0xb7f9856d helper.hpp:4
        1.2                     n(p) 0xb7f98588 helper.hpp:4


Hope this clarifies.

- Volodya

Re: [0/9] Breakpoints at multiple locations

[Nathan J. Williams]

Vladimir Prus <vladimir@codesourcery.com> writes:

> When the library is unloaded, we'll get:
>
>         Num Type           Disp Enb  Address    What
>         1   breakpoint     keep y(p) 0xb7fb6412 bp3b.c:7

Are we really sure that the library will be reloaded to the same
address, in general?

	- Nathan

Re: [0/9] Breakpoints at multiple locations

[Vladimir Prus]

On Saturday 08 September 2007 20:27:00 Nathan J. Williams wrote:
> Vladimir Prus <vladimir@codesourcery.com> writes:
> 
> > When the library is unloaded, we'll get:
> >
> >         Num Type           Disp Enb  Address    What
> >         1   breakpoint     keep y(p) 0xb7fb6412 bp3b.c:7
> 
> Are we really sure that the library will be reloaded to the same
> address, in general?

It won't be in general. The address above is basically the last
address the breakpoint had.  When the library is reloaded, the
address may change.  I have no opinion whether showing last
address is good thing, but that's what current version of gdb does.

- Volodya

Re: [0/9] Breakpoints at multiple locations

[Daniel Jacobowitz]

On Sat, Sep 08, 2007 at 08:40:13PM +0400, Vladimir Prus wrote:
> It won't be in general. The address above is basically the last
> address the breakpoint had.  When the library is reloaded, the
> address may change.  I have no opinion whether showing last
> address is good thing, but that's what current version of gdb does.

Right - actually, I think it would be better if GDB forgot about such
addresses when the library is unloaded; that's separate.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [0/9] Breakpoints at multiple locations

[Markus Deuling]

Hi Vladimir,

great patch set. I'll test it for our ppc64/spu target. I guess it will be very suitable :-)

Vladimir Prus schrieb:
> 
> Once the library is unloaded, we get:
> 
>         Num Type           Disp Enb  Address    What
>         1   breakpoint     keep y    <MULTIPLE>
>                 breakpoint already hit 2 times
>         1.1                     y(p) 0xb7f9856d helper.hpp:4
>         1.2                     n(p) 0xb7f98588 helper.hpp:4

Just an idea but for my opinion it would be great to have a hit-counter per location like:

         Num Type           Disp Hit Enb  Address    What
         1   breakpoint     keep   8 y    <MULTIPLE>
         1.1                       5 y(p) 0xb7f9856d helper.hpp:4
         1.2                       3 n(p) 0xb7f98588 helper.hpp:4

The <MULTIPLE> entity could add up the counts of all locations.  This would also get rid of the 
annoying "breakpoint already hit xy times" line :-)

What do you think?

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com

Re: [0/9] Breakpoints at multiple locations

[Vladimir Prus]

On Monday 10 September 2007 08:46:09 Markus Deuling wrote:
> Hi Vladimir,
> 
> great patch set. I'll test it for our ppc64/spu target. I guess it will be very suitable :-)
> 
> Vladimir Prus schrieb:
> > 
> > Once the library is unloaded, we get:
> > 
> >         Num Type           Disp Enb  Address    What
> >         1   breakpoint     keep y    <MULTIPLE>
> >                 breakpoint already hit 2 times
> >         1.1                     y(p) 0xb7f9856d helper.hpp:4
> >         1.2                     n(p) 0xb7f98588 helper.hpp:4
> 
> Just an idea but for my opinion it would be great to have a hit-counter per location like:
> 
>          Num Type           Disp Hit Enb  Address    What
>          1   breakpoint     keep   8 y    <MULTIPLE>
>          1.1                       5 y(p) 0xb7f9856d helper.hpp:4
>          1.2                       3 n(p) 0xb7f98588 helper.hpp:4
> 
> The <MULTIPLE> entity could add up the counts of all locations.  This would also get rid of the 
> annoying "breakpoint already hit xy times" line :-)
> 
> What do you think?

That can possibly be interesting, but how will you handle 'ignore' 
count then? Should it be per-location, too?

I actually don't think those patches are final -- I expect some UI tweaks
to be made when this feature is more used. 

- Volodya

Re: [0/9] Breakpoints at multiple locations

[Markus Deuling]

>> Vladimir Prus schrieb:
>>> Once the library is unloaded, we get:
>>>
>>>         Num Type           Disp Enb  Address    What
>>>         1   breakpoint     keep y    <MULTIPLE>
>>>                 breakpoint already hit 2 times
>>>         1.1                     y(p) 0xb7f9856d helper.hpp:4
>>>         1.2                     n(p) 0xb7f98588 helper.hpp:4
>> Just an idea but for my opinion it would be great to have a hit-counter per location like:
>>
>>          Num Type           Disp Hit Enb  Address    What
>>          1   breakpoint     keep   8 y    <MULTIPLE>
>>          1.1                       5 y(p) 0xb7f9856d helper.hpp:4
>>          1.2                       3 n(p) 0xb7f98588 helper.hpp:4
>>
>> The <MULTIPLE> entity could add up the counts of all locations.  This would also get rid of the 
>> annoying "breakpoint already hit xy times" line :-)
>>
>> What do you think?
> 
> That can possibly be interesting, but how will you handle 'ignore' 
> count then? Should it be per-location, too?
> 
> I actually don't think those patches are final -- I expect some UI tweaks
> to be made when this feature is more used. 

Hm, after a very-quick-look through breakpoint.c I see no problem in moving
both ignore_count and hit_count to per-location. I guess it would make sense
to let these two together and have them in a per-location way. This reflects
reality best. GDB may want to set different ignore counts to different breakpoint
locations and count how often a specific location was hit.

Maybe thats something to come up with after the initial patch set for your multiple
location breakpoints are committed.

-- 
Markus Deuling
GNU Toolchain for Linux on Cell BE
deuling@de.ibm.com

Re: [0/9] Breakpoints at multiple locations

[Vladimir Prus]

Markus Deuling wrote:

> 
>>> Vladimir Prus schrieb:
>>>> Once the library is unloaded, we get:
>>>>
>>>>         Num Type           Disp Enb  Address    What
>>>>         1   breakpoint     keep y    <MULTIPLE>
>>>>                 breakpoint already hit 2 times
>>>>         1.1                     y(p) 0xb7f9856d helper.hpp:4
>>>>         1.2                     n(p) 0xb7f98588 helper.hpp:4
>>> Just an idea but for my opinion it would be great to have a hit-counter
>>> per location like:
>>>
>>>          Num Type           Disp Hit Enb  Address    What
>>>          1   breakpoint     keep   8 y    <MULTIPLE>
>>>          1.1                       5 y(p) 0xb7f9856d helper.hpp:4
>>>          1.2                       3 n(p) 0xb7f98588 helper.hpp:4
>>>
>>> The <MULTIPLE> entity could add up the counts of all locations.  This
>>> would also get rid of the annoying "breakpoint already hit xy times"
>>> line :-)
>>>
>>> What do you think?
>> 
>> That can possibly be interesting, but how will you handle 'ignore'
>> count then? Should it be per-location, too?
>> 
>> I actually don't think those patches are final -- I expect some UI tweaks
>> to be made when this feature is more used.
> 
> Hm, after a very-quick-look through breakpoint.c I see no problem in
> moving both ignore_count and hit_count to per-location. I guess it would
> make sense to let these two together and have them in a per-location way.
> This reflects reality best. GDB may want to set different ignore counts to
> different breakpoint locations and count how often a specific location was
> hit.

There's no technical problem doing that. However, I'm not quite sure if this
is good idea in general, and I even less sure what MI frontends might want
to do. Recall that now, I operates in kind of compatibility mode -- it does not list
breakpoint locations at all. So if we change to per-location hit/ignore, we need
to think how to not break MI frontends, and we should think how future MI version
will handle all this.

> 
> Maybe thats something to come up with after the initial patch set for your
> multiple location breakpoints are committed.

Definitely, maintaining this all locally is too much work, so tweaking
interface is better done separately.

- Volodya