From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id C7C813858D39 for ; Tue, 3 Oct 2023 19:02:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C7C813858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1696359749; bh=QURYpPQ/fCFBZ6MOfTlao5FEaNqGvkWcUmU0gAty1+I=; h=Date:Subject:To:References:From:In-Reply-To:From; b=kdNtTmOxXq+HgL5ZFrbov/IM2ZMkqDoulNxzLD/m2n/Zw1f/+cRaGTBrBsmomgzbw PmrxYLpClUPPgsCs3/DX4SVrr8H4kTswGaVfrDaXHhyIaW9RA44t9hqwxUhKguspwG rzJ0OcrAfsUS7CbmQjy4Co9Cavd1oHwv65DfJEP4= Received: from [172.16.0.192] (192-222-143-198.qc.cable.ebox.net [192.222.143.198]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 415001E092; Tue, 3 Oct 2023 15:02:29 -0400 (EDT) Message-ID: <3d728a6e-1cb0-49c2-a4c8-0a974be39fee@simark.ca> Date: Tue, 3 Oct 2023 15:02:28 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] [PR gdb/30630] Fix internal-error in remote_target::wait Content-Language: fr To: Mikhail Terekhov , gdb-patches@sourceware.org References: <20231003172627.23077-1-Mikhail.Terekhov@dell.com> From: Simon Marchi In-Reply-To: <20231003172627.23077-1-Mikhail.Terekhov@dell.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,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 List-Id: On 10/3/23 13:26, Mikhail Terekhov via Gdb-patches wrote: > From: Mikhail Terekhov > > [PR gdb/30630] Assert inroduced in 32b1f5e8d6b8ddd3be6e471c26dd85a1dac31dda > causes gdb to fail when connecting to remote server in async > nonstop mode. This fix replaces call to target_can_async_p() > by target_is_async_p() in remote_target::queued_stop_reply() > as suggested by Christian Prochaska in comment6 to this PR. Hi Mikhail, Can you please add a bit more details about the problem and the fix in to the commit message to make things more obvious for readers? My understanding of it is: the important point in the reproducer is that when gdbserver attaches the process, the two threads are stopped and will generate stop reply upon connection. After consuming the first stop reply, the remote target will try to mark its async flag because there is another stop reply in the queue. But it shouldn't do that right now, because the target hasn't yet activated its async mode, as it's in the early stages of connection. > --- > gdb/remote.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gdb/remote.c b/gdb/remote.c > index 9bb4f1de598..a3221b6b5d1 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -7625,7 +7625,7 @@ remote_target::queued_stop_reply (ptid_t ptid) > remote_state *rs = get_remote_state (); > struct stop_reply *r = remote_notif_remove_queued_reply (ptid); > > - if (!rs->stop_reply_queue.empty () && target_can_async_p ()) > + if (!rs->stop_reply_queue.empty () && target_is_async_p ()) > { > /* There's still at least an event left. */ > mark_async_event_handler (rs->remote_async_inferior_event_token); > -- > 2.35.3 > I suppose we could catch the problem earlier than remote_target::wait. Right here it should not be allowed to mark the async handler while the target isn't async. Your change in itself makes sense to me. Could you turn the reproducer from the PR into a test, so we make sure the use case works and doesn't regress? You might need to use some less known features of the testsuite to launch gdbserver directly and connect to it, but you can probably inspire yourself from test gdb.server/attach-flag.exp (or just enhance that one). Actually, when I try to reproduce, I hit this assertion a bit further: void thread_info::set_pending_waitstatus (const target_waitstatus &ws) { gdb_assert (!this->has_pending_waitstatus ()); ... } >From my quick debugging it seems like gdbserver is sending two stop replies for the main thread, there is something fishy going on there. Thanks, Simon