Re: [rfc][rft (procfs, nto-procfs)] Fix signal bypass heuristic with software single-step

[Pedro Alves <pedro@codesourcery.com>]

On Friday 18 March 2011 20:20:49, Ulrich Weigand wrote:
> Hello,
> 
> several targets today implement a performance optimization where some signals
> are not reported back to the GDB core, but instead are directly delivered to
> the target; this can be done either by not even trapping those signals (e.g.
> procfs targets), by restarting the inferior directly in target code (e.g.
> linux-nat), or by restarting the inferior in the remote stub.
> 
> This works via a target callback to_notice_signal, which is called whenever
> the list of signals to be handled by GDB changes.  Target code is then
> expected to itself detect which signals are safe to short-circuit (by calling
> signal_pass_state) etc.  Target code must also handle (implicitly defined)
> special cases, e.g. that signals must always be reported during startup
> or when single-stepping.
> 
> It is this last condition that causes problems on architectures that use
> software single-step, because target code isn't even aware of that
> condition in general.  This causes failures on ARM, see the discussion here:
> http://sourceware.org/ml/gdb-patches/2011-01/msg00398.html
> 
> The solution that was discussed there is basically to make the implicit
> rules of signal passing shortcuts explicit.  The patch below is an
> attempt to implement this idea.  The basic concept is to remove the
> to_notice_signals callback and replace it by a to_pass_signals callback,
> which explicitly gets a list of signals that can be safely bypassed
> at this point.
> 
> Common code then calls this new target callback as appropriate, in
> particular to mark all signals non-bypassable when single-stepping.
> 
> All platforms using to_notice_signals (procfs, nto-procfs, remote)
> or directly accessing signal_pass_state etc (linux-nat) are updated
> to instead use the to_pass_signals callback.
> 
> Tested on i386-linux, both native and gdbserver, with no regressions.
> Fixes the same set of failures on ARM as the original patch.
> 
> Any thoughts on this approach?

As discussed before, I like the approach, but I have a couple of remarks
to the implementation:

 - There are more calls to target_resume in infrun.c.  Don't we need
to consider signals around those?

 - In non-stop, if you have this sequence:

     - step thread 1
     - continue thread 2 (imediately afterwards)
     - thread 1 gets signal FOO (which is normally pass-able).

   when handling the latter event, since the "signal_pass" set is
   global, and has been filled by the previous continue, the target
   will think it doesn't need to report the signal back to core gdb.

You've said before about this when I raised the issue before:

 "If the implementation is conservative in the right direction,
 the worst thing that could happen is that a signal is reported that
 might have gotten short-circuited .."

Might be we just need to check if _any_ thread is stepping, when
deciding whether to tell the target to report back all signals?

-- 
Pedro Alves