From 9822cb57f7570a3b2c8fd95ec2f7b9f9890e30a4 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 2 Mar 2020 15:47:04 -0500 Subject: [PATCH] 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. --- gdb/ChangeLog | 8 ++++++++ gdb/infrun.c | 55 +++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 50 insertions(+), 13 deletions(-) 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 + + * 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 * 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); -- 2.34.1