public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@adacore.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 1/2] Call target_can_do_single_step from maybe_software_singlestep
Date: Mon, 12 Jun 2023 13:21:36 -0600	[thread overview]
Message-ID: <20230612-sw-single-step-v1-1-c06d648e121b@adacore.com> (raw)
In-Reply-To: <20230612-sw-single-step-v1-0-c06d648e121b@adacore.com>

When the PikeOS osabi sniffer was added, Pedro suggested that a target
could omit stepping from its vCont? reply packet to tell gdb that
software single-step must be used:

https://sourceware.org/legacy-ml/gdb-patches/2018-09/msg00312.html

This patch implements this idea by moving the call to
target_can_do_single_step into maybe_software_singlestep.

I've also removed some FIXME comments from gdbarch_components.py, and
slightly updated the documentation for gdbarch_software_single_step.
I think these comments are somewhat obsolete now that
target_can_do_single_step exists -- the current approach isn't exactly
what the comments intended, but on the other hand, it exists and
works.
---
 gdb/arm-linux-tdep.c      |  5 -----
 gdb/gdbarch-gen.h         | 12 +++---------
 gdb/gdbarch_components.py | 12 +++---------
 gdb/infrun.c              | 12 +++++++++---
 4 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index dfa816990ff..f4da996728b 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -923,11 +923,6 @@ 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/gdbarch-gen.h b/gdb/gdbarch-gen.h
index 101b1b73636..1d86879f00a 100644
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -721,16 +721,10 @@ 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);
 
-/* 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.
+/* 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: 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 23e5789327c..7f2380ae0a7 100644
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -1301,16 +1301,10 @@ For a non-zero value, this represents the number of bytes of memory per tag.
 
 Function(
     comment="""
-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.
+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: 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 58da1cef29e..d1dce33bd3a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2242,9 +2242,15 @@ 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);
+  if (execution_direction == EXEC_FORWARD)
+    {
+      if (target_can_do_single_step () == 1)
+	{
+	  /* The target definitely has hardware single step.  */
+	}
+      else if (gdbarch_software_single_step_p (gdbarch))
+	hw_step = !insert_single_step_breakpoints (gdbarch);
+    }
 
   return hw_step;
 }

-- 
2.40.1


  reply	other threads:[~2023-06-12 19:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-12 19:21 [PATCH 0/2] " Tom Tromey
2023-06-12 19:21 ` Tom Tromey [this message]
2023-11-16 10:12   ` [PATCH 1/2] " Andrew Burgess
2023-06-12 19:21 ` [PATCH 2/2] Disabling hardware single step in gdbserver Tom Tromey
2023-11-16 10:14   ` Andrew Burgess
2023-09-07 17:59 ` [PATCH 0/2] Call target_can_do_single_step from maybe_software_singlestep Tom Tromey
2023-11-15 19:01   ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230612-sw-single-step-v1-1-c06d648e121b@adacore.com \
    --to=tromey@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).