[2/3] gdb: add target displaced stepping support

Message ID 20250114191818.2393132-3-simon.marchi@polymtl.ca
State New
Headers
Series Add displaced stepping support for AMD GPUs |

Checks

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

Commit Message

Simon Marchi Jan. 14, 2025, 7:16 p.m. UTC
  From: Simon Marchi <simon.marchi@efficios.com>

The amd-dbgapi library, used in the AMD GPU port, has the capability to
prepare and cleanup displaced step operations.  In order to use it, add
the following target_ops methods:

 - supports_displaced_step
 - displaced_step_prepare
 - displaced_step_finish
 - displaced_step_restore_all_in_ptid

Prior to this patch, displaced stepping preparation and cleanup is done
solely by gdbarches.  Update infrun to use these new target methods
instead of gdbarch hooks.  To keep the behavior for other architectures
unchanged, make the default implementations of the new target_ops method
forward to the thread's gdbarch.

displaced_step_restore_all_in_ptid won't be needed for the AMD GPU port,
but was added for completeness.  It would be weird for infrun displaced
stepping code to call target methods except for that one thing where it
calls a gdbarch method.

Since this patch only adds infrastructure, no behavior change is
expected.

Change-Id: I07c68dddb5759a55cd137a711d2679eedc0d9285
---
 gdb/displaced-stepping.c   |  43 +++++++++++++++
 gdb/displaced-stepping.h   |  67 +++++++++++++++++++++++
 gdb/infrun.c               |  42 ++++++---------
 gdb/target-debug.h         |  18 +++++++
 gdb/target-delegates-gen.c | 108 +++++++++++++++++++++++++++++++++++++
 gdb/target.h               |  19 +++++++
 6 files changed, 270 insertions(+), 27 deletions(-)
  

Patch

diff --git a/gdb/displaced-stepping.c b/gdb/displaced-stepping.c
index 7869ebae7714..ff7f91011082 100644
--- a/gdb/displaced-stepping.c
+++ b/gdb/displaced-stepping.c
@@ -328,3 +328,46 @@  When non-zero, displaced stepping specific debugging is enabled."),
 			    show_debug_displaced,
 			    &setdebuglist, &showdebuglist);
 }
+
+/* See displaced-stepping.h.  */
+
+bool
+default_supports_displaced_step (target_ops *target, thread_info *thread)
+{
+  /* Only check for the presence of `prepare`.  The gdbarch verification ensures
+     that if `prepare` is provided, so is `finish`.  */
+  gdbarch *arch = get_thread_regcache (thread)->arch ();
+  return gdbarch_displaced_step_prepare_p (arch);
+}
+
+/* See displaced-stepping.h.  */
+
+displaced_step_prepare_status
+default_displaced_step_prepare (target_ops *target, thread_info *thread,
+				CORE_ADDR &displaced_pc)
+{
+  gdbarch *arch = get_thread_regcache (thread)->arch ();
+  return gdbarch_displaced_step_prepare (arch, thread, displaced_pc);
+}
+
+/* See displaced-stepping.h.  */
+
+displaced_step_finish_status
+default_displaced_step_finish (target_ops *target,
+			       thread_info *thread,
+			       const target_waitstatus &status)
+{
+  gdbarch *arch = thread->displaced_step_state.get_original_gdbarch ();
+  return gdbarch_displaced_step_finish (arch, thread, status);
+}
+
+/* See displaced-stepping.h.  */
+
+void
+default_displaced_step_restore_all_in_ptid (target_ops *target,
+					    inferior *parent_inf,
+					    ptid_t child_ptid)
+{
+  return gdbarch_displaced_step_restore_all_in_ptid (parent_inf->arch (),
+						     parent_inf, child_ptid);
+}
diff --git a/gdb/displaced-stepping.h b/gdb/displaced-stepping.h
index d7a537a58c92..7949e5dae13f 100644
--- a/gdb/displaced-stepping.h
+++ b/gdb/displaced-stepping.h
@@ -24,6 +24,8 @@ 
 #include "gdbsupport/byte-vector.h"
 
 struct gdbarch;
+struct inferior;
+struct target_ops;
 struct thread_info;
 
 /* True if we are debugging displaced stepping.  */
@@ -48,6 +50,26 @@  enum displaced_step_prepare_status
   DISPLACED_STEP_PREPARE_STATUS_UNAVAILABLE,
 };
 
+/* Return a string representation of STATUS.  */
+
+static inline const char *
+displaced_step_prepare_status_str (displaced_step_prepare_status status)
+{
+  switch (status)
+  {
+  case DISPLACED_STEP_PREPARE_STATUS_OK:
+    return "OK";
+
+  case DISPLACED_STEP_PREPARE_STATUS_CANT:
+    return "CANT";
+
+  case DISPLACED_STEP_PREPARE_STATUS_UNAVAILABLE:
+    return "UNAVAILABLE";
+  }
+
+  gdb_assert_not_reached ("invalid displaced_step_prepare_status value");
+}
+
 enum displaced_step_finish_status
 {
   /* Either the instruction was stepped and fixed up, or the specified thread
@@ -59,6 +81,23 @@  enum displaced_step_finish_status
   DISPLACED_STEP_FINISH_STATUS_NOT_EXECUTED,
 };
 
+/* Return a string representation of STATUS.  */
+
+static inline const char *
+displaced_step_finish_status_str (displaced_step_finish_status status)
+{
+  switch (status)
+  {
+  case DISPLACED_STEP_FINISH_STATUS_OK:
+    return "OK";
+
+  case DISPLACED_STEP_FINISH_STATUS_NOT_EXECUTED:
+    return "NOT_EXECUTED";
+  }
+
+  gdb_assert_not_reached ("invalid displaced_step_finish_status value");
+}
+
 /* Data returned by a gdbarch displaced_step_copy_insn method, to be passed to
    the matching displaced_step_fixup method.  */
 
@@ -207,4 +246,32 @@  struct displaced_step_buffers
   std::vector<displaced_step_buffer> m_buffers;
 };
 
+/* Default implemention of target_ops::supports_displaced_step.
+
+   Forwards the call to the architecture of THREAD.  */
+
+bool default_supports_displaced_step (target_ops *target, thread_info *thread);
+
+/* Default implementation of target_ops::displaced_step_prepare.
+
+   Forwards the call to the architecture of THREAD.  */
+
+displaced_step_prepare_status default_displaced_step_prepare
+  (target_ops *target, thread_info *thread, CORE_ADDR &displaced_pc);
+
+/* Default implementation of target_ops::displaced_step_finish.
+
+   Forwards the call to the architecture of THREAD.  */
+
+displaced_step_finish_status default_displaced_step_finish
+  (target_ops *target, thread_info *thread, const target_waitstatus &status);
+
+/* Default implementation of target_ops::displaced_step_restore_all_in_ptid.
+
+   Forwards the call to the architecture of PARENT_INF.  */
+
+void default_displaced_step_restore_all_in_ptid (target_ops *target,
+						 inferior *parent_inf,
+						 ptid_t child_ptid);
+
 #endif /* GDB_DISPLACED_STEPPING_H */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 4687ee6edb39..79fa31e5ee24 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1699,15 +1699,12 @@  show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
 		  "to step over breakpoints is %s.\n"), value);
 }
 
-/* Return true if the gdbarch implements the required methods to use
-   displaced stepping.  */
+/* Return true if the target behind THREAD supports displaced stepping.  */
 
 static bool
-gdbarch_supports_displaced_stepping (gdbarch *arch)
+target_supports_displaced_stepping (thread_info *thread)
 {
-  /* Only check for the presence of `prepare`.  The gdbarch verification ensures
-     that if `prepare` is provided, so is `finish`.  */
-  return gdbarch_displaced_step_prepare_p (arch);
+  return thread->inf->top_target ()->supports_displaced_step (thread);
 }
 
 /* Return non-zero if displaced stepping can/should be used to step
@@ -1726,11 +1723,8 @@  use_displaced_stepping (thread_info *tp)
       && !target_is_non_stop_p ())
     return false;
 
-  gdbarch *gdbarch = get_thread_regcache (tp)->arch ();
-
-  /* If the architecture doesn't implement displaced stepping, don't use
-     it.  */
-  if (!gdbarch_supports_displaced_stepping (gdbarch))
+  /* If the target doesn't support displaced stepping, don't use it.  */
+  if (!target_supports_displaced_stepping (tp))
     return false;
 
   /* If recording, don't use displaced stepping.  */
@@ -1784,9 +1778,9 @@  displaced_step_prepare_throw (thread_info *tp)
   displaced_step_thread_state &disp_step_thread_state
     = tp->displaced_step_state;
 
-  /* We should never reach this function if the architecture does not
+  /* We should never reach this function if the target does not
      support displaced stepping.  */
-  gdb_assert (gdbarch_supports_displaced_stepping (gdbarch));
+  gdb_assert (target_supports_displaced_stepping (tp));
 
   /* Nor if the thread isn't meant to step over a breakpoint.  */
   gdb_assert (tp->control.trap_expected);
@@ -1847,8 +1841,8 @@  displaced_step_prepare_throw (thread_info *tp)
 				paddress (gdbarch, original_pc), dislen);
     }
 
-  displaced_step_prepare_status status
-    = gdbarch_displaced_step_prepare (gdbarch, tp, displaced_pc);
+  auto status
+    = tp->inf->top_target ()->displaced_step_prepare (tp, displaced_pc);
 
   if (status == DISPLACED_STEP_PREPARE_STATUS_CANT)
     {
@@ -2028,6 +2022,7 @@  displaced_step_finish (thread_info *event_thread,
 {
   /* Check whether the parent is displaced stepping.  */
   inferior *parent_inf = event_thread->inf;
+  target_ops *top_target = parent_inf->top_target ();
 
   /* If this was a fork/vfork/clone, this event indicates that the
      displaced stepping of the syscall instruction has been done, so
@@ -2044,15 +2039,10 @@  displaced_step_finish (thread_info *event_thread,
      gdbarch_displaced_step_restore_all_in_ptid.  This is not enforced
      during gdbarch validation to support architectures which support
      displaced stepping but not forks.  */
-  if (event_status.kind () == TARGET_WAITKIND_FORKED)
-    {
-      struct regcache *parent_regcache = get_thread_regcache (event_thread);
-      struct gdbarch *gdbarch = parent_regcache->arch ();
-
-      if (gdbarch_supports_displaced_stepping (gdbarch))
-	gdbarch_displaced_step_restore_all_in_ptid
-	  (gdbarch, parent_inf, event_status.child_ptid ());
-    }
+  if (event_status.kind () == TARGET_WAITKIND_FORKED
+      && target_supports_displaced_stepping (event_thread))
+    top_target->displaced_step_restore_all_in_ptid
+      (parent_inf, event_status.child_ptid ());
 
   displaced_step_thread_state *displaced = &event_thread->displaced_step_state;
 
@@ -2075,9 +2065,7 @@  displaced_step_finish (thread_info *event_thread,
 
   /* Do the fixup, and release the resources acquired to do the displaced
      step. */
-  displaced_step_finish_status status
-    = gdbarch_displaced_step_finish (displaced->get_original_gdbarch (),
-				     event_thread, event_status);
+  auto status = top_target->displaced_step_finish (event_thread, event_status);
 
   if (event_status.kind () == TARGET_WAITKIND_FORKED
       || event_status.kind () == TARGET_WAITKIND_VFORKED
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 7530cf06f762..aee7d17f706d 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -145,6 +145,10 @@  static std::string
 target_debug_print_CORE_ADDR_p (CORE_ADDR *p)
 { return core_addr_to_string (*p); }
 
+static std::string
+target_debug_print_CORE_ADDR_r (CORE_ADDR &p)
+{ return core_addr_to_string (p); }
+
 static std::string
 target_debug_print_int_p (int *p)
 { return plongest (*p); }
@@ -306,6 +310,10 @@  static std::string
 target_debug_print_target_waitstatus_p (struct target_waitstatus *status)
 { return status->to_string (); }
 
+static std::string
+target_debug_print_const_target_waitstatus_r (const target_waitstatus &status)
+{ return status.to_string (); }
+
 /* Functions that are used via TARGET_DEBUG_PRINTER.  */
 
 static std::string
@@ -379,4 +387,14 @@  target_debug_print_x86_xsave_layout (const x86_xsave_layout &layout)
 
   return s;
 }
+
+static std::string
+target_debug_print_displaced_step_finish_status (displaced_step_finish_status s)
+{ return displaced_step_finish_status_str (s); }
+
+static std::string
+target_debug_print_displaced_step_prepare_status
+  (displaced_step_prepare_status s)
+{ return displaced_step_prepare_status_str (s); }
+
 #endif /* GDB_TARGET_DEBUG_H */
diff --git a/gdb/target-delegates-gen.c b/gdb/target-delegates-gen.c
index dd20e1404c32..7d34f80ab2df 100644
--- a/gdb/target-delegates-gen.c
+++ b/gdb/target-delegates-gen.c
@@ -199,6 +199,10 @@  struct dummy_target : public target_ops
   bool store_memtags (CORE_ADDR arg0, size_t arg1, const gdb::byte_vector &arg2, int arg3) override;
   bool is_address_tagged (gdbarch *arg0, CORE_ADDR arg1) override;
   x86_xsave_layout fetch_x86_xsave_layout () override;
+  bool supports_displaced_step (thread_info *arg0) override;
+  displaced_step_prepare_status displaced_step_prepare (thread_info *arg0, CORE_ADDR &arg1) override;
+  displaced_step_finish_status displaced_step_finish (thread_info *arg0, const target_waitstatus &arg1) override;
+  void displaced_step_restore_all_in_ptid (inferior *arg0, ptid_t arg1) override;
 };
 
 struct debug_target : public target_ops
@@ -376,6 +380,10 @@  struct debug_target : public target_ops
   bool store_memtags (CORE_ADDR arg0, size_t arg1, const gdb::byte_vector &arg2, int arg3) override;
   bool is_address_tagged (gdbarch *arg0, CORE_ADDR arg1) override;
   x86_xsave_layout fetch_x86_xsave_layout () override;
+  bool supports_displaced_step (thread_info *arg0) override;
+  displaced_step_prepare_status displaced_step_prepare (thread_info *arg0, CORE_ADDR &arg1) override;
+  displaced_step_finish_status displaced_step_finish (thread_info *arg0, const target_waitstatus &arg1) override;
+  void displaced_step_restore_all_in_ptid (inferior *arg0, ptid_t arg1) override;
 };
 
 void
@@ -4411,3 +4419,103 @@  debug_target::fetch_x86_xsave_layout ()
 	      target_debug_print_x86_xsave_layout (result).c_str ());
   return result;
 }
+
+bool
+target_ops::supports_displaced_step (thread_info *arg0)
+{
+  return this->beneath ()->supports_displaced_step (arg0);
+}
+
+bool
+dummy_target::supports_displaced_step (thread_info *arg0)
+{
+  return default_supports_displaced_step (this, arg0);
+}
+
+bool
+debug_target::supports_displaced_step (thread_info *arg0)
+{
+  target_debug_printf_nofunc ("-> %s->supports_displaced_step (...)", this->beneath ()->shortname ());
+  bool result
+    = this->beneath ()->supports_displaced_step (arg0);
+  target_debug_printf_nofunc ("<- %s->supports_displaced_step (%s) = %s",
+	      this->beneath ()->shortname (),
+	      target_debug_print_thread_info_p (arg0).c_str (),
+	      target_debug_print_bool (result).c_str ());
+  return result;
+}
+
+displaced_step_prepare_status
+target_ops::displaced_step_prepare (thread_info *arg0, CORE_ADDR &arg1)
+{
+  return this->beneath ()->displaced_step_prepare (arg0, arg1);
+}
+
+displaced_step_prepare_status
+dummy_target::displaced_step_prepare (thread_info *arg0, CORE_ADDR &arg1)
+{
+  return default_displaced_step_prepare (this, arg0, arg1);
+}
+
+displaced_step_prepare_status
+debug_target::displaced_step_prepare (thread_info *arg0, CORE_ADDR &arg1)
+{
+  target_debug_printf_nofunc ("-> %s->displaced_step_prepare (...)", this->beneath ()->shortname ());
+  displaced_step_prepare_status result
+    = this->beneath ()->displaced_step_prepare (arg0, arg1);
+  target_debug_printf_nofunc ("<- %s->displaced_step_prepare (%s, %s) = %s",
+	      this->beneath ()->shortname (),
+	      target_debug_print_thread_info_p (arg0).c_str (),
+	      target_debug_print_CORE_ADDR_r (arg1).c_str (),
+	      target_debug_print_displaced_step_prepare_status (result).c_str ());
+  return result;
+}
+
+displaced_step_finish_status
+target_ops::displaced_step_finish (thread_info *arg0, const target_waitstatus &arg1)
+{
+  return this->beneath ()->displaced_step_finish (arg0, arg1);
+}
+
+displaced_step_finish_status
+dummy_target::displaced_step_finish (thread_info *arg0, const target_waitstatus &arg1)
+{
+  return default_displaced_step_finish (this, arg0, arg1);
+}
+
+displaced_step_finish_status
+debug_target::displaced_step_finish (thread_info *arg0, const target_waitstatus &arg1)
+{
+  target_debug_printf_nofunc ("-> %s->displaced_step_finish (...)", this->beneath ()->shortname ());
+  displaced_step_finish_status result
+    = this->beneath ()->displaced_step_finish (arg0, arg1);
+  target_debug_printf_nofunc ("<- %s->displaced_step_finish (%s, %s) = %s",
+	      this->beneath ()->shortname (),
+	      target_debug_print_thread_info_p (arg0).c_str (),
+	      target_debug_print_const_target_waitstatus_r (arg1).c_str (),
+	      target_debug_print_displaced_step_finish_status (result).c_str ());
+  return result;
+}
+
+void
+target_ops::displaced_step_restore_all_in_ptid (inferior *arg0, ptid_t arg1)
+{
+  this->beneath ()->displaced_step_restore_all_in_ptid (arg0, arg1);
+}
+
+void
+dummy_target::displaced_step_restore_all_in_ptid (inferior *arg0, ptid_t arg1)
+{
+  default_displaced_step_restore_all_in_ptid (this, arg0, arg1);
+}
+
+void
+debug_target::displaced_step_restore_all_in_ptid (inferior *arg0, ptid_t arg1)
+{
+  target_debug_printf_nofunc ("-> %s->displaced_step_restore_all_in_ptid (...)", this->beneath ()->shortname ());
+  this->beneath ()->displaced_step_restore_all_in_ptid (arg0, arg1);
+  target_debug_printf_nofunc ("<- %s->displaced_step_restore_all_in_ptid (%s, %s)",
+	      this->beneath ()->shortname (),
+	      target_debug_print_inferior_p (arg0).c_str (),
+	      target_debug_print_ptid_t (arg1).c_str ());
+}
diff --git a/gdb/target.h b/gdb/target.h
index 5b45f152b51f..0729e32b1495 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1376,6 +1376,25 @@  struct target_ops
     /* Return the x86 XSAVE extended state area layout.  */
     virtual x86_xsave_layout fetch_x86_xsave_layout ()
       TARGET_DEFAULT_RETURN (x86_xsave_layout ());
+
+    /* Return true if the target supports displaced stepping for THREAD.  */
+    virtual bool supports_displaced_step (thread_info *thread)
+      TARGET_DEFAULT_FUNC (default_supports_displaced_step);
+
+    /* See documentation of gdbarch_displaced_step_prepare.  */
+    virtual displaced_step_prepare_status displaced_step_prepare (thread_info *thread,
+								  CORE_ADDR &displaced_pc)
+      TARGET_DEFAULT_FUNC (default_displaced_step_prepare);
+
+    /* See documentation of gdbarch_displaced_step_finish.  */
+    virtual displaced_step_finish_status displaced_step_finish
+      (thread_info *thread, const target_waitstatus &status)
+      TARGET_DEFAULT_FUNC (default_displaced_step_finish);
+
+    /* See documentation of gdbarch_displaced_step_restore_all_in_ptid.  */
+    virtual void displaced_step_restore_all_in_ptid (inferior *parent_inf,
+						     ptid_t child_ptid)
+      TARGET_DEFAULT_FUNC (default_displaced_step_restore_all_in_ptid);
   };
 
 /* Deleter for std::unique_ptr.  See comments in