From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 342DD3870907 for ; Mon, 25 Jan 2021 04:57:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 342DD3870907 X-ASG-Debug-ID: 1611550651-0c856e67e24d04b0001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id 8N8AwvB659YnZlyN (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 24 Jan 2021 23:57:31 -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 84957441D65; Sun, 24 Jan 2021 23:57:31 -0500 (EST) From: Simon Marchi 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 Cc: Simon Marchi Subject: [PATCH v4 2/3] gdb: move commit_resume to process_stratum_target Date: Sun, 24 Jan 2021 23:57:29 -0500 X-ASG-Orig-Subj: [PATCH v4 2/3] gdb: move commit_resume to process_stratum_target Message-Id: <20210125045730.1739754-3-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210125045730.1739754-1-simon.marchi@polymtl.ca> References: <20210125045730.1739754-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: 1611550651 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: 12494 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.50 X-Barracuda-Spam-Status: No, SCORE=0.50 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests=BSF_RULE7568M X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.87494 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.50 BSF_RULE7568M Custom Rule 7568M X-Spam-Status: No, score=-21.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_QUARANTINE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: , X-List-Received-Date: Mon, 25 Jan 2021 04:57:35 -0000 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 e70e3214d7b3..ad6f276038fc 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..ec127f524b74 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 process-stratum-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 c3307e812e71..464f4a85c558 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -1243,11 +1243,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); @@ -1264,7 +1264,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 9a8473d40e1a..58fd75687d0a 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -2053,28 +2053,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 917476d16ab6..c92ecd0f70d7 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.30.0