From patchwork Thu Feb 27 22:44:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 38343 Received: (qmail 57943 invoked by alias); 27 Feb 2020 22:44:32 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 57927 invoked by uid 89); 27 Feb 2020 22:44:32 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW, SPF_SOFTFAIL autolearn=ham version=3.3.1 spammy=Nor, dates, technique, UD:infrun.c X-HELO: barracuda.ebox.ca Received: from barracuda.ebox.ca (HELO barracuda.ebox.ca) (96.127.255.19) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 27 Feb 2020 22:44:29 +0000 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id N4NUD6Xcx5yzrdhM (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 27 Feb 2020 17:44:27 -0500 (EST) Received: from simark.lan (unknown [192.222.164.54]) by smtp.ebox.ca (Postfix) with ESMTP id 9D65C441B21; Thu, 27 Feb 2020 17:44:26 -0500 (EST) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH] Small clean up of use_displaced_stepping Date: Thu, 27 Feb 2020 17:44:24 -0500 Message-Id: <20200227224424.270604-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 X-IsSubscribed: yes This function returns the result of a quite big condition. I think it would be more redeable 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/infrun.c | 52 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index d9a6f7335194..bb1dae638666 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,50 @@ 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) +{ + 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, 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 +1676,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);