Re: [rfc] Fix PowerPC displaced stepping regression

[Pedro Alves <pedro at codesourcery dot com>]

On Monday 28 September 2009 18:12:44, Ulrich Weigand wrote:
> Pedro Alves wrote:
> 
> > On Sunday 27 September 2009 22:47:13, Ulrich Weigand wrote:
> > > - In non-stop mode, we never want to use software single-step as
> > >   common code does not support this in multiple threads at once.
> > 
> > Right.  Shouldn't we switch this particular predicate to
> > check the non_stop global instead?
> 
> I'm not sure which "particular predicate" you're referring to, sorry ...
> 
> The check currently reads:
> 
>   if (use_displaced_stepping (gdbarch)
>       && (tp->trap_expected
>           || (step && gdbarch_software_single_step_p (gdbarch)))
>       && sig == TARGET_SIGNAL_0)
> 
> that is, if we'd otherwise be about to issue a single step (potentially)
> treat it like stepping over a breakpoint.  At what point would you
> suggest to check for non_stop?

At the points where we decide to use displaced stepping because
software single-stepping doesn't work with multiple pending
simultaneous requests.  So, adding a non_stop check there in
front of software_single_step_p,

  if (use_displaced_stepping (gdbarch)
      && (tp->trap_expected
-	  || (step && gdbarch_software_single_step_p (gdbarch)))
+	  || (non_stop && step && gdbarch_software_single_step_p (gdbarch)))


... and in maybe_software_singlestep:

-      if (use_displaced_stepping (gdbarch))
-      if (non_stop)
        hw_step = 0;

(or make resume clear `step' if "non_stop && step && gdbarch_software_single_step_p (gdbarch))"
so to we'd not reach maybe_software_singlestep at all.

But, let's ignore this.  The only benefit would be for
all-stop + displaced stepping=on to not trigger displaced
stepping all the way.

> > Did you consider making the gdbarch_displaced_step_copy_insn
> > callback itself return that it expects the target to be
> > continued instead of stepped?
> 
> Yes, but this would have required changes to the existing gdbarch
> interface that would have meant updating all existing users; and
> I wanted to produce a patch that doesn't touch any platform I
> cannot test at this point ...
> 
> In any case, the two interfaces should be pretty much identical:
> a target can simply set a flag in its "closure" and return this
> flag from the displaced_step_hw_singlestep routine.  That's why
> I'm passing the closure in, even though PPC doesn't need it ...

True.  I like that!

-- 
Pedro Alves