pending breakpoints change breakpoint number

[Jim Ingham <jingham at apple dot com>]

Hi, all...

I have been looking at the pending breakpoint changes (to merge them 
into the Apple gdb).  Overall it is nicer than the future-break version 
that we had, so that is great.

However, one thing really bugs me about it.  That is that it changes 
the breakpoint number out from under me when it manages to resolve the 
breakpoint.

This is pretty annoying to a human user, since if I want to do anything 
to the breakpoint, I have to go find it's number again from info break. 
 But it is very bad for a gui that is trying to run gdb from, say, the 
MI.  The GUI has to use the breakpoint number to do things like disable 
the breakpoint, delete it, etc.  So if the number switches out from 
under it, the gui can no longer operate on the breakpoint.  Very bad...

So there are two things that need to be addressed here.

First off, if we are going to change the breakpoint number, clearly 
need a breakpoint_resolved event/hook/observer thingie that we can use 
to tell the GUI that breakpoint 2 resolved to breakpoint 5, and the GUI 
needs to change accordingly.  This is pretty easy.  If it were a 
gdb_event, the addition to gdb-events.sh would look like:

2004-04-14  Jim Ingham  <jingham@apple.com>

        * gdb-events.sh: define a breakpoint_resolve event.
        * breakpoint.c (create_breakpoints): Post the event.
        * mi-main.h: Declare the mi resolve breakpoint hook.
        * mi-interp.c: Add a gdb_event structure.  Fill the 
breakpoint_resolve field.
        * (mi_interpreter_resume): Set the mi gdb_event handler.
        * mi-cmd-break.c (mi_async_breakpoint_resolve_event) New 
function.

Attachment: pending-break.diff
Description: Binary data

I haven't played with the observer dingus that Andrew mentioned 
earlier, so I  don't know how it would look for that...

But even further, the question comes to mind "Why don't we just copy 
the old breakpoint's number over to the new breakpoint when it is 
made".  There are two objections to this I can think of.

One trivial one is that this leaves the breakpoint chain for a little 
while with two breakpoints with the same number.  I think it would be 
reasonable to declare that if you pass create_breakpoints a pending_bp, 
that breakpoint is create_breakpoint's to do with as it wishes - to 
delete if that is necessary or whatever.  Then we could delete it right 
when we made the new one and avoid this wart.  That might even be a 
little cleaner that having all the callers of 
resolve_pending_breakpoints have to delete the breakpoint.  OTOH, the 
callers all delete the pended breakpoint right away anyway, so this is 
more of a formal concern that a real problem...

A more substantial objection is that the pending breakpoint could end 
up resolving to more than one breakpoint.  Then you obviously can't 
just reuse the original number.

I think in the long term this is not actually correct.  As I understand 
it, the whole point of the breakpoint location structure within the 
breakpoint is to track these multiple breakpoint hits under one logical 
breakpoint.  So for something like a template or an inlined function, I 
would expect that we WOULD want there to be just one breakpoint, with 
multiple locations.  In that case, why not keep the breakpoint number?  
Moreover, in the vast majority of cases, the breakpoint goes one to one 
from the pended one to the new one.  So maybe in the short term, we 
could just detect whether there were only one or more than one sals, 
and reuse the number or not accordingly.  This would make it more 
convenient for casual use.

This is easy to do:

Index: breakpoint.c
===================================================================
RCS file: /cvs/src/src/gdb/breakpoint.c,v
retrieving revision 1.165
diff -p -p -r1.165 breakpoint.c
*** breakpoint.c        23 Mar 2004 14:47:55 -0000      1.165
--- breakpoint.c        16 Apr 2004 00:40:48 -0000
*************** create_breakpoints (struct symtabs_and_l
*** 4904,4911 ****
          describe_other_breakpoints (sal.pc, sal.section);

        b = set_raw_breakpoint (sal, type);
!       set_breakpoint_count (breakpoint_count + 1);
!       b->number = breakpoint_count;
        b->cond = cond[i];
        b->thread = thread;
        if (addr_string[i])
--- 4907,4921 ----
          describe_other_breakpoints (sal.pc, sal.section);

        b = set_raw_breakpoint (sal, type);
!         /* If we only made one breakpoint from a pending breakpoint,
!            don't change its number.  That's just annoying.  */
!         if (pending_bp != NULL && sals.nelts == 1)
!           b->number = pending_bp->number;
!         else
!           {
!             set_breakpoint_count (breakpoint_count + 1);
!             b->number = breakpoint_count;
!           }
        b->cond = cond[i];
        b->thread = thread;
        if (addr_string[i])

BTW, even if you don't make a new breakpoint number the resolved 
message is still useful, since the GUI might want to mark breakpoints 
differently if they are pended or have resolved.

Jim
--
Jim Ingham                                   jingham@apple.com
Developer Tools
Apple Computer