From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id +NiMHOsdW2iG1RwAWB0awg (envelope-from ) for ; Tue, 24 Jun 2025 17:51:39 -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=Zb+/Ex4W; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 446AA1E11C; Tue, 24 Jun 2025 17:51:39 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-10.1 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, RCVD_IN_VALIDITY_RPBL,RCVD_IN_VALIDITY_SAFE 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 8D9181E0C2 for ; Tue, 24 Jun 2025 17:51:36 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 870C93858039 for ; Tue, 24 Jun 2025 21:51:35 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 870C93858039 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=Zb+/Ex4W Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 7B0253858429 for ; Tue, 24 Jun 2025 21:51:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7B0253858429 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 7B0253858429 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1750801862; cv=none; b=c4Sn88B065MN0DSV1pCpDUJ/5VvDNlzE8dk4YW2FFglpvig+LXkjsdCo/EUw+KFyy24mfktvHfzfC2MsnomV70MBqUOyOB0vBJwge49+L8TsKSNl25j/9z+5XhvDpdLEhRi8PWw6w5RnEZ0EtWN3fz0jgKNMRpkQOok3poP5ESI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1750801862; c=relaxed/simple; bh=+cbatE9G+lJjS4b3D7xitCCCE1uL+kQqBfGu6f9xnLU=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=wJOZHgKnfRXdFGbAbNXn9lU0F6hOVMcA2lTvrdABCD/L2rbtmiqgXBc7+yIn7C1kB4++RYi5W70yGsY8VC1U94GjsfHBE2M/Y1eqWgXBJED9hiS7K050xN1E9LarPmKe3SbYBnroH2CAivyqL6IPrG81GuckPizN09lMQz6BK4o= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7B0253858429 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1750801862; 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: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=s+CUZkYExgrJSALh1HZ4ra/l+f4+kMPPctpMC44IgQY=; b=Zb+/Ex4WGAj0LdBzAuH9zi8JmoeS427bxEG1TuG1UK+DGR0YSqXo5L+YWKKx87uALx7/xl bsWKacQHZcs84KSluHCsR6xSqM3BbuvTS8bZ1Lj2PQGWKjxGVVcp5MjCNohOj3X/TsHVxR gsjeaiRZdx0DmfnYHW+Xgjy8LWUmyw4= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-641-fxSwltXGM3GrNhKed9EpBw-1; Tue, 24 Jun 2025 17:51:01 -0400 X-MC-Unique: fxSwltXGM3GrNhKed9EpBw-1 X-Mimecast-MFC-AGG-ID: fxSwltXGM3GrNhKed9EpBw_1750801860 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id C07F71956088; Tue, 24 Jun 2025 21:50:59 +0000 (UTC) Received: from f41-zbm-amd (unknown [10.22.80.61]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id A788719560A3; Tue, 24 Jun 2025 21:50:56 +0000 (UTC) Date: Tue, 24 Jun 2025 14:50:52 -0700 From: Kevin Buettner To: Andrew Burgess Cc: gdb-patches@sourceware.org, Michael Weghorn , Eli Zaretskii , Guinevere Larsen Subject: Re: [PATCH] gdb/gdbserver: pass inferior arguments as a single string Message-ID: <20250624145052.4659642b@f41-zbm-amd> In-Reply-To: References: Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: NKiiJZInx0o_JhHbEnjT2V3oTK00cEHJdQGCEE6EACo_1750801860 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 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