From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id +JhpI+nc919dIAAAWB0awg (envelope-from ) for ; Thu, 07 Jan 2021 23:17:45 -0500 Received: by simark.ca (Postfix, from userid 112) id 7DEAE1E965; Thu, 7 Jan 2021 23:17:45 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 38BD41E99A; Thu, 7 Jan 2021 23:17:41 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E3A883971C3F; Fri, 8 Jan 2021 04:17:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E3A883971C3F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1610079460; bh=NPmr5qj0D/P6pGU10W/xHgMzcp+Lcw/aa6riUzFnMkA=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=iczb+Nv3me0LWLv27dP1EaoDP0M/2Q5QDfQt8RLxQkZvVq/QKw9v0kOIdUQimqfwr v94JTq1N74hhgRt96otz2gdlZ17/kKuKzWfGc1cyuoMJWxoZxx4LhXf3QcpB0/sOBE 6gIa7Qus2BCmf+oRDKiy4v7dCdN45/Y66286A4tc= Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 7E56B3971C2F for ; Fri, 8 Jan 2021 04:17:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 7E56B3971C2F X-ASG-Debug-ID: 1610079455-0c856e67e239b4a0001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id appmzrmeI2vZaOEJ (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 07 Jan 2021 23:17:35 -0500 (EST) X-Barracuda-Envelope-From: simon.marchi@polymtl.ca X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from simark.localdomain (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) by smtp.ebox.ca (Postfix) with ESMTP id 82ADC441D66; Thu, 7 Jan 2021 23:17:35 -0500 (EST) X-Barracuda-RBL-IP: 192.222.157.6 X-Barracuda-Effective-Source-IP: 192-222-157-6.qc.cable.ebox.net[192.222.157.6] X-Barracuda-Apparent-Source-IP: 192.222.157.6 To: gdb-patches@sourceware.org Subject: [PATCH v3 3/5] gdb: move commit_resume to process_stratum_target Date: Thu, 7 Jan 2021 23:17:32 -0500 X-ASG-Orig-Subj: [PATCH v3 3/5] gdb: move commit_resume to process_stratum_target Message-Id: <20210108041734.3873826-4-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20210108041734.3873826-1-simon.marchi@polymtl.ca> References: <20210108041734.3873826-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1610079455 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 12478 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.87085 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Cc: Simon Marchi Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" From: Simon Marchi The following patch will change the commit_resume target method to something stateful. Because it would be difficult to track a state replicated in the various targets of a target stack, and since for the foreseeable future, only process stratum targets are going to use this concept, this patch makes the commit resume concept specific to process stratum targets. So, move the method to process_stratum_target, and move helper functions to process-stratum-target.h. gdb/ChangeLog: * target.h (struct target_ops) : New. (target_commit_resume): Remove. (make_scoped_defer_target_commit_resume): Remove. * target.c (defer_target_commit_resume): Remove. (target_commit_resume): Remove. (make_scoped_defer_target_commit_resume): Remove. * process-stratum-target.h (class process_stratum_target) : New. (maybe_commit_resume_all_process_targets): New. (make_scoped_defer_process_target_commit_resume): New. * process-stratum-target.c (defer_process_target_commit_resume): New. (maybe_commit_resume_process_target): New. (make_scoped_defer_process_target_commit_resume): New. * infrun.c (do_target_resume): Adjust. (commit_resume_all_targets): Rename into... (maybe_commit_resume_all_process_targets): ... this, adjust. (proceed): Adjust. * record-full.c (record_full_wait_1): Adjust. * target-delegates.c: Re-generate. Change-Id: Ifc957817ac5b2303e22760ce3d14740b9598f02c --- gdb/infrun.c | 28 +++++++++------------------- gdb/process-stratum-target.c | 23 +++++++++++++++++++++++ gdb/process-stratum-target.h | 29 +++++++++++++++++++++++++++++ gdb/record-full.c | 8 ++++---- gdb/target-delegates.c | 22 ---------------------- gdb/target.c | 22 ---------------------- gdb/target.h | 20 -------------------- 7 files changed, 65 insertions(+), 87 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 45bedf896419..1a27af51b7e9 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -2172,7 +2172,7 @@ do_target_resume (ptid_t resume_ptid, bool step, enum gdb_signal sig) target_resume (resume_ptid, step, sig); - target_commit_resume (); + maybe_commit_resume_process_target (tp->inf->process_target ()); if (target_can_async_p ()) target_async (1); @@ -2760,28 +2760,17 @@ schedlock_applies (struct thread_info *tp) execution_direction))); } -/* Calls target_commit_resume on all targets. */ +/* Calls maybe_commit_resume_process_target on all process targets. */ static void -commit_resume_all_targets () +maybe_commit_resume_all_process_targets () { scoped_restore_current_thread restore_thread; - /* Map between process_target and a representative inferior. This - is to avoid committing a resume in the same target more than - once. Resumptions must be idempotent, so this is an - optimization. */ - std::unordered_map conn_inf; - - for (inferior *inf : all_non_exited_inferiors ()) - if (inf->has_execution ()) - conn_inf[inf->process_target ()] = inf; - - for (const auto &ci : conn_inf) + for (process_stratum_target *target : all_non_exited_process_targets ()) { - inferior *inf = ci.second; - switch_to_inferior_no_thread (inf); - target_commit_resume (); + switch_to_target_no_thread (target); + maybe_commit_resume_process_target (target); } } @@ -3005,7 +2994,8 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) cur_thr->prev_pc = regcache_read_pc_protected (regcache); { - scoped_restore save_defer_tc = make_scoped_defer_target_commit_resume (); + scoped_restore save_defer_tc + = make_scoped_defer_process_target_commit_resume (); started = start_step_over (); @@ -3075,7 +3065,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal) } } - commit_resume_all_targets (); + maybe_commit_resume_all_process_targets (); finish_state.release (); diff --git a/gdb/process-stratum-target.c b/gdb/process-stratum-target.c index 719167803fff..1436a550ac04 100644 --- a/gdb/process-stratum-target.c +++ b/gdb/process-stratum-target.c @@ -108,3 +108,26 @@ switch_to_target_no_thread (process_stratum_target *target) break; } } + +/* If true, `maybe_commit_resume_process_target` is a no-op. */ + +static bool defer_process_target_commit_resume; + +/* See target.h. */ + +void +maybe_commit_resume_process_target (process_stratum_target *proc_target) +{ + if (defer_process_target_commit_resume) + return; + + proc_target->commit_resume (); +} + +/* See process-stratum-target.h. */ + +scoped_restore_tmpl +make_scoped_defer_process_target_commit_resume () +{ + return make_scoped_restore (&defer_process_target_commit_resume, true); +} diff --git a/gdb/process-stratum-target.h b/gdb/process-stratum-target.h index b513c26ffc2a..c8060c46be93 100644 --- a/gdb/process-stratum-target.h +++ b/gdb/process-stratum-target.h @@ -63,6 +63,20 @@ class process_stratum_target : public target_ops bool has_registers () override; bool has_execution (inferior *inf) override; + /* Commit a series of resumption requests previously prepared with + resume calls. + + GDB always calls `commit_resume` on the process stratum target after + calling `resume` on a target stack. A process stratum target may thus use + this method in coordination with its `resume` method to batch resumption + requests. In that case, the target doesn't actually resume in its + `resume` implementation. Instead, it takes note of resumption intent in + `resume`, and defers the actual resumption `commit_resume`. + + E.g., the remote target uses this to coalesce multiple resumption requests + in a single vCont packet. */ + virtual void commit_resume () {} + /* True if any thread is, or may be executing. We need to track this separately because until we fully sync the thread list, we won't know whether the target is fully stopped, even if we see @@ -92,4 +106,19 @@ extern std::set all_non_exited_process_targets (); extern void switch_to_target_no_thread (process_stratum_target *target); +/* Commit a series of resumption requests previously prepared with + target_resume calls. + + This function is a no-op if commit resumes are deferred (see + `make_scoped_defer_process_target_commit_resume`). */ + +extern void maybe_commit_resume_process_target + (process_stratum_target *target); + +/* Setup to defer `commit_resume` calls, and re-set to the previous status on + destruction. */ + +extern scoped_restore_tmpl + make_scoped_defer_process_target_commit_resume (); + #endif /* !defined (PROCESS_STRATUM_TARGET_H) */ diff --git a/gdb/record-full.c b/gdb/record-full.c index 22eaaa4bb1bc..56ab29479874 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -1242,11 +1242,11 @@ record_full_wait_1 (struct target_ops *ops, break; } + process_stratum_target *proc_target + = current_inferior ()->process_target (); + if (gdbarch_software_single_step_p (gdbarch)) { - process_stratum_target *proc_target - = current_inferior ()->process_target (); - /* Try to insert the software single step breakpoint. If insert success, set step to 0. */ set_executing (proc_target, inferior_ptid, false); @@ -1263,7 +1263,7 @@ record_full_wait_1 (struct target_ops *ops, "issuing one more step in the " "target beneath\n"); ops->beneath ()->resume (ptid, step, GDB_SIGNAL_0); - ops->beneath ()->commit_resume (); + proc_target->commit_resume (); continue; } } diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index 437b19b8581c..8b933fdf82eb 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -14,7 +14,6 @@ struct dummy_target : public target_ops void detach (inferior *arg0, int arg1) override; void disconnect (const char *arg0, int arg1) override; void resume (ptid_t arg0, int arg1, enum gdb_signal arg2) override; - void commit_resume () override; ptid_t wait (ptid_t arg0, struct target_waitstatus *arg1, target_wait_flags arg2) override; void fetch_registers (struct regcache *arg0, int arg1) override; void store_registers (struct regcache *arg0, int arg1) override; @@ -185,7 +184,6 @@ struct debug_target : public target_ops void detach (inferior *arg0, int arg1) override; void disconnect (const char *arg0, int arg1) override; void resume (ptid_t arg0, int arg1, enum gdb_signal arg2) override; - void commit_resume () override; ptid_t wait (ptid_t arg0, struct target_waitstatus *arg1, target_wait_flags arg2) override; void fetch_registers (struct regcache *arg0, int arg1) override; void store_registers (struct regcache *arg0, int arg1) override; @@ -440,26 +438,6 @@ debug_target::resume (ptid_t arg0, int arg1, enum gdb_signal arg2) fputs_unfiltered (")\n", gdb_stdlog); } -void -target_ops::commit_resume () -{ - this->beneath ()->commit_resume (); -} - -void -dummy_target::commit_resume () -{ -} - -void -debug_target::commit_resume () -{ - fprintf_unfiltered (gdb_stdlog, "-> %s->commit_resume (...)\n", this->beneath ()->shortname ()); - this->beneath ()->commit_resume (); - fprintf_unfiltered (gdb_stdlog, "<- %s->commit_resume (", this->beneath ()->shortname ()); - fputs_unfiltered (")\n", gdb_stdlog); -} - ptid_t target_ops::wait (ptid_t arg0, struct target_waitstatus *arg1, target_wait_flags arg2) { diff --git a/gdb/target.c b/gdb/target.c index 3a03a0ad530e..3a5270e5a416 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -2062,28 +2062,6 @@ target_resume (ptid_t ptid, int step, enum gdb_signal signal) clear_inline_frame_state (curr_target, ptid); } -/* If true, target_commit_resume is a nop. */ -static int defer_target_commit_resume; - -/* See target.h. */ - -void -target_commit_resume (void) -{ - if (defer_target_commit_resume) - return; - - current_top_target ()->commit_resume (); -} - -/* See target.h. */ - -scoped_restore_tmpl -make_scoped_defer_target_commit_resume () -{ - return make_scoped_restore (&defer_target_commit_resume, 1); -} - void target_pass_signals (gdb::array_view pass_signals) { diff --git a/gdb/target.h b/gdb/target.h index e1a1d7a9226b..a252c29eafb4 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -478,8 +478,6 @@ struct target_ops int TARGET_DEBUG_PRINTER (target_debug_print_step), enum gdb_signal) TARGET_DEFAULT_NORETURN (noprocess ()); - virtual void commit_resume () - TARGET_DEFAULT_IGNORE (); /* See target_wait's description. Note that implementations of this method must not assume that inferior_ptid on entry is pointing at the thread or inferior that ends up reporting an @@ -1431,24 +1429,6 @@ extern void target_disconnect (const char *, int); target_commit_resume below. */ extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal); -/* Commit a series of resumption requests previously prepared with - target_resume calls. - - GDB always calls target_commit_resume after calling target_resume - one or more times. A target may thus use this method in - coordination with the target_resume method to batch target-side - resumption requests. In that case, the target doesn't actually - resume in its target_resume implementation. Instead, it prepares - the resumption in target_resume, and defers the actual resumption - to target_commit_resume. E.g., the remote target uses this to - coalesce multiple resumption requests in a single vCont packet. */ -extern void target_commit_resume (); - -/* Setup to defer target_commit_resume calls, and reactivate - target_commit_resume on destruction, if it was previously - active. */ -extern scoped_restore_tmpl make_scoped_defer_target_commit_resume (); - /* For target_read_memory see target/target.h. */ /* The default target_ops::to_wait implementation. */ -- 2.29.2