From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by sourceware.org (Postfix) with ESMTPS id 9EE8B3858D1E for ; Wed, 20 Sep 2023 16:32:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9EE8B3858D1E 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-wr1-f50.google.com with SMTP id ffacd0b85a97d-32167a4adaaso5449f8f.1 for ; Wed, 20 Sep 2023 09:32:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695227559; x=1695832359; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=6BKlGY5Vbv2tl00CiALLdBmVtkK5U3z8MuCId0rdqog=; b=HA+16LL4r+UpbDHBVb8unTdxVKjhxj7z750m+m/1Oz3CbbwkXzZ5UolEGCJTT/E7oh F9TRxVauOB+kU9bNwST+QEw129wzPot4DhbDxX/2QzpxXudnzjhwTR7YBVxOf2vlOoE8 P3tvdRaP5KTW0sMA12wm1DyI/2pm6AFC/UDDfSQX6pws47qMuWXF132Py9SvXuJ1x1wG tq5iFlnAkaWaBk7MhXAy7yXHTyC5WgpIPO46lHKLV8ERFrRr72ikxvviN/W5Qjs8yV98 S1P4AAnmXZ8dCTQMCPZlaNlQbGCjPpB078zmdtzc49k1iaeSmc5n5qsFWn9hA9S1iWHp yi+g== X-Gm-Message-State: AOJu0YzWg1Q3ZidnyStm4rBRsJcedRLIzgjoAI/R4W0zQaslQbVb9gkL NtDvvDL8H+LOZ27NpDQt804= X-Google-Smtp-Source: AGHT+IFFxo3y2x87esBH7SSWHE+7jexwiVI2rRPQJM0fy7zEejkySS90wC+xNMJYFDnNTvowCV5SrQ== X-Received: by 2002:adf:f08a:0:b0:31f:f43e:4f3c with SMTP id n10-20020adff08a000000b0031ff43e4f3cmr3131304wro.8.1695227558781; Wed, 20 Sep 2023 09:32:38 -0700 (PDT) Received: from ?IPV6:2001:8a0:f939:d200:f66d:9e3b:9c38:4aec? ([2001:8a0:f939:d200:f66d:9e3b:9c38:4aec]) by smtp.gmail.com with ESMTPSA id e17-20020adffc51000000b0031435731dfasm18825207wrs.35.2023.09.20.09.32.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 20 Sep 2023 09:32:38 -0700 (PDT) Message-ID: <24534cd7-9c80-19da-5a9c-17962ae46fb3@palves.net> Date: Wed, 20 Sep 2023 17:32:28 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.1 Subject: Re: [PATCH 1/1] gdb, gdbserver: replace PBUFSIZ with a target op Content-Language: en-US To: Andrew Burgess , "Gerlicher, Klaus" , Simon Marchi , "gdb-patches@sourceware.org" References: <20230919054511.17998-1-klaus.gerlicher@intel.com> <20230919054511.17998-2-klaus.gerlicher@intel.com> <3d0d3efc-f802-4a3a-a602-dc3e59c99c94@polymtl.ca> <87sf79xanx.fsf@redhat.com> From: Pedro Alves In-Reply-To: <87sf79xanx.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.3 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=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: Hi! On 2023-09-20 13:59, Andrew Burgess via Gdb-patches wrote: > "Gerlicher, Klaus via Gdb-patches" writes: > >> Hi Simon, >> >> Thanks for the quick response. >> >> At least the initial buffer size needs to be fixed since now most >> clients aren't aware of any dynamic behavior here and therefore we >> need at least something pre-allocated for these clients. > > I don't understand your concerns here. For this patch we're only > talking about the gdbserver client, right? And your patch (rightly) > doesn't change things on the GDB side. The client is the GDB side, the server side is, well, gdbserver. :-) > > GDB already uses a dynamic packet buffer size. It is dynamic, but not in the sense that we just append to the buffer with push_back and let the buffer grow unbounded. Instead, GDB tries to guess a sufficient packet size, but if the server tells it explicitly what packet size it supports, then GDB will grow its buffer to that size, no questions asked. /* If we increased the packet size, make sure to increase the global buffer size also. We delay this until after parsing the entire qSupported packet, because this is the same buffer we were parsing. */ if (rs->buf.size () < rs->explicit_packet_size) rs->buf.resize (rs->explicit_packet_size); > So the only initial > buffer I think you can be talking about here is the gdbserver buffer, > which I think could be made dynamic, just as GDB's is. I think he was really talking about the GDB side. Or even other clients, like LLDB, etc. > > We could hard-code gdbserver to return some stupidly large number for > the PacketSize in the qSupported reply, say MAX_INT? Or (MAX_INT / 4), > you pick, this could be anything really, just something huge. I don't think it can, due to the immediate resize mentioned above. I don't understand why the patch added a target method on the gdb side. Also, do we really need the new target method on the gdbserver side? We assert that the buffer size is bigger than the tdesc's register size plus a slack, but how about flipping that around and make the buffer size be dependent on the register size? Maybe the packet size decision is done earlier than we know which tdesc we are using, though, that's something to check. Pedro Alves