Pedro Alves <palves@redhat.com> writes:
> 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_
except displaced stepping on arm linux target. However, GDB won't use
software single step for displaced stepping, because it either
single-step the instructions the scratch pad or resume the program from
the instructions in the scratch pad but these instructions end with a
breakpoint instruction (see arm_displaced_init_closure).
> 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.
I'd like to add some comments, see the patch below,
@@ -1208,8 +1208,6 @@ aarch64_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
set_gdbarch_displaced_step_free_closure (gdbarch,
simple_displaced_step_free_closure);
set_gdbarch_displaced_step_location (gdbarch, linux_displaced_step_location);
- set_gdbarch_displaced_step_hw_singlestep (gdbarch,
- aarch64_displaced_step_hw_singlestep);
}
/* Provide a prototype to silence -Wmissing-prototypes. */
@@ -2637,15 +2637,6 @@ aarch64_displaced_step_fixup (struct gdbarch *gdbarch,
}
}
-/* Implement the "displaced_step_hw_singlestep" gdbarch method. */
-
-int
-aarch64_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
- struct displaced_step_closure *closure)
-{
- return 1;
-}
-
/* Initialize the current architecture based on INFO. If possible,
re-use an architecture from ARCHES, which is a list of
architectures already created during this debugging session.
@@ -67,13 +67,6 @@ simple_displaced_step_free_closure (struct gdbarch *gdbarch,
xfree (closure);
}
-int
-default_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
- struct displaced_step_closure *closure)
-{
- return !gdbarch_software_single_step_p (gdbarch);
-}
-
CORE_ADDR
displaced_step_at_entry_point (struct gdbarch *gdbarch)
{
@@ -45,11 +45,6 @@ extern void
simple_displaced_step_free_closure (struct gdbarch *gdbarch,
struct displaced_step_closure *closure);
-/* Default implementation of gdbarch_displaced_hw_singlestep. */
-extern int
- default_displaced_step_hw_singlestep (struct gdbarch *,
- struct displaced_step_closure *);
-
/* Possible value for gdbarch_displaced_step_location:
Place displaced instructions at the program's entry point,
leaving space for inferior function call return breakpoints. */
@@ -277,7 +277,6 @@ struct gdbarch
gdbarch_skip_permanent_breakpoint_ftype *skip_permanent_breakpoint;
ULONGEST max_insn_length;
gdbarch_displaced_step_copy_insn_ftype *displaced_step_copy_insn;
- gdbarch_displaced_step_hw_singlestep_ftype *displaced_step_hw_singlestep;
gdbarch_displaced_step_fixup_ftype *displaced_step_fixup;
gdbarch_displaced_step_free_closure_ftype *displaced_step_free_closure;
gdbarch_displaced_step_location_ftype *displaced_step_location;
@@ -414,7 +413,6 @@ gdbarch_alloc (const struct gdbarch_info *info,
gdbarch->adjust_dwarf2_line = default_adjust_dwarf2_line;
gdbarch->register_reggroup_p = default_register_reggroup_p;
gdbarch->skip_permanent_breakpoint = default_skip_permanent_breakpoint;
- gdbarch->displaced_step_hw_singlestep = default_displaced_step_hw_singlestep;
gdbarch->displaced_step_fixup = NULL;
gdbarch->displaced_step_free_closure = NULL;
gdbarch->displaced_step_location = NULL;
@@ -624,7 +622,6 @@ verify_gdbarch (struct gdbarch *gdbarch)
/* Skip verify of skip_permanent_breakpoint, invalid_p == 0 */
/* Skip verify of max_insn_length, has predicate. */
/* Skip verify of displaced_step_copy_insn, has predicate. */
- /* Skip verify of displaced_step_hw_singlestep, invalid_p == 0 */
/* Skip verify of displaced_step_fixup, has predicate. */
if ((! gdbarch->displaced_step_free_closure) != (! gdbarch->displaced_step_copy_insn))
fprintf_unfiltered (log, "\n\tdisplaced_step_free_closure");
@@ -873,9 +870,6 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
"gdbarch_dump: displaced_step_free_closure = <%s>\n",
host_address_to_string (gdbarch->displaced_step_free_closure));
fprintf_unfiltered (file,
- "gdbarch_dump: displaced_step_hw_singlestep = <%s>\n",
- host_address_to_string (gdbarch->displaced_step_hw_singlestep));
- fprintf_unfiltered (file,
"gdbarch_dump: displaced_step_location = <%s>\n",
host_address_to_string (gdbarch->displaced_step_location));
fprintf_unfiltered (file,
@@ -3749,23 +3743,6 @@ set_gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch,
}
int
-gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, struct displaced_step_closure *closure)
-{
- gdb_assert (gdbarch != NULL);
- gdb_assert (gdbarch->displaced_step_hw_singlestep != NULL);
- if (gdbarch_debug >= 2)
- fprintf_unfiltered (gdb_stdlog, "gdbarch_displaced_step_hw_singlestep called\n");
- return gdbarch->displaced_step_hw_singlestep (gdbarch, closure);
-}
-
-void
-set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
- gdbarch_displaced_step_hw_singlestep_ftype displaced_step_hw_singlestep)
-{
- gdbarch->displaced_step_hw_singlestep = displaced_step_hw_singlestep;
-}
-
-int
gdbarch_displaced_step_fixup_p (struct gdbarch *gdbarch)
{
gdb_assert (gdbarch != NULL);
@@ -957,20 +957,6 @@ typedef struct displaced_step_closure * (gdbarch_displaced_step_copy_insn_ftype)
extern struct displaced_step_closure * gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch, CORE_ADDR from, CORE_ADDR to, struct regcache *regs);
extern void set_gdbarch_displaced_step_copy_insn (struct gdbarch *gdbarch, gdbarch_displaced_step_copy_insn_ftype *displaced_step_copy_insn);
-/* Return true if GDB should use hardware single-stepping to execute
- the displaced instruction identified by CLOSURE. If false,
- GDB will simply restart execution at the displaced instruction
- location, and it is up to the target to ensure GDB will receive
- control again (e.g. by placing a software breakpoint instruction
- into the displaced instruction buffer).
-
- The default implementation returns false on all targets that
- provide a gdbarch_software_single_step routine, and true otherwise. */
-
-typedef int (gdbarch_displaced_step_hw_singlestep_ftype) (struct gdbarch *gdbarch, struct displaced_step_closure *closure);
-extern int gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, struct displaced_step_closure *closure);
-extern void set_gdbarch_displaced_step_hw_singlestep (struct gdbarch *gdbarch, gdbarch_displaced_step_hw_singlestep_ftype *displaced_step_hw_singlestep);
-
/* Fix up the state resulting from successfully single-stepping a
displaced instruction, to give the result we would have gotten from
stepping the instruction in its original location.
@@ -780,17 +780,6 @@ V:ULONGEST:max_insn_length:::0:0
# that case.
M:struct displaced_step_closure *:displaced_step_copy_insn:CORE_ADDR from, CORE_ADDR to, struct regcache *regs:from, to, regs
-# Return true if GDB should use hardware single-stepping to execute
-# the displaced instruction identified by CLOSURE. If false,
-# GDB will simply restart execution at the displaced instruction
-# location, and it is up to the target to ensure GDB will receive
-# control again (e.g. by placing a software breakpoint instruction
-# into the displaced instruction buffer).
-#
-# The default implementation returns false on all targets that
-# provide a gdbarch_software_single_step routine, and true otherwise.
-m:int:displaced_step_hw_singlestep:struct displaced_step_closure *closure:closure::default_displaced_step_hw_singlestep::0
-
# Fix up the state resulting from successfully single-stepping a
# displaced instruction, to give the result we would have gotten from
# stepping the instruction in its original location.
@@ -2626,8 +2626,33 @@ 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);
+
+ /* Although GDB won't use software single step for displaced
+ stepping, we call software_single_step here to determine
+ whether to
+ - single step the instructions in the scratch pad, (like
+ x86, ppc and aarch64),
+ - or resume the program from the instructions in the scratch
+ pad. These instructions must end with a breakpoint
+ instruction. */
+ if (gdbarch_software_single_step_p (gdbarch))
+ {
+ VEC (CORE_ADDR) * next_pcs;
+
+ next_pcs = gdbarch_software_single_step (gdbarch,
+ get_current_frame ());
+
+ if (next_pcs != NULL)
+ {
+ step = 0;
+ VEC_free (CORE_ADDR, next_pcs);
+ }
+ else
+ step = 1;
+ }
+ else
+ step = 1;
+
}
}
@@ -1136,15 +1136,6 @@ ppc_displaced_step_fixup (struct gdbarch *gdbarch,
from + offset);
}
-/* Always use hardware single-stepping to execute the
- displaced instruction. */
-static int
-ppc_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
- struct displaced_step_closure *closure)
-{
- return 1;
-}
-
/* Checks for an atomic sequence of instructions beginning with a LWARX/LDARX
instruction and ending with a STWCX/STDCX instruction. If such a sequence
is found, attempt to step through it. A breakpoint is placed at the end of
@@ -6071,8 +6062,7 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
/* Setup displaced stepping. */
set_gdbarch_displaced_step_copy_insn (gdbarch,
ppc_displaced_step_copy_insn);
- set_gdbarch_displaced_step_hw_singlestep (gdbarch,
- ppc_displaced_step_hw_singlestep);
+
set_gdbarch_displaced_step_fixup (gdbarch, ppc_displaced_step_fixup);
set_gdbarch_displaced_step_free_closure (gdbarch,
simple_displaced_step_free_closure);
@@ -759,14 +759,6 @@ s390_software_single_step (struct frame_info *frame)
return next_pcs;
}
-static int
-s390_displaced_step_hw_singlestep (struct gdbarch *gdbarch,
- struct displaced_step_closure *closure)
-{
- return 1;
-}
-
-
/* Maps for register sets. */
static const struct regcache_map_entry s390_gregmap[] =
@@ -7990,7 +7982,6 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
set_gdbarch_inner_than (gdbarch, core_addr_lessthan);
set_gdbarch_breakpoint_from_pc (gdbarch, s390_breakpoint_from_pc);
set_gdbarch_software_single_step (gdbarch, s390_software_single_step);
- set_gdbarch_displaced_step_hw_singlestep (gdbarch, s390_displaced_step_hw_singlestep);
set_gdbarch_skip_prologue (gdbarch, s390_skip_prologue);
set_gdbarch_stack_frame_destroyed_p (gdbarch, s390_stack_frame_destroyed_p);