This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: minimalistic MI catch support


> Date: Fri, 10 Feb 2006 11:48:11 -0500
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Nick Roberts <nickrob@snap.net.nz>, gdb-patches@sourceware.org
> 
> On Fri, Feb 10, 2006 at 05:41:18PM +0200, Eli Zaretskii wrote:
> > > Date: Fri, 10 Feb 2006 09:11:38 -0500
> > > From: Daniel Jacobowitz <drow@false.org>
> > > Cc: Nick Roberts <nickrob@snap.net.nz>, gdb-patches@sourceware.org
> > > 
> > > > I think this is a bug (or undocumented misfeature) in
> > > > breakpoint.c:handle_gnu_v3_exceptions.  You will see that it sets the
> > > > type of the raw breakpoint it creates to bp_breakpoint, instead of
> > > > using the argument ex_event that is passed by the caller.  In
> > > > addition, for some reason, it forces a special function to be used for
> > > > printing these catchpoints, and that function
> > > > (print_one_exception_catchpoint) doesn't use the list of descriptions
> > > > in the bptypes[] array used by the more general code in
> > > > print_one_breakpoint.  I think it's bad to have more than one place to
> > > > have these description strings.
> > > 
> > > That's not a bug
> > 
> > Which part?  I was talking about two things, not one, and only called
> > the first one a possible bug.
> 
> Well, I was responding to "I think this is a bug in", which I
> associated with "You will see that".
> 
> > > ex_event is not suitable for two reasons; one is that EX_EVENT_CATCH
> > > has little direct relationship to the bp_* constants
> > 
> > ??? Isn't there a 1:1 correspondence between them?
> 
> Nope.  I agree this is unclear.
> 
> EX_EVENT_CATCH means "the user has asked for a 'catch catch'
> operation".  bp_catch_catch means "we have set an OS-specific
> catchpoint on the 'an exception has been caught' event".  In
> the original case, these two did correspond; but for modern C++ ABIs
> (ignoring the issue of Windows SEH here for the moment...) they no
> longer correspond, and there is no event external to the process
> that we can capture.  Instead, we set a breakpoint at an ABI-defined
> location.

Thanks.  This is so confusing that it ought to be documented
somewhere.

> I've got no objection to changing the info breakpoints output to
> describe them as catchpoints, if that's considered superior.

Nick, would you like to make this change?

> Hmm.  Good idea.  I don't think it really fits into "maint info
> breakpoints" at the moment, but maybe we can rearrange... I think
> we ought to defer this until the one-to-many breakpoint issues are
> finally addressed, which will hopefully be this year.

I'm okay with deferring this, let's just not forget it when the time
comes.

> > I'd suggest some comments saying that this area is in flux and that
> > the intent is to make all the bp_* types look like catch and throw.
> > Otherwise, it's impossible to know where we are aiming, and this could
> > bite someone who will want to submit patches in that area.
> 
> That's a good idea.  I'll leave myself a note to write such, unless
> someone else wants to.

Thanks.

> > > [Now we're talking about fork catchpoints rather than EH.]
> > > 
> > > There's no meangingful address.
> > 
> > Yes, but what code knows about that? i.e. where in the code is it
> > clear that the address isn't displayed?
> 
> For instance,
> 
>       case bp_catch_fork:
>       case bp_catch_vfork:
>         /* Field 4, the address, is omitted (which makes the columns
>            not line up too nicely with the headers, but the effect
>            is relatively readable).  */
>         if (addressprint)
>           ui_out_field_skip (uiout, "addr");

Yes, I was fooled by a similar code that does the same for
bp_catch_catch (which I thought was run for "catch catch", whose
address _is_ displayed).  Also, to _not_ print when the variable
addressprint is non-zero doesn't really help to read the code, and the
comment (repeated N times elsewhere) doesn't really explain anything,
IMHO.

Anyway, if there's no meaningful address in this case, then we
shouldn't print one for "catch catch", either.  I take it that this is
currently hard to do, since there's no sign in the breakpoint data
structure that this breakpoint is special, right?  I guess this all
boils down to adding two more bp_* enum members, one each for the
new-style catch and throw breakpoints.  Then the print routines will
know to DTRT (and the code will be clearer as well).

> > >   - Software watchpoints.  I still plan someday to finish refactoring
> > >     this so that a single watchpoint, software or otherwise, has
> > >     multiple bp_location entries, each with a valid address.
> > 
> > ??? Under what circumstances would a watchpoint have to have several
> > locations?  It's watching data, and data has one address only, right?
> > What am I missing?
> 
> Oh, there's plenty of examples.  Here's an easy one.  Suppose we have
> an int **ptr, and the user sets a watchpoint on **ptr.  That's
> implemented with two watchpoints: one on ptr, and one on *ptr.  When
> ptr changes, the watchpoint on *ptr has to be moved.

You are talking about watching expressions, and you are talking
intermittently about high-level and raw watchpoints.  I was thinking
only about raw watchpoints we insert on the target side, thus the
misunderstanding.

So do you suggest to show all the raw watchpoints used to watch an
expression?  That could mean an awfully large list of addresses.

> We do this with both software and hardware watchpoints.

Sure, this is common to them all.

>       case bp_watchpoint:
>       case bp_hardware_watchpoint:
>       case bp_read_watchpoint:
>       case bp_access_watchpoint:
>         /* Field 4, the address, is omitted (which makes the columns
>            not line up too nicely with the headers, but the effect
>            is relatively readable).  */
> 
> I'm not sure of the address field is used for hardware watchpoints
> or not, actually, but in any case it is not displayed.  Just the
> expression is.

I don't see why not display for the watchpoints the address of the
first value on the value chain.  This will at least show the right
address when the expression is actually a simple variable.  WDYT?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]