From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id B12233858426 for ; Sat, 7 Oct 2023 01:35:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B12233858426 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=polymtl.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=polymtl.ca 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 3971Zjpe001472 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 6 Oct 2023 21:35:50 -0400 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 3971Zjpe001472 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=polymtl.ca; s=default; t=1696642551; bh=qBdMQiZIG1ld+Y6HSXfPLSIn0vZV0xpymndyFvLxSUM=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=dz7RAc4EJ4L/Ah9WJ12ODSPSlruhLx/sfo3MFLP6xChDEb6LAJ5bW21woQvrF6QaW jvsqeopGFYHScSOlReicdjeafOhvmBZq8qD7frmGSilzMO+efwGY/Rq7WOCkmnzVYp rrwhBVR6cn41at0m/fq8hUX1MZat/MGG2jiCXjes= Received: from [10.201.39.111] (209-15-164-2.static.cgocable.ca [209.15.164.2]) (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 B197E1E092; Fri, 6 Oct 2023 21:35:45 -0400 (EDT) Message-ID: <20aa237c-7e6b-48cb-9456-60b6cd0da4e5@polymtl.ca> Date: Fri, 6 Oct 2023 21:35:45 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 3/3] gdb: add assertion when marking the remote async flag Content-Language: fr To: "Terekhov, Mikhail" , "gdb-patches@sourceware.org" Cc: Simon Marchi References: <20231004020701.260411-1-simon.marchi@polymtl.ca> <20231004020701.260411-4-simon.marchi@polymtl.ca> From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Sat, 7 Oct 2023 01:35:46 +0000 X-Spam-Status: No, score=-3037.3 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 List-Id: On 10/6/23 17:09, Terekhov, Mikhail wrote: >> -----Original Message----- >> From: Gdb-patches > bounces+mikhail.terekhov=dell.com@sourceware.org> On Behalf Of Simon >> Marchi via Gdb-patches >> Sent: Tuesday, October 3, 2023 10:04 PM >> To: gdb-patches@sourceware.org >> Cc: Simon Marchi >> Subject: [PATCH 3/3] gdb: add assertion when marking the remote async flag >> >> >> [EXTERNAL EMAIL] >> >> 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://urldefense.com/v3/__https://sourceware.org/bugzilla/show_bug.cgi?id >> =30630__;!!LpKI!j0fTzubsC8RVdwinGn-r82- >> mX7hLUvNffNCQWwFTCYyR9EWj0CtlY9ERyT_AXgqsNQ0ZjS4Ls53fdBnUrLhWIw >> hIBW9P$ [sourceware[.]org] >> >> 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); } > > This change made the need for fix suggested in [1] more obvious. > The assert in mark_async_event_handler () is stronger than check in > remote_target::queued_stop_reply(). > I.e. mark_async_event_handler () should not be called in > remote_target::queued_stop_reply() unless async_handler != NULL i.e. > target_is_async_p() !=0. > > I'd suggest to merge in fix from [1] into this series. Yes, your fix needs to be merged independently of my series. My series is only to add an assertion closer to the root of the problem (which could help catch other problems in the future). Simon