Re: [patch 3/3] bpstat_what removal [rediff]

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Wed, 16 Jun 2010 22:39:44 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
[...]
> One thing I would like to understand better is the long-term design.
> It seems to me that right now we have a few problems.  I would like to
> understand (1) whether and how your patch fixes them, and (2) what our
> (by which I guess I mostly mean Pedro's :-) ideal design would look
> like.
> 
> My understanding of the problems:
> 
> * We don't nicely handle multiple breakpoints at one location.
>   (AFAIK not touched by your change.)

It gets fixed AFAIK.  Or at least improved, not sure if the fix is complete.
It needs also a patch of this series:
	Re: [patch 2/3] bpstat_print should consider all candidates
	http://sourceware.org/ml/gdb-patches/2010-05/msg00368.html

As explained by Pedro currently some events are ignored but they get re-hit on
a vain attempt to resume at a breakpoint location and all the events get
executed properly.  Described by Pedro's "Well, not cancel, but postpone." ...:
	Re: [patch 3/3] bpstat_what removal [rediff]
	http://sourceware.org/ml/gdb-patches/2010-06/msg00404.html


> * bpstat_what is difficult to understand & modify, particularly the table.
>   (You fixed this.)  

The table part gets removed even in the Pedro's generally agreed
simplification:
	Re: [patch 3/3] bpstat_what removal [rediff]
	http://sourceware.org/ml/gdb-patches/2010-06/msg00372.html

Other code readability issues are both not agreed upon and they are strongly
bound to the "re-hit" bug-to-bug compatibility described above.


> * It is too hard to add new kinds of breakpoints.  See ifunc or
>   next-over-throw.
>   (Maybe partially fixed?)

It should become generally easy now even with the Pedro's generally agreed
simplification:
	Re: [patch 3/3] bpstat_what removal [rediff]
	http://sourceware.org/ml/gdb-patches/2010-06/msg00372.html

The is the major change it has been design-approved the actions can get
executed in breakpoint.c instead of infrun.c which simplified it a lot.


> For multiple breakpoints ... to me it seems like we want to have some
> way to notify *all* the breakpoints at a given spot, with some kind of
> decision making coming later about how to proceed.
> 
> A case that has come up a couple times is having a way for Python to add
> new invisible breakpoints, with Python code attached.

As the action can get executed from breakpoint.c now it can accordingly set
the bpstat_what() result making the "new invisible breakpoints" simple IMO.


> Pedro> To recap, IMO, the current problem with bpstat_main_action is
> Pedro> that a few of its values aren't really independent and mutually
> Pedro> exclusive with the others -- BPSTAT_WHAT_CHECK_SHLIBS and
> Pedro> BPSTAT_WHAT_CHECK_JIT.
> 
> Jan> What if a new breakpoint type wants to stop?  What if a new
> Jan> breakpoint type does not want to stop?  And how they combine if
> Jan> they happend together with other events?  While there exists answer
> Jan> for it I do not know offhand.  I know offhand with my
> Jan> implementation.  I should post a patch introducing new breakpoint
> Jan> types in both variants of the bpstat_what implementation.
> 
> Are you saying that the bpstat_main_action actually encodes several
> things which should actually be independent?  I think that is consistent
> with what I see in your patch, I just wanted to be sure.

There are three levels of independence:

No independence - current FSF GDB - everything is encoded as a single
decision.  This is wrong, some functionality gets lost - PR 9436.

Partial independence - Pedro's simplification
	Re: [patch 3/3] bpstat_what removal [rediff]
	http://sourceware.org/ml/gdb-patches/2010-06/msg00372.html
which makes BPSTAT_WHAT_CHECK_SHLIBS and BPSTAT_WHAT_CHECK_JIT independent
thus no longer breaking PR 9436.  But it still hides some information as shown
in my `set debug infrun' reproducer in
	Re: [patch 3/3] bpstat_what removal [rediff]
	http://sourceware.org/ml/gdb-patches/2010-06/msg00403.html
unfortunately current GDB code is (reportedly - I do not know a reproducer)
relies on this "re-hit" bug as described by Pedro in:
	Re: [patch 3/3] bpstat_what removal [rediff]
	http://sourceware.org/ml/gdb-patches/2010-06/msg00404.html

Full independence - my original proposals
	[patch 3/3] bpstat_what removal
	http://sourceware.org/ml/gdb-patches/2010-05/msg00050.html
and with an interface
	Re: [patch 3/3] bpstat_what removal
	http://sourceware.org/ml/gdb-patches/2010-05/msg00370.html
which have been generally approved as a future goal but currently they
(a) seem to be a too large change to be approved in a single step and
(b) it may break some bug-to-bug compatibility relying on the "re-hit"s.



Thanks,
Jan