From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from outbound-ss-820.bluehost.com (outbound-ss-820.bluehost.com [69.89.24.241]) by sourceware.org (Postfix) with ESMTPS id 9AA9338451AE for ; Sat, 26 Nov 2022 01:53:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9AA9338451AE Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tromey.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tromey.com Received: from cmgw14.mail.unifiedlayer.com (unknown [10.0.90.129]) by progateway2.mail.pro1.eigbox.com (Postfix) with ESMTP id D3CEF100479FD for ; Sat, 26 Nov 2022 01:53:10 +0000 (UTC) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with ESMTP id ykNSohsA9hFiDykNSopHf1; Sat, 26 Nov 2022 01:53:10 +0000 X-Authority-Reason: nr=8 X-Authority-Analysis: v=2.4 cv=D6qCltdj c=1 sm=1 tr=0 ts=63817186 a=ApxJNpeYhEAb1aAlGBBbmA==:117 a=ApxJNpeYhEAb1aAlGBBbmA==:17 a=dLZJa+xiwSxG16/P+YVxDGlgEgI=:19 a=9xFQ1JgjjksA:10:nop_rcvd_month_year a=Qbun_eYptAEA:10:endurance_base64_authed_username_1 a=zstS-IiYAAAA:8 a=1PmWp5JYAAAA:8 a=lZ5pUbeOtyR5jdq-opIA:9 a=4G6NA9xxw8l3yy4pmD5M:22 a=YgU2nCY4GfN3Jg0JtKmw:22 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date:References :Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=LhCfi5PbJnnpE9VMaIKfXuGi0u9isGpmkNiNWPrxTiQ=; b=EtGXGPaGySYRXms7D80jv2VfdJ ogKwYor8mcZGNrN11h5AAjhPLCzfbRbUH7UUIQHq2ej/3rdEoJr45X+D/dDBykhm/Jx683p6f+v7j hfeBhHlYj6rg0kqVUJFGbMePr; Received: from 97-122-76-186.hlrn.qwest.net ([97.122.76.186]:33036 helo=prentzel) by box5379.bluehost.com with esmtpsa (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1oykNS-003mlY-7L; Fri, 25 Nov 2022 18:53:10 -0700 From: Tom Tromey To: Simon Marchi Cc: Tom Tromey , gdb-patches@sourceware.org Subject: Re: [PATCH] Remove reset_ecs References: <20221121175830.3585569-1-tom@tromey.com> <102b6335-e33d-ffc0-b7a5-fd79d4cf26af@simark.ca> X-Attribution: Tom Date: Fri, 25 Nov 2022 18:53:08 -0700 In-Reply-To: <102b6335-e33d-ffc0-b7a5-fd79d4cf26af@simark.ca> (Simon Marchi's message of "Mon, 21 Nov 2022 13:14:18 -0500") Message-ID: <87edtqbhx7.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - box5379.bluehost.com X-AntiAbuse: Original Domain - sourceware.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - tromey.com X-BWhitelist: no X-Source-IP: 97.122.76.186 X-Source-L: No X-Exim-ID: 1oykNS-003mlY-7L X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: 97-122-76-186.hlrn.qwest.net (prentzel) [97.122.76.186]:33036 X-Source-Auth: tom+tromey.com X-Email-Count: 2 X-Source-Cap: ZWx5bnJvYmk7ZWx5bnJvYmk7Ym94NTM3OS5ibHVlaG9zdC5jb20= X-Local-Domain: yes X-Spam-Status: No, score=-3028.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,JMQ_SPF_NEUTRAL,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Simon> I think we could get rid of the reset method as well, it's not Simon> used outside of the constructor. Makes sense. Here's v2. Tom commit ef52011b656e6bff7e2cc56f843412bec1eb4525 Author: Tom Tromey Date: Sun Nov 20 15:08:06 2022 -0700 Remove reset_ecs and execution_control_state::reset I noticed that execution_control_state has a 'reset' method, and there's also a 'reset_ecs' function that calls it. This patch cleans this area up a little by adding a parameter to the constructor and (a change Simon suggested) removing the reset method. Some extraneous variables are also removed, like: - struct execution_control_state ecss; - struct execution_control_state *ecs = &ecss; Here 'ecs' is never changed, so this patch removes it entirely in favor of just using the object everywhere. Regression tested on x86-64 Fedora 34. diff --git a/gdb/infrun.c b/gdb/infrun.c index 96346e1f25b..248b71a053b 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1854,55 +1854,33 @@ displaced_step_finish (thread_info *event_thread, enum gdb_signal signal) discarded between events. */ struct execution_control_state { - execution_control_state () + explicit execution_control_state (thread_info *thr = nullptr) + : ptid (thr == nullptr ? null_ptid : thr->ptid), + event_thread (thr), + ws (target_waitstatus ()) { - this->reset (); } - void reset () - { - this->target = nullptr; - this->ptid = null_ptid; - this->event_thread = nullptr; - ws = target_waitstatus (); - stop_func_filled_in = 0; - stop_func_start = 0; - stop_func_end = 0; - stop_func_name = nullptr; - wait_some_more = 0; - hit_singlestep_breakpoint = 0; - } - - process_stratum_target *target; + process_stratum_target *target = nullptr; ptid_t ptid; /* The thread that got the event, if this was a thread event; NULL otherwise. */ struct thread_info *event_thread; struct target_waitstatus ws; - int stop_func_filled_in; - CORE_ADDR stop_func_start; - CORE_ADDR stop_func_end; - const char *stop_func_name; - int wait_some_more; + int stop_func_filled_in = 0; + CORE_ADDR stop_func_start = 0; + CORE_ADDR stop_func_end = 0; + const char *stop_func_name = nullptr; + int wait_some_more = 0; /* True if the event thread hit the single-step breakpoint of another thread. Thus the event doesn't cause a stop, the thread needs to be single-stepped past the single-step breakpoint before we can switch back to the original stepping thread. */ - int hit_singlestep_breakpoint; + int hit_singlestep_breakpoint = 0; }; -/* Clear ECS and set it to point at TP. */ - -static void -reset_ecs (struct execution_control_state *ecs, struct thread_info *tp) -{ - ecs->reset (); - ecs->event_thread = tp; - ecs->ptid = tp->ptid; -} - static void keep_going_pass_signal (struct execution_control_state *ecs); static void prepare_to_wait (struct execution_control_state *ecs); static bool keep_going_stepped_thread (struct thread_info *tp); @@ -1955,8 +1933,6 @@ start_step_over (void) for (thread_info *tp : range) { - struct execution_control_state ecss; - struct execution_control_state *ecs = &ecss; step_over_what step_what; int must_be_in_line; @@ -2027,10 +2003,10 @@ start_step_over (void) continue; switch_to_thread (tp); - reset_ecs (ecs, tp); - keep_going_pass_signal (ecs); + execution_control_state ecs (tp); + keep_going_pass_signal (&ecs); - if (!ecs->wait_some_more) + if (!ecs.wait_some_more) error (_("Command aborted.")); /* If the thread's step over could not be initiated because no buffers @@ -3161,8 +3137,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) struct regcache *regcache; struct gdbarch *gdbarch; CORE_ADDR pc; - struct execution_control_state ecss; - struct execution_control_state *ecs = &ecss; /* If we're stopped at a fork/vfork, follow the branch set by the "set follow-fork-mode" command; otherwise, we'll just proceed @@ -3374,10 +3348,10 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) infrun_debug_printf ("resuming %s", tp->ptid.to_string ().c_str ()); - reset_ecs (ecs, tp); + execution_control_state ecs (tp); switch_to_thread (tp); - keep_going_pass_signal (ecs); - if (!ecs->wait_some_more) + keep_going_pass_signal (&ecs); + if (!ecs.wait_some_more) error (_("Command aborted.")); } } @@ -3390,10 +3364,10 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) && cur_thr->inf->thread_waiting_for_vfork_done != nullptr)) { /* The thread wasn't started, and isn't queued, run it now. */ - reset_ecs (ecs, cur_thr); + execution_control_state ecs (cur_thr); switch_to_thread (cur_thr); - keep_going_pass_signal (ecs); - if (!ecs->wait_some_more) + keep_going_pass_signal (&ecs); + if (!ecs.wait_some_more) error (_("Command aborted.")); } @@ -4001,8 +3975,7 @@ wait_for_inferior (inferior *inf) while (1) { - struct execution_control_state ecss; - struct execution_control_state *ecs = &ecss; + execution_control_state ecs; overlay_cache_invalid = 1; @@ -4012,16 +3985,16 @@ wait_for_inferior (inferior *inf) don't get any event. */ target_dcache_invalidate (); - ecs->ptid = do_target_wait_1 (inf, minus_one_ptid, &ecs->ws, 0); - ecs->target = inf->process_target (); + ecs.ptid = do_target_wait_1 (inf, minus_one_ptid, &ecs.ws, 0); + ecs.target = inf->process_target (); if (debug_infrun) - print_target_wait_results (minus_one_ptid, ecs->ptid, ecs->ws); + print_target_wait_results (minus_one_ptid, ecs.ptid, ecs.ws); /* Now figure out what to do with the result of the result. */ - handle_inferior_event (ecs); + handle_inferior_event (&ecs); - if (!ecs->wait_some_more) + if (!ecs.wait_some_more) break; } @@ -4147,8 +4120,7 @@ fetch_inferior_event () { INFRUN_SCOPED_DEBUG_ENTER_EXIT; - struct execution_control_state ecss; - struct execution_control_state *ecs = &ecss; + execution_control_state ecs; int cmd_done = 0; /* Events are always processed with the main UI as current UI. This @@ -4198,27 +4170,27 @@ fetch_inferior_event () the event. */ scoped_disable_commit_resumed disable_commit_resumed ("handling event"); - if (!do_target_wait (ecs, TARGET_WNOHANG)) + if (!do_target_wait (&ecs, TARGET_WNOHANG)) { infrun_debug_printf ("do_target_wait returned no event"); disable_commit_resumed.reset_and_commit (); return; } - gdb_assert (ecs->ws.kind () != TARGET_WAITKIND_IGNORE); + gdb_assert (ecs.ws.kind () != TARGET_WAITKIND_IGNORE); /* Switch to the target that generated the event, so we can do target calls. */ - switch_to_target_no_thread (ecs->target); + switch_to_target_no_thread (ecs.target); if (debug_infrun) - print_target_wait_results (minus_one_ptid, ecs->ptid, ecs->ws); + print_target_wait_results (minus_one_ptid, ecs.ptid, ecs.ws); /* If an error happens while handling the event, propagate GDB's knowledge of the executing state to the frontend/user running state. */ - ptid_t finish_ptid = !target_is_non_stop_p () ? minus_one_ptid : ecs->ptid; - scoped_finish_thread_state finish_state (ecs->target, finish_ptid); + ptid_t finish_ptid = !target_is_non_stop_p () ? minus_one_ptid : ecs.ptid; + scoped_finish_thread_state finish_state (ecs.target, finish_ptid); /* Get executed before scoped_restore_current_thread above to apply still for the thread which has thrown the exception. */ @@ -4228,13 +4200,13 @@ fetch_inferior_event () = make_scope_exit (delete_just_stopped_threads_infrun_breakpoints); /* Now figure out what to do with the result of the result. */ - handle_inferior_event (ecs); + handle_inferior_event (&ecs); - if (!ecs->wait_some_more) + if (!ecs.wait_some_more) { - struct inferior *inf = find_inferior_ptid (ecs->target, ecs->ptid); + struct inferior *inf = find_inferior_ptid (ecs.target, ecs.ptid); bool should_stop = true; - struct thread_info *thr = ecs->event_thread; + struct thread_info *thr = ecs.event_thread; delete_just_stopped_threads_infrun_breakpoints (); @@ -4243,7 +4215,7 @@ fetch_inferior_event () if (!should_stop) { - keep_going (ecs); + keep_going (&ecs); } else { @@ -4252,7 +4224,7 @@ fetch_inferior_event () stop_all_threads_if_all_stop_mode (); - clean_up_just_stopped_threads_fsms (ecs); + clean_up_just_stopped_threads_fsms (&ecs); if (thr != nullptr && thr->thread_fsm () != nullptr) should_notify_stop @@ -4281,7 +4253,7 @@ fetch_inferior_event () selected.". */ if (!non_stop && cmd_done - && ecs->ws.kind () != TARGET_WAITKIND_NO_RESUMED) + && ecs.ws.kind () != TARGET_WAITKIND_NO_RESUMED) restore_thread.dont_restore (); } } @@ -5977,14 +5949,11 @@ restart_threads (struct thread_info *event_thread, inferior *inf) } else { - struct execution_control_state ecss; - struct execution_control_state *ecs = &ecss; - infrun_debug_printf ("restart threads: [%s] continuing", tp->ptid.to_string ().c_str ()); - reset_ecs (ecs, tp); + execution_control_state ecs (tp); switch_to_thread (tp); - keep_going_pass_signal (ecs); + keep_going_pass_signal (&ecs); } } } @@ -7660,8 +7629,7 @@ restart_after_all_stop_detach (process_stratum_target *proc_target) if (thr->state != THREAD_RUNNING) continue; - execution_control_state ecs; - reset_ecs (&ecs, thr); + execution_control_state ecs (thr); switch_to_thread (thr); keep_going (&ecs); return; @@ -7676,8 +7644,6 @@ static bool keep_going_stepped_thread (struct thread_info *tp) { frame_info_ptr frame; - struct execution_control_state ecss; - struct execution_control_state *ecs = &ecss; /* If the stepping thread exited, then don't try to switch back and resume it, which could fail in several different ways depending @@ -7708,7 +7674,7 @@ keep_going_stepped_thread (struct thread_info *tp) infrun_debug_printf ("resuming previously stepped thread"); - reset_ecs (ecs, tp); + execution_control_state ecs (tp); switch_to_thread (tp); tp->set_stop_pc (regcache_read_pc (get_thread_regcache (tp))); @@ -7757,7 +7723,7 @@ keep_going_stepped_thread (struct thread_info *tp) { infrun_debug_printf ("expected thread still hasn't advanced"); - keep_going_pass_signal (ecs); + keep_going_pass_signal (&ecs); } return true;