aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Tromey <tromey@adacore.com>2025-08-06 07:14:56 -0600
committerTom Tromey <tromey@adacore.com>2025-08-06 07:22:11 -0600
commit2f6db3e2e77d7e10b407fb43b13a985e9b4211ed (patch)
tree463df82c189fa6e074a078299a5d0e57f636de95
parentc2729c37f10af09126b2916215cae425ae724f55 (diff)
downloadbinutils-2f6db3e2e77d7e10b407fb43b13a985e9b4211ed.zip
binutils-2f6db3e2e77d7e10b407fb43b13a985e9b4211ed.tar.gz
binutils-2f6db3e2e77d7e10b407fb43b13a985e9b4211ed.tar.bz2
Revert "Call target_can_do_single_step from maybe_software_singlestep"
This reverts commit 14de1447c9c52c1bfc52588f8652836f66ac6c47. An automated tester said that this patch caused a regression on aarch64: FAIL: gdb.arch/aarch64-atomic-inst.exp: Step through the ldxr/stxr sequence (timeout) I looked into it a bit yesterday but couldn't see an obvious problem; and it's somewhat of a pain to try to debug it at the moment. Tom de Vries also noticed this and filed it in bugzilla. So, I'm backing the patch out until I can port the failing test to the AdaCore internal test suite in order to find out what went wrong. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28440 Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33255
-rw-r--r--gdb/arm-linux-tdep.c5
-rw-r--r--gdb/breakpoint.c25
-rw-r--r--gdb/breakpoint.h8
-rw-r--r--gdb/gdbarch-gen.h12
-rw-r--r--gdb/gdbarch_components.py12
-rw-r--r--gdb/infrun.c19
-rw-r--r--gdb/record-full.c4
7 files changed, 53 insertions, 32 deletions
diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 2f034af..08526d8 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -960,6 +960,11 @@ arm_linux_software_single_step (struct regcache *regcache)
struct gdbarch *gdbarch = regcache->arch ();
struct arm_get_next_pcs next_pcs_ctx;
+ /* If the target does have hardware single step, GDB doesn't have
+ to bother software single step. */
+ if (target_can_do_single_step () == 1)
+ return {};
+
arm_get_next_pcs_ctor (&next_pcs_ctx,
&arm_linux_get_next_pcs_ops,
gdbarch_byte_order (gdbarch),
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f795d7b..d704ad1 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13998,26 +13998,11 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch,
update_global_location_list (UGLL_INSERT);
}
-/* Try to setup for software single stepping. Return true if
- target_resume() should use hardware single step.
+/* Insert single step breakpoints according to the current state. */
- GDBARCH is the current gdbarch. */
-
-bool
-maybe_software_singlestep (struct gdbarch *gdbarch)
+int
+insert_single_step_breakpoints (struct gdbarch *gdbarch)
{
- if (execution_direction != EXEC_FORWARD)
- return true;
-
- if (target_can_do_single_step () == 1)
- {
- /* The target definitely has hardware single step. */
- return true;
- }
-
- if (!gdbarch_software_single_step_p (gdbarch))
- return true;
-
regcache *regcache = get_thread_regcache (inferior_thread ());
std::vector<CORE_ADDR> next_pcs;
@@ -14031,10 +14016,10 @@ maybe_software_singlestep (struct gdbarch *gdbarch)
for (CORE_ADDR pc : next_pcs)
insert_single_step_breakpoint (gdbarch, aspace, pc);
- return false;
+ return 1;
}
else
- return true;
+ return 0;
}
/* See breakpoint.h. */
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index e9201bc..9341112 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1884,10 +1884,10 @@ extern void insert_single_step_breakpoint (struct gdbarch *,
const address_space *,
CORE_ADDR);
-/* Try to setup for software single stepping. Return true if
- target_resume() should use hardware single step. GDBARCH is the
- current gdbarch. */
-extern bool maybe_software_singlestep (struct gdbarch *);
+/* Insert all software single step breakpoints for the current frame.
+ Return true if any software single step breakpoints are inserted,
+ otherwise, return false. */
+extern int insert_single_step_breakpoints (struct gdbarch *);
/* Check whether any hardware watchpoints have triggered or not,
according to the target, and record it in each watchpoint's
diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
index 1e51081..281b97b 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -776,10 +776,16 @@ extern void set_gdbarch_get_memtag (struct gdbarch *gdbarch, gdbarch_get_memtag_
extern CORE_ADDR gdbarch_memtag_granule_size (struct gdbarch *gdbarch);
extern void set_gdbarch_memtag_granule_size (struct gdbarch *gdbarch, CORE_ADDR memtag_granule_size);
-/* Return a vector of addresses at which the software single step
- breakpoints should be inserted. An empty vector means software single
- step is not used.
+/* FIXME/cagney/2001-01-18: This should be split in two. A target method that
+ indicates if the target needs software single step. An ISA method to
+ implement it.
+ FIXME/cagney/2001-01-18: The logic is backwards. It should be asking if the
+ target can single step. If not, then implement single step using breakpoints.
+
+ Return a vector of addresses on which the software single step
+ breakpoints should be inserted. NULL means software single step is
+ not used.
Multiple breakpoints may be inserted for some instructions such as
conditional branch. However, each implementation must always evaluate
the condition and only put the breakpoint at the branch destination if
diff --git a/gdb/gdbarch_components.py b/gdb/gdbarch_components.py
index 55df102..91c867e 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -1378,10 +1378,16 @@ For a non-zero value, this represents the number of bytes of memory per tag.
Function(
comment="""
-Return a vector of addresses at which the software single step
-breakpoints should be inserted. An empty vector means software single
-step is not used.
+FIXME/cagney/2001-01-18: This should be split in two. A target method that
+indicates if the target needs software single step. An ISA method to
+implement it.
+FIXME/cagney/2001-01-18: The logic is backwards. It should be asking if the
+target can single step. If not, then implement single step using breakpoints.
+
+Return a vector of addresses on which the software single step
+breakpoints should be inserted. NULL means software single step is
+not used.
Multiple breakpoints may be inserted for some instructions such as
conditional branch. However, each implementation must always evaluate
the condition and only put the breakpoint at the branch destination if
diff --git a/gdb/infrun.c b/gdb/infrun.c
index e0e9ffa..9d3e1b7 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -93,6 +93,8 @@ static void insert_step_resume_breakpoint_at_caller (const frame_info_ptr &);
static void insert_longjmp_resume_breakpoint (struct gdbarch *, CORE_ADDR);
+static bool maybe_software_singlestep (struct gdbarch *gdbarch);
+
static void resume (gdb_signal sig);
static void wait_for_inferior (inferior *inf);
@@ -2357,6 +2359,23 @@ set_schedlock_func (const char *args, int from_tty, struct cmd_list_element *c)
process. */
bool sched_multi = false;
+/* Try to setup for software single stepping. Return true if target_resume()
+ should use hardware single step.
+
+ GDBARCH the current gdbarch. */
+
+static bool
+maybe_software_singlestep (struct gdbarch *gdbarch)
+{
+ bool hw_step = true;
+
+ if (execution_direction == EXEC_FORWARD
+ && gdbarch_software_single_step_p (gdbarch))
+ hw_step = !insert_single_step_breakpoints (gdbarch);
+
+ return hw_step;
+}
+
/* See infrun.h. */
ptid_t
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 7d4ef87..7e3da27 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1105,7 +1105,7 @@ record_full_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
record_full_resume_step = 1;
}
else
- step = maybe_software_singlestep (gdbarch);
+ step = !insert_single_step_breakpoints (gdbarch);
}
}
@@ -1277,7 +1277,7 @@ record_full_wait_1 (struct target_ops *ops,
};
reinit_frame_cache ();
- step = maybe_software_singlestep (gdbarch);
+ step = !insert_single_step_breakpoints (gdbarch);
}
if (record_debug)