diff mbox series

[RFC,1/4,gdb/tdep] Add dummy amd64_software_single_step

Message ID 20231129203326.11952-1-tdevries@suse.de
State New
Headers
Series [RFC,1/4,gdb/tdep] Add dummy amd64_software_single_step |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Tom de Vries Nov. 29, 2023, 8:33 p.m. UTC 
  The documentation of gdbarch_software_single_step states:
...
Return a vector of addresses on which the software single step
breakpoints should be inserted.  NULL means software single step is
not used.
...

Add a dummy amd64_software_single_step, returning an empty vector.

As per the documentation, this shouldn't have any effect.  However, there are
regressions.

The reason for these regressions is the use of gdbarch_software_single_step_p:
...
bool
gdbarch_software_single_step_p (struct gdbarch *gdbarch)
{
  gdb_assert (gdbarch != NULL);
  return gdbarch->software_single_step != NULL;
}
...
which simply tests for the presence of the hook, and which returns true
instead of false after adding amd64_software_single_step.

Fix this by introducing a software_single_step_p which follows the logic of
insert_single_step_breakpoints, and using this instead of
gdbarch_software_single_step_p.

This leaves gdbarch_software_single_step_p used only in:
- software_single_step_p, and
- of gdbarch_displaced_step_hw_singlestep.

In the latter case, software_single_step_p doesn't work well, so instead
use a amd64_displaced_step_hw_singlestep always returning true.

Tested on x86_64-linux.
---
 gdb/amd64-tdep.c  | 15 +++++++++++++++
 gdb/breakpoint.c  | 15 +++++++++++++++
 gdb/breakpoint.h  |  2 ++
 gdb/infrun.c      |  2 +-
 gdb/linux-nat.c   |  4 ++--
 gdb/record-full.c |  4 ++--
 6 files changed, 37 insertions(+), 5 deletions(-)


base-commit: 35efddd5a12bbe514ff3870ec67a0357774fbe04
diff mbox series

Patch

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index e6feee677b3..85721bbadf4 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -3158,6 +3158,18 @@  amd64_in_indirect_branch_thunk (struct gdbarch *gdbarch, CORE_ADDR pc)
 				       AMD64_RIP_REGNUM);
 }
 
+static std::vector<CORE_ADDR>
+amd64_software_single_step (struct regcache *regcache)
+{
+  return {};
+}
+
+static bool
+amd64_displaced_step_hw_singlestep (gdbarch *)
+{
+  return true;
+}
+
 void
 amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
 		const target_desc *default_tdesc)
@@ -3329,6 +3341,9 @@  amd64_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch,
 					amd64_in_indirect_branch_thunk);
 
   register_amd64_ravenscar_ops (gdbarch);
+
+  set_gdbarch_software_single_step (gdbarch, amd64_software_single_step);
+  set_gdbarch_displaced_step_hw_singlestep (gdbarch, amd64_displaced_step_hw_singlestep);
 }
 
 /* Initialize ARCH for x86-64, no osabi.  */
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 699919e32b3..60e795e1849 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13945,6 +13945,21 @@  insert_single_step_breakpoint (struct gdbarch *gdbarch,
   update_global_location_list (UGLL_INSERT);
 }
 
+bool
+software_single_step_p (struct gdbarch *gdbarch)
+{
+  if (!gdbarch_software_single_step_p (gdbarch))
+    return false;
+
+  regcache *regcache = get_thread_regcache (inferior_thread ());
+  std::vector<CORE_ADDR> next_pcs;
+
+  next_pcs = gdbarch_software_single_step (gdbarch, regcache);
+
+  return !next_pcs.empty ();
+}
+
+
 /* Insert single step breakpoints according to the current state.  */
 
 int
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index feb798224c0..20b1eb23b83 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1815,6 +1815,8 @@  extern void insert_single_step_breakpoint (struct gdbarch *,
 					   const address_space *,
 					   CORE_ADDR);
 
+extern bool software_single_step_p (struct gdbarch *gdbarch);
+
 /* Insert all software single step breakpoints for the current frame.
    Return true if any software single step breakpoints are inserted,
    otherwise, return false.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 45c1b4a79bb..f7f76737c40 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2366,7 +2366,7 @@  maybe_software_singlestep (struct gdbarch *gdbarch)
   bool hw_step = true;
 
   if (execution_direction == EXEC_FORWARD
-      && gdbarch_software_single_step_p (gdbarch))
+      && software_single_step_p (gdbarch))
     hw_step = !insert_single_step_breakpoints (gdbarch);
 
   return hw_step;
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 737e0f7bda6..9954750fc84 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -490,8 +490,8 @@  linux_nat_target::follow_fork (inferior *child_inf, ptid_t child_ptid,
 	  /* Note that we consult the parent's architecture instead of
 	     the child's because there's no inferior for the child at
 	     this point.  */
-	  if (!gdbarch_software_single_step_p (target_thread_architecture
-					       (parent_ptid)))
+	  if (!software_single_step_p (target_thread_architecture
+				       (parent_ptid)))
 	    {
 	      int status;
 
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 2fc9e433ca8..023e9cb7b26 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1074,7 +1074,7 @@  record_full_target::resume (ptid_t ptid, int step, enum gdb_signal signal)
       if (!step)
 	{
 	  /* This is not hard single step.  */
-	  if (!gdbarch_software_single_step_p (gdbarch))
+	  if (!software_single_step_p (gdbarch))
 	    {
 	      /* This is a normal continue.  */
 	      step = 1;
@@ -1249,7 +1249,7 @@  record_full_wait_1 (struct target_ops *ops,
 		      process_stratum_target *proc_target
 			= current_inferior ()->process_target ();
 
-		      if (gdbarch_software_single_step_p (gdbarch))
+		      if (software_single_step_p (gdbarch))
 			{
 			  /* Try to insert the software single step breakpoint.
 			     If insert success, set step to 0.  */