catchpoint - bptype

Hello,

I need some input on the bptype enum, and catchpoint
treatment:

Right now we internally treat some of them as bp_breakpoint
and some as special bp_catch_*. Catchpoints
"catch" and "throw" are treated as
bp_breakpoint while others are treated as special types. For
example "fork" is bp_catch_fork.

I see that bp_catch_catch and bp_catch_trhow were removed in
Dec. 2007 and this confuses me a bit: what is the intention?
To get rid of all bp_catch_*? 

Since bptype is used to give more specific detail about a
breakpoint it makes sense to have bp_catch_catch and
bp_catch_throw there. It would also make those two
catchpoints first-class citizens again.

Thoughts? 

Thanks,

Aleksandar Ristovski
QNX Software Systems

> I see that bp_catch_catch and bp_catch_trhow were
removed in Dec. 2007 and 
> this confuses me a bit: what is the intention? To get
rid of all 
> bp_catch_*? 

I haven't looked at the implementation of the other
catchpoints, but
when I implemented Ada exception catchpoints, I really
appreciated
the new infrastructure which allowed me to use bp_breakpoint
instead
of having to add my own new bp_catchpoint enums (I tried the
latter
first). It allowed me to basically implement the
functionality in
a couple of functions instead of littering "case
bp_catchpoint_exception..."
everywhere in breakpoint.c.

I am not sure about the long term intentions in this area. 
I think
that the new approach based on breakpoint_ops can be
extremely effective,
but that assumes that the functionality is in fact
implemented using
an underlying breakpoint, regardless of the architecture.
Catchpoints
on fork or exec events, for instance, are not in this
category,
and thus require their own bp_ enum kind.

> Since bptype is used to give more specific detail about
a breakpoint it 
> makes sense to have bp_catch_catch and bp_catch_throw
there. It would also 
> make those two catchpoints first-class citizens again.

I don't understand why you think that not having their own
bp_catch
enum makes them less equal than the others.  Like I said
above, it
certainly made the implementation more compact and easier to
maintain.
At the user level, I don't think he's seeing much of a
difference either.

-- 
Joel

Joel Brobecker wrote:
>> I see that bp_catch_catch and bp_catch_trhow were
removed in Dec. 2007 and 
>> this confuses me a bit: what is the intention? To
get rid of all 
>> bp_catch_*? 
> 
> I haven't looked at the implementation of the other
catchpoints, but
> when I implemented Ada exception catchpoints, I really
appreciated
> the new infrastructure which allowed me to use
bp_breakpoint instead
> of having to add my own new bp_catchpoint enums (I
tried the latter
> first). It allowed me to basically implement the
functionality in
> a couple of functions instead of littering "case
bp_catchpoint_exception..."
> everywhere in breakpoint.c.
This is where breakpoint_ops could be used... but I first
want to understand the intent and what conceptually makes
more sense, and then we can talk about the implementation
details.

> 
> I am not sure about the long term intentions in this
area.  I think
> that the new approach based on breakpoint_ops can be
extremely effective,

I agree, but without knowing the long term intent it is hard
to tell. At the moment it introduces slight complication
since only "catch" and "throw" use ops
and nothing else (and, therefore, take different printing
route than anything else). I can see how breakpoint_ops can
be very useful, if used consistently - it could be used to,
for example, get rid of the switch statements you mentioned
above.

> I don't understand why you think that not having their
own bp_catch
> enum makes them less equal than the others.  Like I
said above, it
> certainly made the implementation more compact and
easier to maintain.
> At the user level, I don't think he's seeing much of a
difference either.
 
For example:
(gdb) catch catch
Catchpoint 2 (catch)
(gdb) catch fork
Catchpoint 3 (fork)
(gdb) info b
Num     Type           Disp Enb Address    What
2       breakpoint     keep y   0xb7f75896 exception catch
3       catch fork     keep y
(gdb)

See how "fork" is cool and "catch"
isn't. "Catch" looks just like any other
breakpoint; the only diff. is in "What" field,
while catch fork is clearly a catchpoint.

On Mon, Apr 28, 2008 at 01:28:57PM -0400, Aleksandar
Ristovski wrote:
> I agree, but without knowing the long term intent it is
hard to
> tell. At the moment it introduces slight complication
since only
> "catch" and "throw" use ops and
nothing else (and, therefore, take
> different printing route than anything else). I can see
how
> breakpoint_ops can be very useful, if used consistently
- it could
> be used to, for example, get rid of the switch
statements you
> mentioned above.

Why do you assume there is a long term intent? 

I don't want to add new elements to those switches unless
they are
really for things that do not behave like breakpoints.  I'd
be happy
to see patches removing existing cases.  That's why, when I
wrote new
code to catch C++ exceptions, I used breakpoint_ops.

> (gdb) info b
> Num     Type           Disp Enb Address    What
> 2       breakpoint     keep y   0xb7f75896 exception
catch
> 3       catch fork     keep y
> (gdb)
>
> See how "fork" is cool and "catch"
isn't. "Catch" looks just like
> any other breakpoint; the only diff. is in
"What" field, while catch
> fork is clearly a catchpoint.

If you can convince us it matters, we can change the
output.
Personally I think "breakpoint on exception catch"
is a perfectly
reasonable thing to call it - that's what it is.  The fork
catchpoints
are not like a breakpoint, though, since they do not
correspond to
any code address - just an OS event.

-- 
Daniel Jacobowitz
CodeSourcery

Daniel Jacobowitz wrote:
> On Mon, Apr 28, 2008 at 01:28:57PM -0400, Aleksandar
Ristovski wrote:
>> I agree, but without knowing the long term intent
it is hard to
>> tell. At the moment it introduces slight
complication since only
>> "catch" and "throw" use ops and
nothing else (and, therefore, take
>> different printing route than anything else). I can
see how
>> breakpoint_ops can be very useful, if used
consistently - it could
>> be used to, for example, get rid of the switch
statements you
>> mentioned above.
> 
> Why do you assume there is a long term intent? 

Just asking...

> 
> I don't want to add new elements to those switches
unless they are
> really for things that do not behave like breakpoints. 
I'd be happy
> to see patches removing existing cases.  That's why,
when I wrote new
> code to catch C++ exceptions, I used breakpoint_ops.

I think breakpoint_ops is a good approach, but I would dare
to say - incomplete. 
>> See how "fork" is cool and
"catch" isn't. "Catch" looks just like
>> any other breakpoint; the only diff. is in
"What" field, while catch
>> fork is clearly a catchpoint.
> 
> If you can convince us it matters, we can change the
output.

Just that the documentation treats them differently and
calls them catchpoints. And I would say that logically they
are kind of special... that's all. 

> Personally I think "breakpoint on exception
catch" is a perfectly
> reasonable thing to call it - that's what it is.  The
fork catchpoints
> are not like a breakpoint, though, since they do not
correspond to
> any code address - just an OS event.
>

On Mon, Apr 28, 2008 at 02:00:02PM -0400, Aleksandar
Ristovski wrote:
>> I don't want to add new elements to those switches
unless they are
>> really for things that do not behave like
breakpoints.  I'd be happy
>> to see patches removing existing cases.  That's
why, when I wrote new
>> code to catch C++ exceptions, I used
breakpoint_ops.
>
> I think breakpoint_ops is a good approach, but I would
dare to say - 
> incomplete. 

Yes, that's accurate.  I didn't change any of the existing
ones when I added the mechanism.

>>> See how "fork" is cool and
"catch" isn't. "Catch" looks just like
>>> any other breakpoint; the only diff. is in
"What" field, while catch
>>> fork is clearly a catchpoint.
>>
>> If you can convince us it matters, we can change
the output.
>
> Just that the documentation treats them differently and
calls them 
> catchpoints. And I would say that logically they are
kind of special... 
> that's all. 

If you want them displayed as catchpoints I'm amenable to a
patch.

-- 
Daniel Jacobowitz
CodeSourcery