From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id gVQUA2jyw2g3SwEAWB0awg (envelope-from ) for ; Fri, 12 Sep 2025 06:14:00 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=R0LLKbEx; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 08D581E0BA; Fri, 12 Sep 2025 06:14:00 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (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 ESMTPS id 29AC51E04C for ; Fri, 12 Sep 2025 06:13:59 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AF9E43857C4F for ; Fri, 12 Sep 2025 10:13:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AF9E43857C4F Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=R0LLKbEx Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 711C53858D21 for ; Fri, 12 Sep 2025 10:13:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 711C53858D21 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 711C53858D21 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1757672002; cv=none; b=JfQYkFggT6oMx1PCEWrJ8+fe6aa0dKSq+yg63GF8xdbKqvRdFg3dT0kJDew6UhQCaH+quOYsp+kahq9RSu1r6e0aT+AuOGvht/wRA14YMqusZy4M84LPyweNpOY8WQhBGbROdVY93Np//clnbsZB0W4fQKnbOPvVHL7eo9Os6cU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1757672002; c=relaxed/simple; bh=5QRlTQUDtWCiTAbtOwHaEgoeCvxNWEGifvJtsyfJDiY=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=QrIg0ws4VB/H8eUBcczKGJ6JymW1/+s7WODklMyFN7TnBqFZvHa30FicyfJVrHZeOG5tFEt8FVCPQSZYhEzXGEbGDOLRjK2UCsljUtLg3uzQbNVjmnxJe66GA4wvwnJoQ/FBzSIce+qCvduHRaOhhsR3DAlJACwSUQC6WSZbswI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 711C53858D21 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1757672002; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=9M3yUg2a5uzxFUvBr33WK1BLHJSaKTT4fPYk8U+2wic=; b=R0LLKbExPJBpGnGbk+Jnbn/irRXYzz/DJCuwnMwGuPOclLgM7J5cmkFqFDP6LtDaAVH65n EGl08EvBrJ4igo9F+yQgYoAAWu6oxZTqVPDeT6/RRnqeZR7R/onhQtFOXYzxxEYX2LDxHG 6VjA1DFC7IAt8VXixu4iI/u12VTedxE= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-270-XBnv8HpCOuSAySC6xFmmwA-1; Fri, 12 Sep 2025 06:13:21 -0400 X-MC-Unique: XBnv8HpCOuSAySC6xFmmwA-1 X-Mimecast-MFC-AGG-ID: XBnv8HpCOuSAySC6xFmmwA_1757672000 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-45decfa5b26so9740575e9.3 for ; Fri, 12 Sep 2025 03:13:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1757672000; x=1758276800; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=9M3yUg2a5uzxFUvBr33WK1BLHJSaKTT4fPYk8U+2wic=; b=UH4oiXb7oI5whI0vSRrJMZLANhJeDwHUc10reVbpISz/Pe6WKZUF9SHqPLMjDXyVlA keNPLBZoHK+2qroEzRzHcCUaXzevXxInEMMfve9kns/vXXIqfUOaUb9feNCE8x74WFGk ObXMutYNYUalzsDIiAF7ioMXb6kpc3Bu9K/3YwzPNSOSA4ddkGp8Dk251VBm1OsC+Him 8/6DzKl54m3mH7vXTKvZs5XBn1bLvLosZmXsXIL+INxvLPgNTSctMTCHc5IvTNk3IY8u y1HXebnvPGwWeDUMOTDuo0ZHhdbPgEiVkKvGzhiwM2puwviKzbg/x4xnGmzIQhob6+h7 ZPqg== X-Gm-Message-State: AOJu0YxA54EDvKcgmvJC3VTdzrNT2mG26bUx0MU8v5EERNINNQVeNnvT qnI9Aihe0f4sbh1gAmCAQ+d3uED4+1e7cS4VhkLa7uuGYOSLQMgAxlpEnwIYeX6ySLqKdmT/LJr Zxni0WN+SaJp+5t359pQi+urbjZ4F8Lpc5mU9pThEQfGeHk5aclL/VMZoDKcMnAc= X-Gm-Gg: ASbGncvNzb7hktBkctVUHDy2rf+SDasSzDr9uVrWhVP1+8vOE6T1g0WLzlRNjmkjR50 jjPRpVHwvaqg2osEYO/jIUoViPaComDQc9KZQ016mjPBNafXEuJaPLzVwrF7aC1Rv20lcaNjD8a Jvyk390n8NCWQYQFiQfq9TdrQ33Tgvs9Fljyrpg51fjr2EVhhXd8i6vl4aFlx0K2s7Ptq02EkYt Haf4JAql5OsMmQXWXvFP+A1Q6+e5Tqm3kV+toKxeAMOfwTt8PHFQm1CYpuuX8ryKDTBSbQvN2yz mW3w7I9nzZCmcVF33qunrrmf1l3rK529tBI= X-Received: by 2002:a05:600c:8486:b0:456:18cf:66b5 with SMTP id 5b1f17b1804b1-45f211f7aaamr20025875e9.22.1757671999643; Fri, 12 Sep 2025 03:13:19 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEwkyhKzaLNsQd+AlIUVrSzE0C343yIrtxe/BZY/RMBUrTsJ3ZwbOO5moSFMTG4fOeGa0vU7w== X-Received: by 2002:a05:600c:8486:b0:456:18cf:66b5 with SMTP id 5b1f17b1804b1-45f211f7aaamr20025675e9.22.1757671999110; Fri, 12 Sep 2025 03:13:19 -0700 (PDT) Received: from localhost ([31.111.84.207]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-45e015775f1sm60812405e9.8.2025.09.12.03.13.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Sep 2025 03:13:18 -0700 (PDT) From: Andrew Burgess To: Kevin Buettner Cc: gdb-patches@sourceware.org, Michael Weghorn , Eli Zaretskii , Guinevere Larsen Subject: Re: [PATCH] gdb/gdbserver: pass inferior arguments as a single string In-Reply-To: <20250624145052.4659642b@f41-zbm-amd> References: <20250624145052.4659642b@f41-zbm-amd> Date: Fri, 12 Sep 2025 11:13:17 +0100 Message-ID: <874it8htc2.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: AgGBHm41HdxPQnCXaZz8UiWKzaCXDLSCIX9IAnx5C24_1757672000 X-Mimecast-Originator: redhat.com Content-Type: text/plain 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: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Kevin Buettner writes: > Hi Andrew, > > I have a few commit log nits - see (inline) below. > > On Tue, 13 May 2025 11:31:04 +0100 > Andrew Burgess wrote: > >> GDB holds the inferior arguments as a single string. Currently when >> GDB needs to pass the inferior arguments to a remote target as part of >> a vRun packet, this is done by splitting the single argument string >> into its component arguments by calling gdb::remote_args::split, which >> uses the gdb_argv class to split the arguments for us. >> >> The same gdb_argv class is used when the user has asked GDB/gdbserver >> to start the inferior without first invoking a shell; the gdb_argv >> class is used to split the argument string into it component >> arguments, and each is passed as a separate argument to the execve >> call which spawns the inferior. >> >> There is however, a problem with using gdb_argv to split the arguments >> before passing them to a remote target. To understand this problem we >> must first understand how gdb_argv is used when invoking an inferior >> without a shell. >> >> And to understand how gdb_argv is used to start an inferior without a >> shell, I feel we need to first look at an example of starting an >> inferior with a shell. >> >> Consider these two cases: >> >> (a) (gdb) set args \$VAR >> (b) (gdb) set args $VAR >> >> When starting with a shell, in case (a) the user expects the inferior >> to receive a literal '$VAR' string as an argument, while in case (b) >> the user expects to see the shell expanded value of the variable $VAR. >> >> If the user does 'set startup-with-shell off', then in (a) GDB will >> strip the '\' while splitting the arguments, and the inferior will be >> passed a literal '$VAR'. In (b) there is no '\' to strip, so also in >> this case the inferior will receive a literal '$VAR', remember >> startup-with-shell is off, so there is no shell than can ever expand > > s/than/that/ > >> $VAR. >> >> Notice, that when startup-with-shell is off, we end up with a many to >> one mapping, both (a) and (b) result in the literal string $VAR being >> passed to the inferior. I think this is the correct behaviour in this >> case. >> >> However, as we use gdb_argv to split the remote arguments we have the >> same many to one mapping within the vRun packet. But the vRun packet >> will be used when startup-with-shell is both on and off. What this >> means is that when gdbserver receives a vRun packet containing '$VAR' >> it doesn't know if GDB actually had '$VAR', or if GDB had '\$VAR'. >> And this is a huge problem. >> >> We can address this by making the argument splitting for remote >> targets smarter, and I do have patches that try to do this in this >> series: >> >> https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com >> >> That series was pretty long, and wasn't getting reviewed, so I'm >> pulling the individual patches out and posting them separately. >> >> This patch doesn't try to make remote argument splitting. I think > > s/make/do/ > >> that splitting and then joining the arguments is a mistake which can >> only introduce problems. The patch in the above series which tries to >> make the splitting and joining "smarter" handles unquoted, single >> quoted, and double quoted strings. But doesn't really address > > s/But doesn't/But that doesn't/ > >> parameter substitution, command substitution, or arithmetic expansion. >> And even if we did try to address these cases, what rules exactly >> would we implement? Probably POSIX shell rules, but what if the >> remote target doesn't have a POSIX shell? The only reason we're >> talking about which shell rules to follow is because the splitting and >> joining logic needs to mirror those rules. If we stop splitting and >> joining then we no longer need to care about the target's shell. >> >> Clearly, for backward compatibility we need to maintain some degree of >> argument splitting and joining as we currently have; and that's why I >> have a later patch (see the series above) that tries to improve that >> splitting and joining a little. But I think, what we should really >> do, is add a new feature flag (as used by the qSupported packet) and, >> if GDB and the remote target agree, we should pass the inferior >> arguments as a single string. >> >> This solves all our problems. In the startup with shell case, we no >> longer need to worry about splitting at all. The arguments are passed >> unmodified to the remote target, that can then pass the arguments to >> the shell directly. >> >> In the 'startup-with-shell off' case it is now up to the remote target >> to split the arguments, though in gdbserver we already did this, so >> nothing really changes in this case. And if the remote target doesn't >> have a POSIX shell, well GDB just doesn't need to worry about it! >> >> Something similar to this was originally suggested in this series: >> >> https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/ >> >> though this series didn't try to maintain backward compatibility, >> which I think is an issue that my patch solves. Additionally, this >> series only passed the arguments as a single string in some cases, >> I've simplified this so that, when GDB and the remote agree, the >> arguments are always passed as a single string. I think this is a >> little cleaner. >> >> I've also added documentation and some tests with this commit, >> including ensuring that we test both the new single string approach, >> and the fallback split/join approach. >> >> I've credited the author of the referenced series as co-author as they >> did come to a similar conclusion, though I think my implementation is >> different enough that I'm happy to list myself as primary author. >> >> Co-Authored-By: Michael Weghorn >> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392 >> >> Reviewed-By: Eli Zaretskii >> Tested-By: Guinevere Larsen > > Thanks for the detailed explanation. This makes sense to me. > > I also tested this patch and found no problems. > > Approved-by: Kevin Buettner Thanks for the feedback. I made these changes and pushed this patch. Thanks, Andrew