public inbox for gdb-testers@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] Small clean up of use_displaced_stepping
@ 2020-03-14  0:55 gdb-buildbot
  2020-03-14  0:55 ` Failures on Fedora-i686, branch master gdb-buildbot
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: gdb-buildbot @ 2020-03-14  0:55 UTC (permalink / raw)
  To: gdb-testers

*** TEST RESULTS FOR COMMIT 9822cb57f7570a3b2c8fd95ec2f7b9f9890e30a4 ***

commit 9822cb57f7570a3b2c8fd95ec2f7b9f9890e30a4
Author:     Simon Marchi <simon.marchi@polymtl.ca>
AuthorDate: Mon Mar 2 15:47:04 2020 -0500
Commit:     Simon Marchi <simon.marchi@efficios.com>
CommitDate: Mon Mar 2 15:57:15 2020 -0500

    Small clean up of use_displaced_stepping
    
    This function returns the result of a quite big condition.  I think it
    would be more readeable if it was broken up in smaller pieces and
    commented.  This is what this patch does.
    
    I also introduced gdbarch_supports_displaced_stepping, since it shows
    the intent better than checking for gdbarch_displaced_step_copy_insn_p.
    I also used that new function in displaced_step_prepare_throw.
    
    I also updated the comment on top of can_use_displaced_stepping, which
    seemed a bit outdated with respect to non-stop.  The comment likely
    dates from before it was possible to have targets that always operate
    non-stop under the hood, even when the user-visible mode is all-stop.
    
    No functional changes intended.
    
    gdb/ChangeLog:
    
            * infrun.c (gdbarch_supports_displaced_stepping): New.
            (use_displaced_stepping): Break up conditions in smaller pieces.
            Use gdbarch_supports_displaced_stepping.
            (displaced_step_prepare_throw): Use
            gdbarch_supports_displaced_stepping.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e303a81673..89eff2931e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2020-03-02  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* infrun.c (gdbarch_supports_displaced_stepping): New.
+	(use_displaced_stepping): Break up conditions in smaller pieces.
+	Use gdbarch_supports_displaced_stepping.
+	(displaced_step_prepare_throw): Use
+	gdbarch_supports_displaced_stepping.
+
 2020-03-02  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* NEWS: Mention new behaviour of the history filename.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 0f2b9a5412..b8c1bbc329 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1556,8 +1556,7 @@ infrun_inferior_exit (struct inferior *inf)
    doesn't support it, GDB will instead use the traditional
    hold-and-step approach.  If AUTO (which is the default), GDB will
    decide which technique to use to step over breakpoints depending on
-   which of all-stop or non-stop mode is active --- displaced stepping
-   in non-stop mode; hold-and-step in all-stop mode.  */
+   whether the target works in a non-stop way (see use_displaced_stepping).  */
 
 static enum auto_boolean can_use_displaced_stepping = AUTO_BOOLEAN_AUTO;
 
@@ -1577,23 +1576,53 @@ 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.  */
+
+static bool
+gdbarch_supports_displaced_stepping (gdbarch *arch)
+{
+  /* Only check for the presence of step_copy_insn.  Other required methods
+     are checked by the gdbarch validation.  */
+  return gdbarch_displaced_step_copy_insn_p (arch);
+}
+
 /* Return non-zero if displaced stepping can/should be used to step
    over breakpoints of thread TP.  */
 
-static int
-use_displaced_stepping (struct thread_info *tp)
+static bool
+use_displaced_stepping (thread_info *tp)
 {
-  struct regcache *regcache = get_thread_regcache (tp);
-  struct gdbarch *gdbarch = regcache->arch ();
+  /* If the user disabled it explicitly, don't use displaced stepping.  */
+  if (can_use_displaced_stepping == AUTO_BOOLEAN_FALSE)
+    return false;
+
+  /* If "auto", only use displaced stepping if the target operates in a non-stop
+     way.  */
+  if (can_use_displaced_stepping == AUTO_BOOLEAN_AUTO
+      && !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))
+    return false;
+
+  /* If recording, don't use displaced stepping.  */
+  if (find_record_target () != nullptr)
+    return false;
+
   displaced_step_inferior_state *displaced_state
     = get_displaced_stepping_state (tp->inf);
 
-  return (((can_use_displaced_stepping == AUTO_BOOLEAN_AUTO
-	    && target_is_non_stop_p ())
-	   || can_use_displaced_stepping == AUTO_BOOLEAN_TRUE)
-	  && gdbarch_displaced_step_copy_insn_p (gdbarch)
-	  && find_record_target () == NULL
-	  && !displaced_state->failed_before);
+  /* If displaced stepping failed before for this inferior, don't bother trying
+     again.  */
+  if (displaced_state->failed_before)
+    return false;
+
+  return true;
 }
 
 /* Simple function wrapper around displaced_step_inferior_state::reset.  */
@@ -1650,7 +1679,7 @@ displaced_step_prepare_throw (thread_info *tp)
 
   /* We should never reach this function if the architecture does not
      support displaced stepping.  */
-  gdb_assert (gdbarch_displaced_step_copy_insn_p (gdbarch));
+  gdb_assert (gdbarch_supports_displaced_stepping (gdbarch));
 
   /* Nor if the thread isn't meant to step over a breakpoint.  */
   gdb_assert (tp->control.trap_expected);


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-03-16 17:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-14  0:55 [binutils-gdb] Small clean up of use_displaced_stepping gdb-buildbot
2020-03-14  0:55 ` Failures on Fedora-i686, branch master gdb-buildbot
2020-03-14  0:58 ` Failures on Fedora-x86_64-cc-with-index, " gdb-buildbot
2020-03-14  1:30 ` Failures on Fedora-x86_64-m64, " gdb-buildbot
2020-03-14  1:38 ` Failures on Fedora-x86_64-m32, " gdb-buildbot
2020-03-14  2:08 ` Failures on Fedora-x86_64-native-extended-gdbserver-m32, " gdb-buildbot
2020-03-14  2:29 ` Failures on Fedora-x86_64-native-extended-gdbserver-m64, " gdb-buildbot
2020-03-15 14:55 ` Failures on Ubuntu-Aarch64-m64, " gdb-buildbot
2020-03-15 15:12 ` Failures on Ubuntu-Aarch64-native-extended-gdbserver-m64, " gdb-buildbot
2020-03-15 19:31 ` Failures on Fedora-x86_64-native-gdbserver-m64, " gdb-buildbot
2020-03-16 17:35 ` Failures on Fedora-x86_64-native-gdbserver-m32, " gdb-buildbot

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).