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

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Fri, 18 Jun 2010 13:42:30 +0200, Pedro Alves wrote:
> There are code paths in handle_inferior_event that _want_ to resume until
> a breakpoint is rehit, related to (nested) signals.  Not sure they apply in
> this case, I/We'll need to dig further.  (another reason for wanting to have
> that change separated).

That is very surprising to me, so far I have considered such re-hits as
clearly a bug in handle_inferior_event, not considering other code may expect
it.  I agree now my patch went too far and it should be split (like you did).


> > But former BPSTAT_WHAT_STEP_RESUME and BPSTAT_WHAT_SET_LONGJMP_RESUME
> > (therefore those using `keep_going (ecs); return;' make a mess there as they
> > cancel lower priorities wanting to stop.
> 
> Well, not cancel, but postpone.

Without of possibility getting some event lost?  (watchpoints come to my mind)
Not important now as I see there is an approved plan to remove it anyway.


> I have trouble thinking how you'd have a simultaneous
> BPSTAT_WHAT_STEP_RESUME along with an solib or jit event, but,

There is missing and planned "new invisible breakpoints, with Python code
attached" stated in:
	Tom Tromey: Re: [patch 3/3] bpstat_what removal [rediff]
	http://sourceware.org/ml/gdb-patches/2010-06/msg00376.html

which can happen for (after re-hits start to be prevented):
	call_func_without_debuginfo ();
	line_with_python_invisible_breakpoint;


> let's put thread event breakpoints and other kind of event breakpoints we'll
> come up with, or even gdb side tracepoints in the mix.

I assume you mean the case I have described now above.


> All these breakpoints have the property that you do want to
> avoid considering rehits introduced by these "spurious" resumes as separate
> hits.  Easier to think about if you consider tracepoints (you'd have double,
> or more collects for the same tracepoint hit), and that does indeed suggest
> something needs to change.

Great there is approval of this intention of re-hits removal.


> The goal was getting the table our of the way, which is supposedly
> a non-behaviour (almost mechanical actually) change, so we can concentrate
> on the interface and infrun issues.

OK, true the table itself is currently the most obvious pain.


> > This violates the goal of my patch to make its reviewing easier by not
> > changing the behavior in any way for the cases only a single event happens.
> 
> (while making it hard to review because it changes the behavior when
>  multiple breakpoints happen :-) )

... as the (IMO) "obviously broken behavior" is expected by the current code.



Thanks,
Jan