Re: [PATCH 2/2] Remove gdbarch method displaced_step_hw_singlestep

[Pedro Alves <palves@redhat.com>]

On 03/23/2016 02:10 PM, Yao Qi wrote:
> displaced_step_hw_singlestep was added for some targets, which can do
> hardware single step, but need software single step for some special
> instructions.  After we change gdbarch method software_single_step,
> displaced_step_hw_singlestep is no longer necessary, because we can
> get the same information from software_single_step.  

I wasn't sure whether this is safe, but I convinced myself it is.
I'd have been nice if there had been a note in the log about the below:

Currently, when stepping past an instruction in the scratch pad, these
archs won't ever reach the software_single_step method, always forcing a
hardware single-step, even if the software_single_step method would
insert some breakpoint.  The question is: is it safe now for them to
analyse the instruction copied to the scratch pad, and potentially insert
a software single-step?

I think it is safe, because we won't ever use displaced stepping
for the cases where the software_single_step method would return
something non-NULL.  software_single_step returns non-NULL _only_
for atomic regions, while OTOH, displaced_step_copy_insn always returns
NULL for atomic regions.  E.g., notice how ppc_displaced_step_copy_insn
vs ppc_deal_with_atomic_sequence.

> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2611,8 +2611,25 @@ resume (enum gdb_signal sig)
>   	  pc = regcache_read_pc (get_thread_regcache (inferior_ptid));
>   
>   	  displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid));
> -	  step = gdbarch_displaced_step_hw_singlestep (gdbarch,
> -						       displaced->step_closure);
> +
> +	  if (gdbarch_software_single_step_p (gdbarch))
> +	    {
> +	      VEC (CORE_ADDR) * next_pcs = NULL;

No need to initialize.

> +
> +	      next_pcs = gdbarch_software_single_step (gdbarch,
> +						       get_current_frame ());
> +
> +	      if (next_pcs != 0)

next_pcs != NULL.

Thanks,
Pedro Alves