From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by sourceware.org (Postfix) with ESMTPS id B71823858D35 for ; Thu, 16 Dec 2021 21:15:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B71823858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f42.google.com with SMTP id bg2-20020a05600c3c8200b0034565c2be15so2709940wmb.0 for ; Thu, 16 Dec 2021 13:15:34 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:from :subject:to:references:content-language:in-reply-to :content-transfer-encoding; bh=AZs6T9b1aYfczE4j7+idBkOGagA3ZRkZv/TSwsvM1Ws=; b=RZ72HiYsHkCpQ6O0u7dFwNy2k0doPIfyf0Q6v6qdMXqKpdx1cy+hF78ithMfCAMzTr J7MdVohYzqRVj+DbZsrvwvEABEpI3LYkpBMtYOIZNxiGNlNqkW9oQ/SeBcb0Y3f4DsVE Vx4TXpIoG13U6t6kxYHtLpIn0iJr7QbB2Qz6kHng4qrd7SDSmw5vB4JKI6+Do0WWfwo6 KZgN5A5vEb6Ty0DDHGiBC2PcI3WWNDPfJhdVo/XRzA2TN1R6kqRsVwI5ZUHgMcdF1Vbd RlrSQfFKOaUZ1rvRHiSlTXAobG/8PRb7+0HymmLZrWbMYcdUKwywzQ3RuEbi+SciMQtQ HkqA== X-Gm-Message-State: AOAM530Eg3bZ79m7kSc5tBIxjUYKUqiWLNYHmAv3FFh1hOEVTOr8UAw0 elWOg5g7oPWyrNoXWwYRN3ayz0UFHnU= X-Google-Smtp-Source: ABdhPJw8QG2rd+24jWjbe0Xq67bODWPVQ49nz91riJekmyvxCf/rCh2287gIxKbsLz438/faJOKk0g== X-Received: by 2002:a05:600c:3ba2:: with SMTP id n34mr6694158wms.162.1639689333687; Thu, 16 Dec 2021 13:15:33 -0800 (PST) Received: from ?IPV6:2001:8a0:f912:1a00:fb57:3faf:e98:b979? ([2001:8a0:f912:1a00:fb57:3faf:e98:b979]) by smtp.gmail.com with ESMTPSA id f15sm7059593wmg.30.2021.12.16.13.15.32 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 16 Dec 2021 13:15:32 -0800 (PST) Message-ID: <209f1d51-b4fb-0086-9034-b2720219f2f6@palves.net> Date: Thu, 16 Dec 2021 21:15:31 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0 From: Pedro Alves Subject: Re: [PATCHv3 1/2] gdb/remote: some fixes for 'maint set target-async off' To: Andrew Burgess , gdb-patches@sourceware.org References: <849047bbe03fae7f392d2b6f8f50ba299bf694f4.1638354426.git.aburgess@redhat.com> Content-Language: en-US In-Reply-To: <849047bbe03fae7f392d2b6f8f50ba299bf694f4.1638354426.git.aburgess@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Thu, 16 Dec 2021 21:15:36 -0000 On 2021-12-01 10:40, Andrew Burgess via Gdb-patches wrote: > This problem can clearly be seen I feel by looking at the > remote_state::cached_wait_status flag. This flag tells GDB if there > is a wait status cached in remote_state::buf. However, in > remote_target::putpkt_binary and remote_target::getpkt_or_notif_sane_1 > this flag is just set back to 0, doing this immediately discards any > cached data. > > I don't know if this scheme ever made sense, maybe once upon a time, > GDB would, when it found it had no cached stop, simply re-request the > stop information from the target, however, this certainly isn't the > case now, and resetting the cached_wait_status is (I claim) a bad > thing. I don't think that was ever the case. Take a look at 2d717e4f8a54, where the cached status was introduced to handle "attach". It was simply the case back then that nothing would talk to the target between the initial attach and consuming the event. It's not clear to me why putpkt/getpkt would need to clear the flag back then. Looks more like a "just in case" safeguard. > So, finally, in this commit, I propose to remove the > remote_state::cached_wait_status flag and to stop using the ::buf to > cache stop replies. Instead, stop replies will now always be stored > in the ::stop_reply_queue. To be honest, I don't recall exactly why I didn't do that when introducing the stop reply queue. > @@ -8163,15 +8151,12 @@ remote_target::wait_as (ptid_t ptid, target_waitstatus *status, > > stop_reply = queued_stop_reply (ptid); > if (stop_reply != NULL) > - return process_stop_reply (stop_reply, status); > - > - if (rs->cached_wait_status) > - /* Use the cached wait status, but only once. */ > - rs->cached_wait_status = 0; > + { > + rs->waiting_for_stop_reply = 0; This is a difference described in the commit log, but looking at the resulting code, I don't understand why clearing this flag is needed here, it looks like dead code to me. I mean, if we have a cached status already, then we're not waiting for a stop reply from the target. Did you run into a case where it was needed? Otherwise the patch LGTM. Thanks for doing this.