From patchwork Wed Oct 4 02:03:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 77072 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 54714385800C for ; Wed, 4 Oct 2023 02:08:12 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 54714385800C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1696385292; bh=4Kt6HGPWygrrpvUsZHZXmj69R54hUpLccP/vevvKQtw=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=ymlGbTr/2YdN4YLPas0WagmRALTLBmjSuSgKfb9TbHIQhaJ9ZXo1sR70cm6uK4otC 8IesaLisy6mW7kd/n8gzuk8k4vEK1jviz6MO5OsBTS59xLljV9KW3PGrAqdQ6xIt5o 6FuK1a0dwUH8WRHo2puyX1UGvFQU2JnCQKYpOYaw= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 3339B3858028 for ; Wed, 4 Oct 2023 02:07:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3339B3858028 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 394273rb018713 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 3 Oct 2023 22:07:08 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 394273rb018713 Received: from simark.localdomain (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 6E1291E0C2; Tue, 3 Oct 2023 22:07:03 -0400 (EDT) To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 1/3] gdb: make remote_state's async token private Date: Tue, 3 Oct 2023 22:03:59 -0400 Message-ID: <20231004020701.260411-2-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231004020701.260411-1-simon.marchi@polymtl.ca> References: <20231004020701.260411-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 4 Oct 2023 02:07:03 +0000 X-Spam-Status: No, score=-3188.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Simon Marchi via Gdb-patches From: Simon Marchi Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" From: Simon Marchi Make remote_async_inferior_event_token private (rename to m_async_event_handler_token) and add methods for the various operations we do on it. This will help by: - allowing to break on those methods when debugging - allowing to add assertions in the methods Change-Id: Ia3b8a2bc48ad4849dbbe83442c3f83920f03334d --- gdb/remote.c | 70 ++++++++++++++++++++++++++++++++-------------------- 1 file changed, 43 insertions(+), 27 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 9bb4f1de5982..2deba06d7d47 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -413,6 +413,31 @@ class remote_state /* Get the remote arch state for GDBARCH. */ struct remote_arch_state *get_remote_arch_state (struct gdbarch *gdbarch); + void create_async_event_handler () + { + m_async_event_handler_token + = ::create_async_event_handler ([] (gdb_client_data data) + { + inferior_event_handler (INF_REG_EVENT); + }, + nullptr, "remote"); + } + + void mark_async_event_handler () + { ::mark_async_event_handler (m_async_event_handler_token); } + + void clear_async_event_handler () + { ::clear_async_event_handler (m_async_event_handler_token); } + + bool async_event_handler_marked () const + { return ::async_event_handler_marked (m_async_event_handler_token); } + + void delete_async_event_handler () + { + if (m_async_event_handler_token != nullptr) + ::delete_async_event_handler (&m_async_event_handler_token); + } + public: /* data */ /* A buffer to use for incoming packets, and its current size. The @@ -540,10 +565,6 @@ class remote_state immediately, so queue is not needed for them. */ std::vector stop_reply_queue; - /* Asynchronous signal handle registered as event loop source for - when we have pending events ready to be passed to the core. */ - struct async_event_handler *remote_async_inferior_event_token = nullptr; - /* FIXME: cagney/1999-09-23: Even though getpkt was called with ``forever'' still use the normal timeout mechanism. This is currently used by the ASYNC code to guarentee that target reads @@ -554,6 +575,10 @@ class remote_state bool wait_forever_enabled_p = true; private: + /* Asynchronous signal handle registered as event loop source for + when we have pending events ready to be passed to the core. */ + async_event_handler *m_async_event_handler_token = nullptr; + /* Mapping of remote protocol data for each gdbarch. Usually there is only one entry here, though we may see more with stubs that support multi-process. */ @@ -1384,8 +1409,6 @@ static void show_remote_protocol_packet_cmd (struct ui_file *file, static ptid_t read_ptid (const char *buf, const char **obuf); -static void remote_async_inferior_event_handler (gdb_client_data); - static bool remote_read_description_p (struct target_ops *target); static void remote_console_output (const char *msg); @@ -4389,8 +4412,7 @@ remote_target::~remote_target () everything of this target. */ discard_pending_stop_replies_in_queue (); - if (rs->remote_async_inferior_event_token) - delete_async_event_handler (&rs->remote_async_inferior_event_token); + rs->delete_async_event_handler (); delete rs->notif_state; } @@ -5989,9 +6011,8 @@ remote_target::open_1 (const char *name, int from_tty, int extended_p) current_inferior ()->push_target (std::move (target_holder)); /* Register extra event sources in the event loop. */ - rs->remote_async_inferior_event_token - = create_async_event_handler (remote_async_inferior_event_handler, nullptr, - "remote"); + rs->create_async_event_handler (); + rs->notif_state = remote_notif_state_allocate (remote); /* Reset the target state; these things will be queried either by @@ -7086,7 +7107,7 @@ remote_target::has_pending_events () { remote_state *rs = get_remote_state (); - if (async_event_handler_marked (rs->remote_async_inferior_event_token)) + if (rs->async_event_handler_marked ()) return true; /* Note that BUFCNT can be negative, indicating sticky @@ -7420,7 +7441,7 @@ remote_notif_stop_can_get_pending_events (remote_target *remote, may exit and we have no chance to process them back in remote_wait_ns. */ remote_state *rs = remote->get_remote_state (); - mark_async_event_handler (rs->remote_async_inferior_event_token); + rs->mark_async_event_handler (); return 0; } @@ -7628,7 +7649,7 @@ remote_target::queued_stop_reply (ptid_t ptid) if (!rs->stop_reply_queue.empty () && target_can_async_p ()) { /* There's still at least an event left. */ - mark_async_event_handler (rs->remote_async_inferior_event_token); + rs->mark_async_event_handler (); } return r; @@ -7655,7 +7676,7 @@ remote_target::push_stop_reply (struct stop_reply *new_event) enabled, and there are events in this queue, we will mark the event token at that point, see remote_target::async. */ if (target_is_async_p ()) - mark_async_event_handler (rs->remote_async_inferior_event_token); + rs->mark_async_event_handler (); } /* Returns true if we have a stop reply for PTID. */ @@ -8515,10 +8536,9 @@ remote_target::wait (ptid_t ptid, struct target_waitstatus *status, we'll mark it again at the end if needed. If the target is not in async mode then the async token should not be marked. */ if (target_is_async_p ()) - clear_async_event_handler (rs->remote_async_inferior_event_token); + rs->clear_async_event_handler (); else - gdb_assert (!async_event_handler_marked - (rs->remote_async_inferior_event_token)); + gdb_assert (!rs->async_event_handler_marked ()); ptid_t event_ptid; @@ -8533,7 +8553,7 @@ remote_target::wait (ptid_t ptid, struct target_waitstatus *status, notifications, then tell the event loop to call us again. */ if (!rs->stop_reply_queue.empty () || rs->notif_state->pending_event[notif_client_stop.id] != nullptr) - mark_async_event_handler (rs->remote_async_inferior_event_token); + rs->mark_async_event_handler (); } return event_ptid; @@ -14859,12 +14879,6 @@ remote_async_serial_handler (struct serial *scb, void *context) inferior_event_handler (INF_REG_EVENT); } -static void -remote_async_inferior_event_handler (gdb_client_data data) -{ - inferior_event_handler (INF_REG_EVENT); -} - int remote_target::async_wait_fd () { @@ -14884,7 +14898,8 @@ remote_target::async (bool enable) /* If there are pending events in the stop reply queue tell the event loop to process them. */ if (!rs->stop_reply_queue.empty ()) - mark_async_event_handler (rs->remote_async_inferior_event_token); + rs->mark_async_event_handler (); + /* For simplicity, below we clear the pending events token without remembering whether it is marked, so here we always mark it. If there's actually no pending notification to @@ -14899,7 +14914,8 @@ remote_target::async (bool enable) /* If the core is disabling async, it doesn't want to be disturbed with target events. Clear all async event sources too. */ - clear_async_event_handler (rs->remote_async_inferior_event_token); + rs->clear_async_event_handler (); + if (target_is_non_stop_p ()) clear_async_event_handler (rs->notif_state->get_pending_events_token); } From patchwork Wed Oct 4 02:04:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 77070 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B2A5E3857036 for ; Wed, 4 Oct 2023 02:07:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B2A5E3857036 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1696385254; bh=j4HaqsaDSs+ZfJ85cI+lFqwbuideMEP0nGusNHjOzCE=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=CR+bRsxRM1BKDOZJtFsXc7iOabOBGwry7O+5ensi11A0DIzgqZ4ofJs0ECEC3OwEr /SwUXk9ciGely2a9JjQKT7J8sztKjfXccd1Y2/i4Mj8mnEOFnxyBh5jzImmpvhcgTG W0AdRiRz5gkYf/ERFNWKFTsdxOYZmrYUyhY5fhYY= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id F2CA23858017 for ; Wed, 4 Oct 2023 02:07:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org F2CA23858017 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 3942739k018717 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 3 Oct 2023 22:07:08 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 3942739k018717 Received: from simark.localdomain (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id B3EB01E0C3; Tue, 3 Oct 2023 22:07:03 -0400 (EDT) To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 2/3] gdb: add remote_state::{is_async_p,can_async_p} Date: Tue, 3 Oct 2023 22:04:00 -0400 Message-ID: <20231004020701.260411-3-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231004020701.260411-1-simon.marchi@polymtl.ca> References: <20231004020701.260411-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 4 Oct 2023 02:07:03 +0000 X-Spam-Status: No, score=-3188.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Simon Marchi via Gdb-patches From: Simon Marchi Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" From: Simon Marchi A subsequent patch will want to know if the remote is async within a remote_state method. Add a helper method for that, and for "can async" as well, for symmetry. Change-Id: Id0f648ee4896736479fa942f5453eeeb0e5d4352 --- gdb/remote.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index 2deba06d7d47..38d0027dbf9e 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -438,6 +438,20 @@ class remote_state ::delete_async_event_handler (&m_async_event_handler_token); } + bool is_async_p () const + { + /* We're async whenever the serial device is. */ + return (this->remote_desc + != nullptr && serial_is_async_p (this->remote_desc)); + } + + bool can_async_p () const + { + /* We can async whenever the serial device can. */ + return (this->remote_desc + != nullptr && serial_can_async_p (this->remote_desc)); + } + public: /* data */ /* A buffer to use for incoming packets, and its current size. The @@ -14853,16 +14867,14 @@ remote_target::can_async_p () gdb_assert (target_async_permitted); /* We're async whenever the serial device can. */ - struct remote_state *rs = get_remote_state (); - return serial_can_async_p (rs->remote_desc); + return get_remote_state ()->can_async_p (); } bool remote_target::is_async_p () { /* We're async whenever the serial device is. */ - struct remote_state *rs = get_remote_state (); - return serial_is_async_p (rs->remote_desc); + return get_remote_state ()->is_async_p (); } /* Pass the SERIAL event on and up to the client. One day this code From patchwork Wed Oct 4 02:04:01 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 77071 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 76D653857036 for ; Wed, 4 Oct 2023 02:07:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 76D653857036 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1696385259; bh=DMGtDh9Ma0iaTZ4bwyiTuU+U9SYsr6UhJH1zGGFSTR4=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=VRoU8+hX54WdkZNAtVmNLpVV2t1nDjuGPgChnz39ihuVCQnEEnocV/H0E7gncphe1 GGK7u/n/UzqzYDgM0JofKlfAiQiHT9PLT41E7LBEghr4m/mvIwR62MF9/0zklooJW3 Q/Plrs8VKcve1CV+JRW/P5mucnrpT3vLngOc40i4= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 089673858025 for ; Wed, 4 Oct 2023 02:07:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 089673858025 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 394274wA018720 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 3 Oct 2023 22:07:08 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 394274wA018720 Received: from simark.localdomain (modemcable238.237-201-24.mc.videotron.ca [24.201.237.238]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id DF2DB1E0D0; Tue, 3 Oct 2023 22:07:03 -0400 (EDT) To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 3/3] gdb: add assertion when marking the remote async flag Date: Tue, 3 Oct 2023 22:04:01 -0400 Message-ID: <20231004020701.260411-4-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231004020701.260411-1-simon.marchi@polymtl.ca> References: <20231004020701.260411-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 4 Oct 2023 02:07:04 +0000 X-Spam-Status: No, score=-3188.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_PASS, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Simon Marchi via Gdb-patches From: Simon Marchi Reply-To: Simon Marchi Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" From: Simon Marchi As reported in bug 30630 [1], we hit a case where the remote target's async flag is marked while the target is not configured (yet) to work async. This should not happen. It is caught thanks to this assert in remote_target::wait: /* Start by clearing the flag that asks for our wait method to be called, we'll mark it again at the end if needed. If the target is not in async mode then the async token should not be marked. */ if (target_is_async_p ()) rs->clear_async_event_handler (); else gdb_assert (!rs->async_event_handler_marked ()); This is helpful, but I think that we could have caught the problem earlier than that, at the moment we marked the handler. Catching problems earlier makes them easier to debug. [1] https://sourceware.org/bugzilla/show_bug.cgi?id=30630 Change-Id: I7e229c74b04da82bef6a817d5a676be5cf52e833 --- gdb/remote.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gdb/remote.c b/gdb/remote.c index 38d0027dbf9e..7830b5cec33f 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -424,7 +424,10 @@ class remote_state } void mark_async_event_handler () - { ::mark_async_event_handler (m_async_event_handler_token); } + { + gdb_assert (this->is_async_p ()); + ::mark_async_event_handler (m_async_event_handler_token); + } void clear_async_event_handler () { ::clear_async_event_handler (m_async_event_handler_token); }