Call target_can_do_single_step from maybe_software_singlestep

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.

Following review comments from Andrew, this version changes
record-full to use maybe_software_singlestep, and then combines
maybe_software_singlestep with insert_single_step_breakpoint.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28440

diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 08526d8e39ddb624e5291689279cdb26496d9209..2f034afcd8047109d89b384b506063cdb97d15c6 100644 (file)
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -960,11 +960,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/breakpoint.c b/gdb/breakpoint.c
index d704ad1e3e2001c2fedab92ec7d46756ad1db7c6..f795d7b9931359d07bc08abdd082604a7b41e59c 100644 (file)
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -13998,11 +13998,26 @@ insert_single_step_breakpoint (struct gdbarch *gdbarch,
   update_global_location_list (UGLL_INSERT);
 }
 
-/* Insert single step breakpoints according to the current state.  */
+/* Try to setup for software single stepping.  Return true if
+   target_resume() should use hardware single step.
 
-int
-insert_single_step_breakpoints (struct gdbarch *gdbarch)
+   GDBARCH is the current gdbarch.  */
+
+bool
+maybe_software_singlestep (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;
 
@@ -14016,10 +14031,10 @@ insert_single_step_breakpoints (struct gdbarch *gdbarch)
       for (CORE_ADDR pc : next_pcs)
        insert_single_step_breakpoint (gdbarch, aspace, pc);
 
-      return 1;
+      return false;
     }
   else
-    return 0;
+    return true;
 }
 
 /* See breakpoint.h.  */

diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 93411122e9079925d4c828124a9624817a8f82c2..e9201bc63cdfd3f0234f8eb62e8cb55157d66134 100644 (file)
--- 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);
 
-/* 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 *);
+/* 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 *);
 
 /* 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 281b97b7aa8ce00a350e18fc83d1b92e8da8c5b4..1e5108135afa226204d972821cb24cdf378e8f95 100644 (file)
--- a/gdb/gdbarch-gen.h
+++ b/gdb/gdbarch-gen.h
@@ -776,16 +776,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 91c867e69bfb0e1b5023771840b24965e2720355..55df10266657006f56e26552eee8c1a4c852f465 100644 (file)
--- a/gdb/gdbarch_components.py
+++ b/gdb/gdbarch_components.py
@@ -1378,16 +1378,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 9d3e1b7782e4ddf84089156e910f77c6396586a4..e0e9ffa4caf82e790ff983e744280e57bad1ac45 100644 (file)
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -93,8 +93,6 @@ 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);
@@ -2359,23 +2357,6 @@ 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 7e3da277998e36126a4e3903084515f15bb22be6..7d4ef87fe76844fa344ed2b57b3537f3d35a413b 100644 (file)
--- 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 = !insert_single_step_breakpoints (gdbarch);
+               step = maybe_software_singlestep (gdbarch);
            }
        }
 
@@ -1277,7 +1277,7 @@ record_full_wait_1 (struct target_ops *ops,
                            };
 
                          reinit_frame_cache ();
-                         step = !insert_single_step_breakpoints (gdbarch);
+                         step = maybe_software_singlestep (gdbarch);
                        }
 
                      if (record_debug)