Re: [PATCH] gdb/gdbserver: pass inferior arguments as a single string

[Luis <luis.machado.foss@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 6921 bytes --]

I think I'm running into a change in behavior for --args, where it now says
no program was specified when previously it was ok.

Just a standard....

./gdb/gdb -ex run --args gdb/gdb gdb/gdb -ex start

Did something change in terms of invocation format?

On Fri, Sep 12, 2025, 11:13 Andrew Burgess <aburgess@redhat.com> wrote:

> Kevin Buettner <kevinb@redhat.com> writes:
>
> > Hi Andrew,
> >
> > I have a few commit log nits - see (inline) below.
> >
> > On Tue, 13 May 2025 11:31:04 +0100
> > Andrew Burgess <aburgess@redhat.com> 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 <m.weghorn@posteo.de>
> >> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392
> >>
> >> Reviewed-By: Eli Zaretskii <eliz@gnu.org>
> >> Tested-By: Guinevere Larsen <guinevere@redhat.com>
> >
> > Thanks for the detailed explanation.  This makes sense to me.
> >
> > I also tested this patch and found no problems.
> >
> > Approved-by: Kevin Buettner <kevinb@redhat.com>
>
> Thanks for the feedback.  I made these changes and pushed this patch.
>
> Thanks,
> Andrew
>
>

[-- Attachment #2: Type: text/html, Size: 9603 bytes --]