[PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Bogdan Slusarczyk]

Hi everyone, I wrote my own patch for -break-list. I'm not sure that it 
meets all requirements mentioned in 
http://sourceware.org/ml/gdb-patches/2008-01/msg00251.html and previous 
discussions, but combination -break-list + multiple breakpoints is now 
usable for me. I'm not familiar with gdb test suit, so it's NOT tested 
at all (except few my own cases).

What does it do? Until now -break-list returned:

(variable) bkpt
(tuple) {
   (result)
     (variable) number
     (c-string) 4
   (result)
     (variable) type
     (c-string) breakpoint
   (result)
     (variable) disp
     (c-string) keep
   (result)
     (variable) enabled
     (c-string) n
   (result)
     (variable) addr
     (c-string) <MULTIPLE>
   (result)
     (variable) addr
     (c-string) 0x6f14137f
   (result)
     (variable) times
     (c-string) 1
....


After my changes it returns additional list named 'locations' instead of 
second 'addr' field:

(variable) bkpt
(tuple) {
   (result)
     (variable) number
     (c-string) 4
   (result)
     (variable) type
     (c-string) breakpoint
   (result)
     (variable) disp
     (c-string) keep
   (result)
     (variable) enabled
     (c-string) y
   (result)
     (variable) addr
     (c-string) <MULTIPLE>
   (result)
     (variable) times
     (c-string) 1
   (result)
     (variable) locations
     (list) [
       (tuple) {
         (result)
           (variable) number
           (c-string) 4.1
         (result)
           (variable) enabled
           (c-string) y
         (result)
           (variable) addr
           (c-string) 0x6f14137f
         (result)
           (variable) func
           (c-string) inv::negate()
         (result)
           (variable) file
           (c-string) src/inv.cpp
         (result)
           (variable) fullname
           (c-string) c:/test/src/inv.cpp
         (result)
           (variable) line
           (c-string) 17
       (tuple) {
         (result)
           (variable) number
           (c-string) 4.2
         (result)
           (variable) enabled
           (c-string) y
         (result)
           (variable) addr
           (c-string) 0x0442137f
         (result)
           (variable) func
           (c-string) inv::negate()
         (result)
           (variable) file
           (c-string) src/inv.cpp
         (result)
           (variable) fullname
           (c-string) c:/test/src/inv.cpp
         (result)
           (variable) line
           (c-string) 17



Regards,
Bogdan



--- breakpoint.c	Tue Feb 26 09:14:12 2008
+++ c:/3/gdb-6.8/gdb/breakpoint.c	Wed Apr 02 11:45:31 2008
@@ -3385,7 +3385,7 @@ print_one_breakpoint_location (struct br
      loc = b->loc;

    annotate_record ();
-  bkpt_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "bkpt");
+  bkpt_chain = make_cleanup_ui_out_tuple_begin_end (uiout, 
!part_of_multiple ? "bkpt" : NULL);

    /* 1 */
    annotate_field (0);
@@ -3401,27 +3401,22 @@ print_one_breakpoint_location (struct br
        ui_out_field_int (uiout, "number", b->number);
      }

-  /* 2 */
+  /* 2 */
    annotate_field (1);
-  if (part_of_multiple)
-    ui_out_field_skip (uiout, "type");
-  else
-    {
+  if (!part_of_multiple)
+  {	
        if (((int) b->type >= (sizeof (bptypes) / sizeof (bptypes[0])))
  	  || ((int) b->type != bptypes[(int) b->type].type))
  	internal_error (__FILE__, __LINE__,
  			_("bptypes table does not describe type #%d."),
  			(int) b->type);
        ui_out_field_string (uiout, "type", bptypes[(int) 
b->type].description);
-    }
+  }

    /* 3 */
    annotate_field (2);
-  if (part_of_multiple)
-    ui_out_field_skip (uiout, "disp");
-  else
-    ui_out_field_string (uiout, "disp", bpdisps[(int) b->disposition]);
-
+  if (!part_of_multiple)
+	ui_out_field_string (uiout, "disp", bpdisps[(int) b->disposition]);

    /* 4 */
    annotate_field (3);
@@ -3542,15 +3537,19 @@ print_one_breakpoint_location (struct br
  	  {
  	    annotate_field (4);
  	    if (header_of_multiple)
-	      ui_out_field_string (uiout, "addr", "<MULTIPLE>");
-	    if (b->loc == NULL || loc->shlib_disabled)
-	      ui_out_field_string (uiout, "addr", "<PENDING>");
-	    else
-	      ui_out_field_core_addr (uiout, "addr", loc->address);
+			ui_out_field_string (uiout, "addr", "<MULTIPLE>");
+		else
+		{
+			if (b->loc == NULL || loc->shlib_disabled)
+			ui_out_field_string (uiout, "addr", "<PENDING>");
+			else
+			ui_out_field_core_addr (uiout, "addr", loc->address);
+		}
  	  }
  	annotate_field (5);
  	if (!header_of_multiple)
  	  print_breakpoint_location (b, loc, wrap_indent, stb);
+
  	if (b->loc)
  	  *last_addr = b->loc->address;
  	break;
@@ -3633,6 +3632,22 @@ print_one_breakpoint_location (struct br
        print_command_lines (uiout, l, 4);
        do_cleanups (script_chain);
      }
+
+	if ( header_of_multiple && ui_out_is_mi_like_p (uiout) )
+	{
+		struct cleanup *locations_chain;
+		struct bp_location *loc;
+		int n = 1;
+
+		annotate_field (10);
+		locations_chain = make_cleanup_ui_out_list_begin_end( uiout, 
"locations" );
+		
+		for (loc = b->loc; loc; loc = loc->next, n++ )
+			print_one_breakpoint_location (b, loc, n, last_addr);
+
+		do_cleanups (locations_chain);
+	}
+
    do_cleanups (bkpt_chain);
    do_cleanups (old_chain);
  }

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Nick Roberts]

Bogdan Slusarczyk writes:
 > Hi everyone, I wrote my own patch for -break-list. I'm not sure that it 
 > meets all requirements mentioned in 
 > http://sourceware.org/ml/gdb-patches/2008-01/msg00251.html and previous 
 > discussions, but combination -break-list + multiple breakpoints is now 
 > usable for me. I'm not familiar with gdb test suit, so it's NOT tested 
 > at all (except few my own cases).
 > 
 > What does it do? Until now -break-list returned:
 > ...
 > 
 > After my changes it returns additional list named 'locations' instead of 
 > second 'addr' field:

I don't think the second 'addr' field should be there, but I'm not sure that I
really like this 'locations' field.  4.1, 4.2 etc are breakpoints and so I
think they should be identified as such (using the bkptno field).  

Notice that you can set them individually:

-break-insert *0x6f14137f
-break-insert *0x0442137f

I also think that 

info break 4.1

and

-break-info 4.1

should just list that breakpoint.  With your patch it doesn't.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Vladimir Prus]

Nick Roberts wrote:

> Bogdan Slusarczyk writes:
>  > Hi everyone, I wrote my own patch for -break-list. I'm not sure that it
>  > meets all requirements mentioned in
>  > http://sourceware.org/ml/gdb-patches/2008-01/msg00251.html and previous
>  > discussions, but combination -break-list + multiple breakpoints is now
>  > usable for me. I'm not familiar with gdb test suit, so it's NOT tested
>  > at all (except few my own cases).
>  > 
>  > What does it do? Until now -break-list returned:
>  > ...
>  > 
>  > After my changes it returns additional list named 'locations' instead of
>  > second 'addr' field:
> 
> I don't think the second 'addr' field should be there, but I'm not sure that I
> really like this 'locations' field.  4.1, 4.2 etc are breakpoints and so I
> think they should be identified as such (using the bkptno field).

They are not really independent breakpoints. For example, condition is part of
the main breakpoint, and removing main breakpoint removes all locations, so
it's better to represent things this way.

- Volodya

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Nick Roberts]

 > > I don't think the second 'addr' field should be there, but I'm not sure
 > > that I really like this 'locations' field.  4.1, 4.2 etc are breakpoints
 > > and so I think they should be identified as such (using the bkptno field).
 > 
 > They are not really independent breakpoints. For example, condition is part
 > of the main breakpoint, and removing main breakpoint removes all locations,
 > so it's better to represent things this way.

Using the bkptno field means that existing frontends will recognise these
locations as breakpoints.   Only new frontends will be able to parse new
fields.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Vladimir Prus]

Nick Roberts wrote:

>  > > I don't think the second 'addr' field should be there, but I'm not sure
>  > > that I really like this 'locations' field.  4.1, 4.2 etc are breakpoints
>  > > and so I think they should be identified as such (using the bkptno field).
>  > 
>  > They are not really independent breakpoints. For example, condition is part
>  > of the main breakpoint, and removing main breakpoint removes all locations,
>  > so it's better to represent things this way.
> 
> Using the bkptno field means that existing frontends will recognise these
> locations as breakpoints.   

Yes, but those are not a breakpoints, do it will do a disservice to the existing
frontends. In particular, might find it very interesting experience to edit condition
of one breakpoint, and having conditions on other breakpoints change. Likewise, 
changing any properly of location will not work.

I think that right now, the only thing that existing frontend cannot do is to
individually disable and enable locations. This is nice thing to have, but not
very critical, so I think it's fine for only new frontends to have this capability,
given the presenting locations as breakpoints comes with a bunch of issues.

- Volodya

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Nick Roberts]

 > > Using the bkptno field means that existing frontends will recognise these
 > > locations as breakpoints.   
 > 
 > Yes, but those are not a breakpoints, do it will do a disservice to the
 > existing frontends. In particular, might find it very interesting experience
 > to edit condition of one breakpoint, and having conditions on other
 > breakpoints change. Likewise, changing any properly of location will not
 > work.
 > 
 > I think that right now, the only thing that existing frontend cannot do is
 > to individually disable and enable locations. This is nice thing to have,
 > but not very critical, so I think it's fine for only new frontends to have
 > this capability, given the presenting locations as breakpoints comes with a
 > bunch of issues.

If set individually, the multiple breakpoint locations can be used with
deleted, ignore, condition and commands.  Why would this not true when the
location is part of a multiple breakpoint?  Is it just due to the
implementation or a fundamental limitation?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Vladimir Prus]

Nick Roberts wrote:

>  > > Using the bkptno field means that existing frontends will recognise these
>  > > locations as breakpoints.
>  > 
>  > Yes, but those are not a breakpoints, do it will do a disservice to the
>  > existing frontends. In particular, might find it very interesting experience
>  > to edit condition of one breakpoint, and having conditions on other
>  > breakpoints change. Likewise, changing any properly of location will not
>  > work.
>  > 
>  > I think that right now, the only thing that existing frontend cannot do is
>  > to individually disable and enable locations. This is nice thing to have,
>  > but not very critical, so I think it's fine for only new frontends to have
>  > this capability, given the presenting locations as breakpoints comes with a
>  > bunch of issues.
> 
> If set individually, the multiple breakpoint locations can be used with
> deleted, ignore, condition and commands.  

To set individual breakpoint on constructor instances, for example,
you need to set breakpoint at address.

> Why would this not true when the 
> location is part of a multiple breakpoint?  Is it just due to the
> implementation or a fundamental limitation?

That's the design of multiple-location breakpoints. You specify the line
or function on which such a breakpoint should be set. GDB that arranges
for the list of locations to automatically include all relevant addresses,
including when shared libraries are loaded and unloaded. Note that while
there's mechanism to enable and disable individual locations, it's a bit
heuristic, so the enable/disable state might not be carried over when
new shared libraries are loaded.

Allowing the user to manipulate individual locations will interfere with
this automatic updating of location list. Furthermore, what is the use case?
For constructors, one is not likely to ever want to do anything with 
individual locations. For inlined functions, I don't know why you would
specifically treat one inlined instance, but if you wish, you can always create
a more specific breakpoint, like on address, and do anything.

Of course, we can provide a command that creates individual breakpoints on
each address matching a specification, and does not do any auto-update of
those breakpoints. If you think such a behaviour will be useful, can you
explain why, and then work on implementing it?

- Volodya

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Nick Roberts]

 > > If set individually, the multiple breakpoint locations can be used with
 > > deleted, ignore, condition and commands.  
 > 
 > To set individual breakpoint on constructor instances, for example,
 > you need to set breakpoint at address.
 > 
 > > Why would this not true when the 
 > > location is part of a multiple breakpoint?  Is it just due to the
 > > implementation or a fundamental limitation?
 > 
 > That's the design of multiple-location breakpoints. You specify the line
 > or function on which such a breakpoint should be set. GDB that arranges
 > for the list of locations to automatically include all relevant addresses,
 > including when shared libraries are loaded and unloaded. Note that while
 > there's mechanism to enable and disable individual locations, it's a bit
 > heuristic, so the enable/disable state might not be carried over when
 > new shared libraries are loaded.
 > 
 > Allowing the user to manipulate individual locations will interfere with
 > this automatic updating of location list. 

I'm not familar with the need to load and unload shared libraries, Gdb just
loads them automatically for me.

 >                                           Furthermore, what is the use case?
 > For constructors, one is not likely to ever want to do anything with
 > individual locations. For inlined functions, I don't know why you would
 > specifically treat one inlined instance, but if you wish, you can always
 > create a more specific breakpoint, like on address, and do anything.
 > 
 > Of course, we can provide a command that creates individual breakpoints on
 > each address matching a specification, and does not do any auto-update of
 > those breakpoints. If you think such a behaviour will be useful, can you
 > explain why, and then work on implementing it?

I can imagine it might be useful to control the breakpoint locations
individually but, in practice I've never needed multiple breakpoints yet.
However, if it's not useful then it's probably unlikely that anyone would
try to do it in a frontend as you suggested earlier:


     Yes, but those are not a breakpoints, do it will do a disservice to the
     existing frontends. In particular, might find it very interesting
     experience to edit condition of one breakpoint, and having conditions on
     other breakpoints change. Likewise, changing any properly of location will
     not work.


-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Vladimir Prus]

On Thursday 03 April 2008 15:09:30 you wrote:
>  > > If set individually, the multiple breakpoint locations can be used with
>  > > deleted, ignore, condition and commands.  
>  > 
>  > To set individual breakpoint on constructor instances, for example,
>  > you need to set breakpoint at address.
>  > 
>  > > Why would this not true when the 
>  > > location is part of a multiple breakpoint?  Is it just due to the
>  > > implementation or a fundamental limitation?
>  > 
>  > That's the design of multiple-location breakpoints. You specify the line
>  > or function on which such a breakpoint should be set. GDB that arranges
>  > for the list of locations to automatically include all relevant addresses,
>  > including when shared libraries are loaded and unloaded. Note that while
>  > there's mechanism to enable and disable individual locations, it's a bit
>  > heuristic, so the enable/disable state might not be carried over when
>  > new shared libraries are loaded.
>  > 
>  > Allowing the user to manipulate individual locations will interfere with
>  > this automatic updating of location list. 
> 
> I'm not familar with the need to load and unload shared libraries, Gdb just
> loads them automatically for me.

And while it loads them automatically, it also updates breakpoints. In particular
new locations may be added to a breakpoint. Was this not clear from what I wrote
above?

> 
>  >                                           Furthermore, what is the use case?
>  > For constructors, one is not likely to ever want to do anything with
>  > individual locations. For inlined functions, I don't know why you would
>  > specifically treat one inlined instance, but if you wish, you can always
>  > create a more specific breakpoint, like on address, and do anything.
>  > 
>  > Of course, we can provide a command that creates individual breakpoints on
>  > each address matching a specification, and does not do any auto-update of
>  > those breakpoints. If you think such a behaviour will be useful, can you
>  > explain why, and then work on implementing it?
> 
> I can imagine it might be useful to control the breakpoint locations
> individually but, in practice I've never needed multiple breakpoints yet.
> However, if it's not useful then it's probably unlikely that anyone would
> try to do it in a frontend as you suggested earlier:
> 
> 
>      Yes, but those are not a breakpoints, do it will do a disservice to the
>      existing frontends. In particular, might find it very interesting
>      experience to edit condition of one breakpoint, and having conditions on
>      other breakpoints change. Likewise, changing any properly of location will
>      not work.

So, you suggest that frontend should display a number of fields for breakpoint
locations in the hope that the user will never touch those fields? This will
just clutter the UI. 
 
- Volodya

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Bogdan Slusarczyk]

> I can imagine it might be useful to control the breakpoint locations
> individually but, in practice I've never needed multiple breakpoints yet.

I need it. Problem is that specific applications loads the same binaries 
few times (see SystemC simulators), and multiple breakpoints are very 
useful.

> However, if it's not useful then it's probably unlikely that anyone would
> try to do it in a frontend as you suggested earlier:
> 
> 
>      Yes, but those are not a breakpoints, do it will do a disservice to the
>      existing frontends. In particular, might find it very interesting
>      experience to edit condition of one breakpoint, and having conditions on
>      other breakpoints change. Likewise, changing any properly of location will
>      not work.

I did changes for -break-list because I have situation when I have to 
disable one of location, and I have to find such location. Location 
behaves in different way as regular breakpoint, so I think it's not good 
idea to mix this things. I treat locations rather as breakpoint 
properties than regular breakpoints, but it's only my own opinion.

Regards,
Bogdan

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Vladimir Prus]

Bogdan Slusarczyk wrote:

>> I can imagine it might be useful to control the breakpoint locations
>> individually but, in practice I've never needed multiple breakpoints yet.
> 
> I need it. Problem is that specific applications loads the same binaries
> few times (see SystemC simulators), and multiple breakpoints are very
> useful.

Do you need to just enable/disable individual locations, or edit any other
properties (which is not allowed)?

- Volodya

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Bogdan Slusarczyk]

>>> I can imagine it might be useful to control the breakpoint locations
>>> individually but, in practice I've never needed multiple breakpoints yet.
>> I need it. Problem is that specific applications loads the same binaries
>> few times (see SystemC simulators), and multiple breakpoints are very
>> useful.
> 
> Do you need to just enable/disable individual locations, or edit any other
> properties (which is not allowed)?
> 

At the moment only disable/enable (because of 
http://sourceware.org/ml/gdb/2008-02/msg00180.html).

Regards,
Bogdan

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Daniel Jacobowitz]

On Wed, Apr 02, 2008 at 04:12:58PM +0200, Bogdan Slusarczyk wrote:
> Hi everyone, I wrote my own patch for -break-list. I'm not sure that it  
> meets all requirements mentioned in  
> http://sourceware.org/ml/gdb-patches/2008-01/msg00251.html and previous  
> discussions, but combination -break-list + multiple breakpoints is now  
> usable for me. I'm not familiar with gdb test suit, so it's NOT tested at 
> all (except few my own cases).

There was some discussion of the behavior of multiple-location
breakpoints after this patch was posted, but no discussion of the
actual patch.  Volodya, could you look at it - does this seem like the
right way?

Bogdan, thanks for the patch.  Are you interested in further
contribution to GDB?  If so, I can send you the copyright paperwork we
require; I think this sneaks by just under the limit, but we prefer to
have it on file for the future.

To commit this we'll eventually need manual and testsuite updates.
You don't necessarily have to handle those yourself (though I'm sure
it would be appreciated if you did - on the other hand, at that point
we'd definitely need the paperwork).

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Vladimir Prus]

On Monday 14 April 2008 21:50:43 Daniel Jacobowitz wrote:
> On Wed, Apr 02, 2008 at 04:12:58PM +0200, Bogdan Slusarczyk wrote:
> > Hi everyone, I wrote my own patch for -break-list. I'm not sure that it  
> > meets all requirements mentioned in  
> > http://sourceware.org/ml/gdb-patches/2008-01/msg00251.html and previous  
> > discussions, but combination -break-list + multiple breakpoints is now  
> > usable for me. I'm not familiar with gdb test suit, so it's NOT tested at 
> > all (except few my own cases).
> 
> There was some discussion of the behavior of multiple-location
> breakpoints after this patch was posted, but no discussion of the
> actual patch.  Volodya, could you look at it - does this seem like the
> right way?

I'll review this patch, but I think only when I get back from the 
KDevelop hackathon -- so not earlier than this Saturday.

- Volodya

Re: [PATCH, gdb6.8] -break-list doesn't list multiple breakpoints

[Bogdan Slusarczyk]

> Bogdan, thanks for the patch.  Are you interested in further
> contribution to GDB? 

If I have any solutions I will share it of course, but I don't think it 
will be very often.

If so, I can send you the copyright paperwork we
> require; I think this sneaks by just under the limit, but we prefer to
> have it on file for the future.

Fell free to use my changes. If you need my agreement, send me copyright 
paperwork.

> To commit this we'll eventually need manual and testsuite updates.
> You don't necessarily have to handle those yourself (though I'm sure
> it would be appreciated if you did - on the other hand, at that point
> we'd definitely need the paperwork).

I realize that I should do it myself, but I need some time to 
familiarize with all your rules and mechanisms. If you can check my 
changes yourself, do it, please.

Regards,
Bogdan