Re: [rfc] Fix PowerPC displaced stepping regression

Pedro Alves
2009-09-28T17:31:37Z

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