Re: [WIP] pending breakpoint support

["J. Johnston" <jjohnstn at redhat dot com>]

Daniel Jacobowitz wrote:
> On Tue, Nov 18, 2003 at 08:03:09PM -0500, J. Johnston wrote:
>
> > I have been hacking around with supporting "future-break" functionality 
> > (note, I said "hacking").  I have seen past discussion regarding the issue 
> > and the requirement to add functionality to the current breakpoint command 
> > as opposed to creating a separate command.
>
>
> Wonderful!  As you may guess, I have some comments :)
>
>
> > For my change, I wrappered the call to parse_breakpoint_sals() to catch any 
> > error.  If the call fails, then I simply give the user the option as 
> > marking it as pending.  I then make a fake breakpoint with a special 
> > enable_state of bp_shlib_pending.  I also save the original breakpoint 
> > command as the addr_string plus some needed state to recreate the command 
> > at a later time.  I attempted to have any original condition parsed but it 
> > isn't used in my current design as I end up reparsing the original command 
> > from scratch anyway.
>
>
> I don't really like this implementation (bp_shlib_pending).  To explain
> why, I need to take a step back to the last conversation about multiple
> breakpoints.  Particularly, the question of what "break foo" should do
> in various cases.  Here's some of the simple ones:
>
> 1.	int foo(void);
> 	int foo(int);
>
> 2.	int foo<int>(void);
> 	int foo<char>(void);
>
> 3.	foo.c:static int foo(void);
> 	bar.c:static int foo(void);
>
> 4.	libfoo.so:int foo(void);
> 	libbar.so:int foo(void);
> 	appbar.exe:int foo(void);
>
> The answer for (1) is pretty straightforward.  Right now we prompt and
> create multiple independent breakpoints, and I don't see a problem with
> that.  I believe that's currently what we do for (2) also - a little
> less clearcut but it still seems reasonable to me.
>
> For (3) we currently pick one at random (whichever appears first); when
> the global symbol lookup fails we iterate through static blocks. 
> That's bad.  We should prompt, and do as in (1).
>
> All clear so far, but the one I really wanted to point out was (4).  I
> believe that it shouldn't be handled like the others.  Instead it
> should be handled like I am planning to handle inlined functions - as
> one breakpoint with multiple addresses (and some special handling for
> PLT entries...).  For a particular example of where this is useful, on
> at least Darwin the libsupc++ functions used for C++ exception handling
> can appear in multiple shared objects, and each will use its own.  A
> breakpoint on one of them had best be placed on all of them if you
> really want GDB to catch all exceptions!  In general, in this case,
> there are too many factors to predict which copy of a function will be
> called; so the least confusing thing for GDB to do would be to
> breakpoint on all of them.
>
> This suggests that a different implementation is in order, because when
> a shared library is loaded we may want to expand a bp_enabled
> breakpoint to have two addresses also.  Support for breakpoints with a
> variable number of location elements is coming "soon"; it's my next
> planned project.

Well, we could treat the breakpoint as "try for all newly loaded shared 
libraries".  This way, the pending (or whatever you want to call it) breakpoint 
sticks around instead of being deleted after first resolution.  We could easily 
change it to be enabled/disabled simply by adding an additional enable_state 
(one means pending_enabled, the second means pending_disabled).  Alternatively, 
a new flag could be added.

To address Jim's concerns about having too many libraries to try the breakpoint 
on, a conditional could be added for the shared library or libraries to attempt 
the breakpoint on.  For example,

b printf
Function printf not found
Reattempt breakpoint later on shared library loads (Y/N)?
.
cond 1 $_shlib==libc.so.* || $_shlib == libm.so.*

In this case, the breakpoint would only be attempted if a shared library 
matching the templates libc.so.* or libm.so.* is being loaded.  I have 
overloaded the conditional as an example.  This could be done with a 
meta-conditional but I figure this might be simpler as a user can see everything 
at once with the current output.

One question would be what to do with the pending breakpoint when the program is 
rerun.  We could do a number of following options:

 1. remove it unconditionally (assume program has resolved and created all
    needed breakpoints)

 2. leave it around and check all resolved pending breakpoints, avoiding
    duplication (this would require a flag to indicate a breakpoint was a
    resolved pending breakpoint)

 3. put it under control of a new option

> > When we are loading shared libraries, the function 
> > re_enable_breakpoints_in_shlibs() gets called.  I have added code in there 
> > to attempt to reparse any pending break command.  If successful, a new 
> > breakpoint is created by basically reissing the command with saved state 
> > accounted for. After creating the new breakpoint(s), I delete the pending 
> > place holder breakpoint.
>
>
> This raises an unpleasant question.  If one of the two functions above
> doesn't have adequate debug information to evaluate the condition, what
> do we do?  Punt?  What's punting in this case - making one of them
> unconditional, making both of them unconditional, not breakpointing the
> one?  Warning the user presumably.

IMHO, it would be reasonable to issue a message regarding the conditional is not 
applicable for the shared library "x" and then offer the user the choice to 
offer an alternative condition (including empty condition) or to simply not set 
the breakpoint.

> One of many sticky interface problems I've thought of but not come up
> with a good answer for.
>
>
> > Ideally, I would liked to have reused the same breakpoint structure that 
> > was initially allocated but in my code there still was the possibility of 
> > one command generating multiple breakpoints so I took the aforementioned 
> > strategy. I have been informed that the newer breakpoint model will 
> > eliminate that problem and I should be able to reuse the breakpoint number, 
> > etc...
>
>
> Yes, this should be OK.

Actually, by making the pending breakpoint stick around as I suggest above, I 
won't be deleting the breakpoint so this is no longer applicable.

> > A problem I didn't solve yet has to do with the issuing of error messages 
> > for pending and disabled shared-library breakpoints when the reenablement 
> > is attempted and it fails.  This can be very annoying when you have a large 
> > number of shared libraries loaded each time (e.g. Eclipse).
> >
> > I have included a gdb session below along with the patch I used so folks 
> > can see how it currently works.
> >
> > What I would like to do is get some discussion going:
> >
> >  1. Is the user interface on the right track?
> >  2. What gotchas do I need to think about?
> >  3. Any design recommendations for implementing this better?
>
>
> > (gdb) b printf
> > Function "printf" not defined.
> > Make breakpoint pending? (y or n) y
> > Breakpoint 1 (printf) pending.
>
>
> The wording needs to be improved.  If I hadn't been working on pending
> breakpoints I'd have no idea what it meant.  How about something like
> this (still very awkward):
>
> (gb) b printf
> Function "printf" not currently defined.
> Create a pending breakpoint for later definitions of "printf" loaded
> from shared libraries? (y or n) y
> Breakpoint 1 (printf) pending.
>
>
> > (gdb) info break
> > Num Type           Disp Enb Address            What
> > 1   breakpoint     keep n   <PENDING>          printf
>
>
> One reason I'd like this separated from enable state; I think it should
> be marked as y.  That way the user can "disable" it if necessary. 
> Also, it would be nice to be symmetric with the bp_shlib_disabled
> handling (similar problem).

I agree with the ability to enable/disable.  See above.
>