From patchwork Thu Feb 13 23:04:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 38058 Received: (qmail 28057 invoked by alias); 13 Feb 2020 23:04:50 -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 28043 invoked by uid 89); 13 Feb 2020 23:04:49 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-23.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, SPF_PASS autolearn=ham version=3.3.1 spammy=UD:osci.io, stepped, gnutoolchaingerritosciio, thr X-HELO: mail.efficios.com Received: from mail.efficios.com (HELO mail.efficios.com) (167.114.26.124) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 13 Feb 2020 23:04:47 +0000 Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 39495262943; Thu, 13 Feb 2020 18:04:45 -0500 (EST) Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id cGAN_cMBaDC6; Thu, 13 Feb 2020 18:04:44 -0500 (EST) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id B626E26275A; Thu, 13 Feb 2020 18:04:44 -0500 (EST) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com B626E26275A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficios.com; s=default; t=1581635084; bh=o76SA2eUul9SrSB1/tr64fg/Q0/RaqScymcW3esNcis=; h=From:To:Date:Message-Id:MIME-Version; b=ceCCnwbZ/F7mE35l5xc+6GMqCxcOaeyW6nhCc0OVuapN+F5znnV9PIsiV7dgS35wq oTOsy4hITMUYLyo4geuz/plpyLsk0gywGq/KVXVVcdScvbrrptWCG5sfukUtnZlxvc WPxVetY/8+TnjqonHEe7G7PqjbE4rHuFLL34OzP40fx12Q7onmUH0OkrB+yz13MQgg tzYQUpAhc6LPy8FMoUxqWKFS4ztlPGbC2VKQ+rDuwPG6UigUkyU4iEoWs2aPIaqU0h eEJAaXuaVXw78iSO/mk0CcGqn25Kz/4HgFS96NvOT5Vokyn9rRfSzXmi9VF/ivHMPU Z8vqdVDho9n0Q== Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id WP7iYjUR56WR; Thu, 13 Feb 2020 18:04:44 -0500 (EST) Received: from smarchi-efficios.internal.efficios.com (192-222-181-218.qc.cable.ebox.net [192.222.181.218]) by mail.efficios.com (Postfix) with ESMTPSA id 9097C262A70; Thu, 13 Feb 2020 18:04:44 -0500 (EST) From: Simon Marchi To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH v3] Pass thread_info pointer to various inferior control functions Date: Thu, 13 Feb 2020 18:04:28 -0500 Message-Id: <20200213230428.14476-1-simon.marchi@efficios.com> MIME-Version: 1.0 From: Simon Marchi [ Migrating this from Gerrit: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/321 ] I noticed that some functions in infcmd and infrun call each other and all call inferior_thread, while they could just get the thread_info pointer from their caller. That means less calls to inferior_thread, so less reliance on global state, since inferior_thread reads inferior_ptid. The paths I am unsure about are: - fetch_inferior_event calls... - step_command_fsm::should_stop calls... - prepare_one_step and - process_event_stop_test calls... - set_step_info Before this patch, prepare_one_step gets the thread pointer using inferior_thread. After this patch, it gets it from the execution_control_state structure in fetch_inferior_event. Are we sure that the thread from the execution_control_state structure is the same as the one inferior_thread would return? This code path is used when a thread completes a step, but the user had specified a step count (e.g. "step 5") so we decide to do one more step. It would be strange (and even a bug I suppose) if the thread in the ecs structure in fetch_inferior_event was not the same thread that is prepared to stepped by prepare_one_step. So I believe passing the ecs thread is fine. The same logic applies to process_event_stop_test calling set_step_info. gdb/ChangeLog: * infrun.h: Forward-declare thread_info. (set_step_info): Add thread_info parameter, add doc. * infrun.c (set_step_info): Add thread_info parameter, move doc to header. * infrun.c (process_event_stop_test): Pass thread to set_step_info call. * infcmd.c (set_step_frame): Add thread_info pointer, pass it to set_step_info. (prepare_one_step): Add thread_info parameter, pass it to set_step_frame and prepare_one_step (recursive) call. (step_1): Pass thread to prepare_one_step call. (step_command_fsm::should_stop): Pass thread to prepare_one_step. (until_next_fsm): Pass thread to set_step_frame call. (finish_command): Pass thread to set_step_info call. --- gdb/infcmd.c | 29 +++++++++++------------------ gdb/infrun.c | 10 +++++----- gdb/infrun.h | 5 ++++- 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 62890bde2a22..8b5fe5646759 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -914,18 +914,17 @@ continue_command (const char *args, int from_tty) continue_1 (all_threads_p); } -/* Record the starting point of a "step" or "next" command. */ +/* Record in TP the starting point of a "step" or "next" command. */ static void -set_step_frame (void) +set_step_frame (thread_info *tp) { frame_info *frame = get_current_frame (); symtab_and_line sal = find_frame_sal (frame); - set_step_info (frame, sal); + set_step_info (tp, frame, sal); CORE_ADDR pc = get_frame_pc (frame); - thread_info *tp = inferior_thread (); tp->control.step_start_function = find_pc_function (pc); } @@ -1002,7 +1001,7 @@ step_command_fsm_prepare (struct step_command_fsm *sm, thread->control.stepping_command = 1; } -static int prepare_one_step (struct step_command_fsm *sm); +static int prepare_one_step (thread_info *, struct step_command_fsm *sm); static void step_1 (int skip_subroutines, int single_inst, const char *count_string) @@ -1040,7 +1039,7 @@ step_1 (int skip_subroutines, int single_inst, const char *count_string) loop. Let the continuation figure out how many other steps we need to do, and handle them one at the time, through step_once. */ - if (!prepare_one_step (step_sm)) + if (!prepare_one_step (thr, step_sm)) proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); else { @@ -1070,7 +1069,7 @@ step_command_fsm::should_stop (struct thread_info *tp) /* There are more steps to make, and we did stop due to ending a stepping range. Do another step. */ if (--count > 0) - return prepare_one_step (this); + return prepare_one_step (tp, this); set_finished (); } @@ -1102,19 +1101,13 @@ step_command_fsm::do_async_reply_reason () resumed. */ static int -prepare_one_step (struct step_command_fsm *sm) +prepare_one_step (thread_info *tp, struct step_command_fsm *sm) { if (sm->count > 0) { struct frame_info *frame = get_current_frame (); - /* Don't assume THREAD is a valid thread id. It is set to -1 if - the longjmp breakpoint was not required. Use the - INFERIOR_PTID thread instead, which is the same thread when - THREAD is set. */ - struct thread_info *tp = inferior_thread (); - - set_step_frame (); + set_step_frame (tp); if (!sm->single_inst) { @@ -1146,7 +1139,7 @@ prepare_one_step (struct step_command_fsm *sm) || !function_name_is_marked_for_skip (fn, sal)) { sm->count--; - return prepare_one_step (sm); + return prepare_one_step (tp, sm); } } @@ -1488,7 +1481,7 @@ until_next_command (int from_tty) struct until_next_fsm *sm; clear_proceed_status (0); - set_step_frame (); + set_step_frame (tp); frame = get_current_frame (); @@ -1945,7 +1938,7 @@ finish_command (const char *arg, int from_tty) called by that frame. We don't use the magic "1" value for step_range_end, because then infrun will think this is nexti, and not step over the rest of this inlined function call. */ - set_step_info (frame, {}); + set_step_info (tp, frame, {}); tp->control.step_range_start = get_frame_pc (frame); tp->control.step_range_end = tp->control.step_range_start; tp->control.step_over_calls = STEP_OVER_ALL; diff --git a/gdb/infrun.c b/gdb/infrun.c index 3e846f8e6802..58c63b3f8411 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -4073,12 +4073,12 @@ fetch_inferior_event (void *client_data) printf_unfiltered (_("completed.\n")); } -/* Record the frame and location we're currently stepping through. */ +/* See infrun.h. */ + void -set_step_info (struct frame_info *frame, struct symtab_and_line sal) +set_step_info (thread_info *tp, struct frame_info *frame, + struct symtab_and_line sal) { - struct thread_info *tp = inferior_thread (); - tp->control.step_frame_id = get_frame_id (frame); tp->control.step_stack_frame_id = get_stack_frame_id (frame); @@ -7171,7 +7171,7 @@ process_event_stop_test (struct execution_control_state *ecs) ecs->event_thread->control.step_range_start = stop_pc_sal.pc; ecs->event_thread->control.step_range_end = stop_pc_sal.end; ecs->event_thread->control.may_range_step = 1; - set_step_info (frame, stop_pc_sal); + set_step_info (ecs->event_thread, frame, stop_pc_sal); if (debug_infrun) fprintf_unfiltered (gdb_stdlog, "infrun: keep going\n"); diff --git a/gdb/infrun.h b/gdb/infrun.h index 8040b28f0172..220bc770c2cc 100644 --- a/gdb/infrun.h +++ b/gdb/infrun.h @@ -26,6 +26,7 @@ struct frame_info; struct address_space; struct return_value_info; struct process_stratum_target; +struct thread_info; /* True if we are debugging run control. */ extern unsigned int debug_infrun; @@ -150,7 +151,9 @@ extern int thread_is_stepping_over_breakpoint (int thread); triggers a non-steppable watchpoint. */ extern int stepping_past_nonsteppable_watchpoint (void); -extern void set_step_info (struct frame_info *frame, +/* Record in TP the frame and location we're currently stepping through. */ +extern void set_step_info (thread_info *tp, + struct frame_info *frame, struct symtab_and_line sal); /* Several print_*_reason helper functions to print why the inferior