This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/2] Remove gdbarch method displaced_step_hw_singlestep
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>, gdb-patches at sourceware dot org
- Date: Wed, 23 Mar 2016 17:26:23 +0000
- Subject: Re: [PATCH 2/2] Remove gdbarch method displaced_step_hw_singlestep
- Authentication-results: sourceware.org; auth=none
- References: <1458742206-622-1-git-send-email-yao dot qi at linaro dot org> <1458742206-622-3-git-send-email-yao dot qi at linaro dot org>
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