[Bug server/28392] New: gdb server no longer supports argument globbing and variable substitution

[Bug server/28392] New: gdb server no longer supports argument globbing and variable substitution

[nikita6 at bk dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

            Bug ID: 28392
           Summary: gdb server no longer supports argument globbing and
                    variable substitution
           Product: gdb
           Version: HEAD
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: server
          Assignee: unassigned at sourceware dot org
          Reporter: nikita6 at bk dot ru
  Target Milestone: ---

Due to change in the way arguments are handled by gdbserver (bisected as commit
bea571ebd78ee29cb94adf648fbcda1e109e1be6), it is no longer possible to do shell
variable substitution, globbing and inferior output redirection via command
line.

Steps to reproduce:
1) Run gdbserver
gdbserver --startup-with-shell localhost:5006 ls ">test.txt"

2) (in another terminal) Connect to it with gdb
gdb --eval-command='target remote localhost:5006' --eval-command='c'
--eval-command='q'

Before the offending commit you would get:

Process ls created; pid = 2060713
Listening on port 5006
Remote debugging from host 127.0.0.1, port 48496

Child exited with status 0


and a file test.txt created

After the offending commit you would get:

Process ls created; pid = 2060582
Listening on port 5006
Remote debugging from host 127.0.0.1, port 48494
ls: cannot access '>test.txt': No such file or directory

Child exited with status 2


It appears that the commit was trying to fix PR 25893, which was about handling
arguments with spaces. It appears that it was too restrictive about escaping
though, not allowing the usage of any of the shell features

Those features are actually documented in --no-startup-with-shell flag help:

> Exec PROG directly instead of using a shell.
> Disables argument globbing and variable substitution
> on UNIX-like systems.

(with the commit applied and flag --startup-with-shell used this does not
work).

The problem was discovered when using bazel extension for CLion (tracked at
https://github.com/bazelbuild/intellij/issues/2958). The plugin executes
gdbserver, redirecting gdbserver's and interior's outputs to different files.
Due to changed parsing inferior gets something like
`1>/tmp/gdbserver_wrapper.KxJr27Av/inferior.out
2>/tmp/gdbserver_wrapper.KxJr27Av/inferior.err` as arguments when using gdb
10.2, which is.. Inconvenient to say the least.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[nikita6 at bk dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

Nikita Strygin <nikita6 at bk dot ru> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |nikita6 at bk dot ru

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

Simon Marchi <simark at simark dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |m.weghorn at posteo dot de,
                   |                            |simark at simark dot ca

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[m.weghorn at posteo dot de]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #1 from Michael Weghorn <m.weghorn at posteo dot de> ---
Thanks for the detailed report. I have to admit I was not aware of the features
my commit broke, nor the '--no-startup-with-shell' arg for gdbserver.

Some thoughts/observations:

* Args with spaces still don't work when using the '--no-startup-with-shell'
arg for gdbserver. (Before bea571ebd78ee29cb94adf648fbcda1e109e1be6, such args
were silently split into multiple args as described in PR 25893, now gdbserver
exits saying "can't handle command-line argument containing whitespace".) That
should IMHO be fixed independently of the case where 'startup-with-shell' is
enabled.

* What gdb essentially does on UNIX-like systems when 'startup-with-shell' is
enabled is to run the program in a shell using "<SHELL> -c '<EXEC> <ARGS>'".

* "Plain" gdb by default has "startup-with-shell" enabled, but it still escapes
special characters in args passed when running 'gdb --args <ARGS>', as
gdbserver does now after bea571ebd78ee29cb94adf648fbcda1e109e1be6 ("plain" GDB
also uses 'construct_inferior_arguments', s. 'set_inferior_args_vector' in
gdb/infcmd.c).
Features (described in "startup-with-shell" section at [1]) like variable
substitution, expanding wildcard characters and I/O redirection in the shell
only apply for args set in the GDB prompt or via the '--eval-command' arg, e.g.
using 'run <PROGRAM> <ARGS>' or 'set args <ARGS>'.

* So far, I see no way to make both, the features described here as well as the
case with spaces in arguments (as described in PR 25893) work at the same time
for args passed on the command line when 'startup-with-shell' is enabled. From
how I understand it, just escaping specific characters wouldn't help. E.g. when
only escaping spaces, an
'>out.txt 2>err.txt' would then still not work as previously/expected, would
behave like an unescaped '>"out.txt 2" >"err.txt"'.

Given that, the most reasonable approach I can currently come up with to bring
back globbing, variable substitution and I/O redirection in the shell:

* for the case where 'startup-with-shell' is enabled, change back to the
behaviour before commit bea571ebd78ee29cb94adf648fbcda1e109e1be6

    * accept that gdbserver and "plain" gdb treat args passed via command line
differently by default (i.e. when '--no-startup-with-shell' is not explicitly
set for gdbserver)

    * given that args are passed to the shell in an unmodified way, the
scenario from PR 25893 is then possibly supposed to be regarded as a feature,
not a bug...

* As opposed to the expectation in PR 25893, args with spaces and special
characters then need to be escaped/quoted by the user when startup-with-shell
is enabled for gdbserver, e.g. by running

    gdbserver localhost:50505 ./myprogram '"hello world"'

or

    gdbserver localhost:50505 ./myprogram "hello\ world"

instead of just

    gdbserver localhost:50505 ./myprogram "hello world"

* when using the '--no-startup-with-shell' arg for gdbserver, no extra escaping
needs to be done by the user, i.e. the following should work and result in a
single "hello world" arg being passed to the inferior:

    gdbserver --no-startup-with-shell localhost:50505 ./myprogram "hello world"


What do you think? Any thoughts or better ideas how to deal with the situation
are highly appreciated.


[1] https://sourceware.org/gdb/onlinedocs/gdb/Starting.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[nikita6 at bk dot ru]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #2 from Nikita Strygin <nikita6 at bk dot ru> ---
I think your solution sounds reasonable: it will fix the
'--no-startup-with-shell' with spaces and keep the compatibility for
'--startup-with-shell'

I think it might also be beneficial to update the docs to make clear what is
the difference. Currently it only says that 'startup-with-shell off' might be
useful when shell crashes for some reason.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[m.weghorn at posteo dot de]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #3 from Michael Weghorn <m.weghorn at posteo dot de> ---
@Simon: Do you think the approach mentioned in comment 1 is reasonable?

If so, I'll work on implementing that (presumably sometime next week).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[simon.marchi at polymtl dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

Simon Marchi <simon.marchi at polymtl dot ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |simon.marchi at polymtl dot ca

--- Comment #4 from Simon Marchi <simon.marchi at polymtl dot ca> ---
I don't really know what's best here.  I just tried your different examples to
get a feel of the problem, but I don't have a definite answer.  It seems
difficult to reconcile all use cases.

I think it's good if GDB and GDBserver have the same behavior.  It's also good
if that behavior is predictable.  I think we have to choose the behavior that
makes the most sense, make sure it works the same in GDB and GDBserver, and
stick to it / make sure it is properly tested.  Whatever we choose, I think
some people will be unhappy (will see a change of behavior in their use case).

Disregarding things work today, I think makes sense that when using
startup-with-shell, the arguments are passed unchanged to the shell (although
concatenated with spaces if more than one is given).  Otherwise, the option to
run through a shell is kind of pointless.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[m.weghorn at posteo dot de]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #5 from Michael Weghorn <m.weghorn at posteo dot de> ---
(In reply to Michael Weghorn from comment #1)
> * "Plain" gdb by default has "startup-with-shell" enabled, but it still
> escapes special characters in args passed when running 'gdb --args <ARGS>',
> as gdbserver does now after bea571ebd78ee29cb94adf648fbcda1e109e1be6
> ("plain" GDB also uses 'construct_inferior_arguments', s.
> 'set_inferior_args_vector' in gdb/infcmd.c).
> Features (described in "startup-with-shell" section at [1]) like variable
> substitution, expanding wildcard characters and I/O redirection in the shell
> only apply for args set in the GDB prompt or via the '--eval-command' arg,
> e.g. using 'run <PROGRAM> <ARGS>' or 'set args <ARGS>'.

Note that for gdbserver (as opposed to how it works for plain gdb), args set
via 'run <PROGRAM> <ARGS>' or 'set args <ARGS>' are now escaped as well (s.a.
PR 28446).
Here, it seems clear to me that no escaping should happen, so the remaining
question is how to treat args passed directly on gdb/gdbserver invocation.

(In reply to Simon Marchi from comment #4)
> I think it's good if GDB and GDBserver have the same behavior.  It's also
> good if that behavior is predictable.  I think we have to choose the
> behavior that makes the most sense, make sure it works the same in GDB and
> GDBserver, and stick to it / make sure it is properly tested.  Whatever we
> choose, I think some people will be unhappy (will see a change of behavior
> in their use case).
> 
> Disregarding things work today, I think makes sense that when using
> startup-with-shell, the arguments are passed unchanged to the shell
> (although concatenated with spaces if more than one is given).  Otherwise,
> the option to run through a shell is kind of pointless.

If, other than my original suggestion on comment 1, we want to have gdbserver
and gdb treat args passed directly on invocation ('gdb --args <ARGS>' or
'gdbserver <ARGS>') the same, I essentially see two options:

1) Don't escape args in either 'gdb --args <ARGS>' nor 'gdbserver <ARGS>', i.e.
revert to the previous behaviour in gdbserver and change the way that gdb
behaves, requiring that arguments passed via 'gdb --args <ARGS>' are escaped by
the user as needed (as used to be the case for gdbserver before commit
bea571ebd78ee29cb94adf648fbcda1e109e1be6).

2) Escape args for both 'gdb --args <ARGS>' and 'gdbserver <ARGS>' by default,
but add a new option for gdbserver to disable that (e.g. '--no-escape-args'),
so that the gdbserver behaviour can explicitly be switched to the one before
commit bea571ebd78ee29cb94adf648fbcda1e109e1be6. 'gdb --args' behaviour remains
unchanged in that case.

If we want to unify this, I currently tend to prefer option 2). I personally
appreciate that just prepending a "gdb --args" to the usual program call
without having to think about any escaping is nice.
At least for "simple" use cases where gdb is just invoked from a shell, I
*think* it often doesn't even make a huge difference whether globbing and
variable substitution (as opposed to I/O redirection) is already done by the
shell from which gdb is invoked.

E.g. if I run this in a shell for option 1)

    gdb --args ./myprogram $SHELL

it will finally run './myprogram /bin/bash' in gdb (in my case where
SHELL=/bin/bash), just as is the case for this command for option 2):

    gdb --args ./myprogram '$SHELL'

just that variable expansion happens in the shell before the args are passed to
gdb (case 1) as opposed to it being done by the shell invoked by GDB (case 2).
However, there are certainly more complex scenarios (I/O redirection in any
case, but also others where env variables are manually set before execution or
work directory is changed,...) where it does make a difference in practice.
However, at least for gdb, it's already possible to set unescaped args using
'--eval-command='set args <ARGS>' if necessary.

I'd guess that changing the way that 'gdb --args' behaves (option 1) would be a
"more breaking change" than changing gdbserver (option 2).
But honestly, I don't know what's the way that most people use gdb and you can
probably judge that in a much better way and will be happy to work on the
implementation that is considered most reasonable.

What do you think?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[simon.marchi at polymtl dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #6 from Simon Marchi <simon.marchi at polymtl dot ca> ---
(In reply to Michael Weghorn from comment #5)
> (In reply to Michael Weghorn from comment #1)
> > * "Plain" gdb by default has "startup-with-shell" enabled, but it still
> > escapes special characters in args passed when running 'gdb --args <ARGS>',
> > as gdbserver does now after bea571ebd78ee29cb94adf648fbcda1e109e1be6
> > ("plain" GDB also uses 'construct_inferior_arguments', s.
> > 'set_inferior_args_vector' in gdb/infcmd.c).
> > Features (described in "startup-with-shell" section at [1]) like variable
> > substitution, expanding wildcard characters and I/O redirection in the shell
> > only apply for args set in the GDB prompt or via the '--eval-command' arg,
> > e.g. using 'run <PROGRAM> <ARGS>' or 'set args <ARGS>'.
> 
> Note that for gdbserver (as opposed to how it works for plain gdb), args set
> via 'run <PROGRAM> <ARGS>' or 'set args <ARGS>' are now escaped as well
> (s.a. PR 28446).
> Here, it seems clear to me that no escaping should happen, so the remaining
> question is how to treat args passed directly on gdb/gdbserver invocation.

Ok.

> If, other than my original suggestion on comment 1, we want to have
> gdbserver and gdb treat args passed directly on invocation ('gdb --args
> <ARGS>' or 'gdbserver <ARGS>') the same, I essentially see two options:
> 
> 1) Don't escape args in either 'gdb --args <ARGS>' nor 'gdbserver <ARGS>',
> i.e. revert to the previous behaviour in gdbserver and change the way that
> gdb behaves, requiring that arguments passed via 'gdb --args <ARGS>' are
> escaped by the user as needed (as used to be the case for gdbserver before
> commit bea571ebd78ee29cb94adf648fbcda1e109e1be6).
> 
> 2) Escape args for both 'gdb --args <ARGS>' and 'gdbserver <ARGS>' by
> default, but add a new option for gdbserver to disable that (e.g.
> '--no-escape-args'), so that the gdbserver behaviour can explicitly be
> switched to the one before commit bea571ebd78ee29cb94adf648fbcda1e109e1be6.
> 'gdb --args' behaviour remains unchanged in that case.
> 
> If we want to unify this, I currently tend to prefer option 2). I personally
> appreciate that just prepending a "gdb --args" to the usual program call
> without having to think about any escaping is nice.

By that argument, the current behavior of GDB, to escape arguments from the
command line, is right then.  This program prints the arguments it receives:

$ ./a.out '$SHELL'
[0] ./a.out
[1] $SHELL
$ ./gdb -nx -q --data-directory=data-directory -batch -ex r --args ./a.out
'$SHELL'
[0] /home/smarchi/build/binutils-gdb/gdb/a.out
[1] $SHELL

$ ./a.out "> out"       
[0] ./a.out
[1] > out
$ ./gdb -nx -q --data-directory=data-directory -batch -ex r --args ./a.out '>
out' 
[0] /home/smarchi/build/binutils-gdb/gdb/a.out
[1] > out

Imagine if GDB didn't escape the arguments, in that last example the "> out"
would end up interpreted as SHELL redirection (unlike the non-GDB invokation),
it would be surprising I think.

> At least for "simple" use cases where gdb is just invoked from a shell, I
> *think* it often doesn't even make a huge difference whether globbing and
> variable substitution (as opposed to I/O redirection) is already done by the
> shell from which gdb is invoked.
> 
> E.g. if I run this in a shell for option 1)
> 
>     gdb --args ./myprogram $SHELL
> 
> it will finally run './myprogram /bin/bash' in gdb (in my case where
> SHELL=/bin/bash), just as is the case for this command for option 2):
> 
>     gdb --args ./myprogram '$SHELL'



> 
> just that variable expansion happens in the shell before the args are passed
> to gdb (case 1) as opposed to it being done by the shell invoked by GDB
> (case 2). However, there are certainly more complex scenarios (I/O
> redirection in any case, but also others where env variables are manually
> set before execution or work directory is changed,...) where it does make a
> difference in practice.
> However, at least for gdb, it's already possible to set unescaped args using
> '--eval-command='set args <ARGS>' if necessary.

Indeed, with GDB you can fall back on "set args".  But with gdbserver, you
can't, and that might be a problem.  So having a --no-escape-args could be a
good option.  And if we are adding it to gdbserver, we might as well add it to
gdb as well. i.e:

  $ ./gdb ... --args ./a.out '$SHELL'

would show '$SHELL', and

  $ ./gdb ... --no-escape-args --args ./a.out '$SHELL'

would show '/bin/bash' or whatever.

> I'd guess that changing the way that 'gdb --args' behaves (option 1) would
> be a "more breaking change" than changing gdbserver (option 2).
> But honestly, I don't know what's the way that most people use gdb and you
> can probably judge that in a much better way and will be happy to work on
> the implementation that is considered most reasonable.
> 
> What do you think?

I would probably lean towards option 1:

 - GDB has worked this way for a while, and it would probably break the
workflow of more users than changing gdbserver
 - Given the examples above, I think the GDB behavior is somewhat sane
 - The "new" gdbserver behavior is now released in the wild, in gdbserver 11.1
(IIUC).  And it's what is in master today.  We could always revert to the old
behavior as a "bug fix" in gdbserver 11.2, but that might end up even more
confusing.  So where we are standing today, we can see the change in
gdbserver's behavior as being on purpose (to align gdbserver's behavior with
gdb's), but add --no-escape-args to provide an escape hatch.

There would be a third option, to revert gdbserver's behavior to be different
than gdb's, and add an --escape-args switch.  But I don't really like that, as
that makes gdb and gdbserver behave differently, whereas we try to make the two
converge.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[m.weghorn at posteo dot de]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #7 from Michael Weghorn <m.weghorn at posteo dot de> ---
> I would probably lean towards option 1:
> 
>  - GDB has worked this way for a while, and it would probably break the
> workflow of more users than changing gdbserver
>  - Given the examples above, I think the GDB behavior is somewhat sane
>  - The "new" gdbserver behavior is now released in the wild, in gdbserver
> 11.1 (IIUC).  And it's what is in master today.  We could always revert to
> the old behavior as a "bug fix" in gdbserver 11.2, but that might end up
> even more confusing.  So where we are standing today, we can see the change
> in gdbserver's behavior as being on purpose (to align gdbserver's behavior
> with gdb's), but add --no-escape-args to provide an escape hatch.
> 
> There would be a third option, to revert gdbserver's behavior to be
> different than gdb's, and add an --escape-args switch.  But I don't really
> like that, as that makes gdb and gdbserver behave differently, whereas we
> try to make the two converge.

Thanks a lot for you input.
Unless I completely misunderstand something, I'd read that as being in favour
of what I tried to describe as option **2** in comment 5, though, right? (not
option 1)

If so, I'll try to come up with patches to implement that.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[simark at simark dot ca]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #8 from Simon Marchi <simark at simark dot ca> ---
> Thanks a lot for you input.
> Unless I completely misunderstand something, I'd read that as being in
> favour of what I tried to describe as option **2** in comment 5, though,
> right? (not option 1)
> 
> If so, I'll try to come up with patches to implement that.

That's right.  Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[m.weghorn at posteo dot de]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #9 from Michael Weghorn <m.weghorn at posteo dot de> ---
(In reply to Michael Weghorn from comment #7)
> If so, I'll try to come up with patches to implement that.

First version of the patches just sent to the mailing list:
https://sourceware.org/pipermail/gdb-patches/2021-October/182723.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vries at gcc dot gnu.org

--- Comment #10 from Tom de Vries <vries at gcc dot gnu.org> ---
Latest state:
https://inbox.sourceware.org/gdb-patches/cover.1704809585.git.aburgess@redhat.com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #11 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f56707caa021dbe9a6e68aaa2dd0b13ba01f53df

commit f56707caa021dbe9a6e68aaa2dd0b13ba01f53df
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Fri Dec 15 13:03:26 2023 +0000

    gdb/testsuite: add some xfail in gdb.base/startup-with-shell.exp

    There are two tests that fail in gdb.base/startup-with-shell.exp when
    using the native-extended-remote board.  I plan to fix these issues,
    and I've posted a series that does just that:

     
https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com

    But until that series is reviewed, I thought I'd merge this commit,
    which marks the FAIL as XFAIL and links them to the relevant bug
    number.

    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392

    Tested-By: Guinevere Larsen <guinevere@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #12 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ac8f3fc9330da0302ebb491bf2bac8da5e035e35

commit ac8f3fc9330da0302ebb491bf2bac8da5e035e35
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Fri Nov 3 21:40:29 2023 +0000

    gdbserver: convert program_args to a single string

    This commit changes how gdbserver stores the inferior arguments from
    being a vector of separate arguments into a single string with all of
    the arguments combined together.

    Making this change might feel a little strange; intuitively it feels
    like we would be better off storing the arguments as a vector, but
    this change is part of a larger series of work that aims to improve
    GDB's inferior argument handling.  The full series was posted here:

     
https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com

    But asking people to review a 14 patch series in unreasonable, so I'm
    instead posting the patches in smaller batches.  This patch can stand
    alone, and I do think this change makes sense on its own:

    First, GDB already stores the inferior arguments as a single string,
    so doing this moves gdbserver into line with GDB.  The common code
    into which gdbserver calls requires the arguments to be a single
    string, so currently each target's create_inferior implementation
    merged the arguments anyway, so all this commit really does is move
    the merging up the call stack, and store the merged result rather than
    storing the separate parts.

    However, the biggest reason for why this commit is needed, is an issue
    with passing arguments from GDB to gdbserver when starting a new
    inferior.

    Consider:

      (gdb) set args $VAR
      (gdb) run
      ...

    When using a native target the inferior will see the value of $VAR
    expanded by the shell GDB uses to start the inferior.  However, if
    using an extended-remote target the inferior will see literally $VAR,
    the unexpanded name of the variable, the reason for this is that,
    although GDB sends '$VAR' to gdbserver, when gdbserver receives this,
    it converts this to '\$VAR', which prevents the variable from being
    expanded by the shell.

    The reason for this is that construct_inferior_arguments escapes all
    special shell characters within its arguments, and it is
    construct_inferior_arguments that is used to combine the separate
    arguments into a single string.

    In the future I will change construct_inferior_arguments so that
    it can apply different escaping strategies.  When this happens we will
    want to escape arguments coming from the gdbserver command line
    differently than arguments coming from GDB (via a vRun packet), which
    means we need to call construct_inferior_arguments earlier, at the
    point where we know if the arguments came from the gdbserver command
    line, or from the vRun packet.

    This argument escaping issue is discussed in PR gdb/28392.

    This commit doesn't fix any issues, nor does it change
    construct_inferior_arguments to actually do different escaping, that
    will all come later.  This is purely a restructuring.

    There should be no user visible changes after this commit.

    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392

    Tested-By: Guinevere Larsen <guinevere@redhat.com>
    Approved-By: Simon Marchi <simon.marchi@efficios.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #13 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0df62bf09ecf242e3a932255d24ee54407b3c593

commit 0df62bf09ecf242e3a932255d24ee54407b3c593
Author: Michael Weghorn <m.weghorn@posteo.de>
Date:   Fri Oct 22 07:19:33 2021 +0000

    gdb: Support some escaping of args with startup-with-shell being off

    I (Andrew Burgess) have taken this patch from this series:

     
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/

    I started off reviewing that series, but wanted to explore some
    alternative strategies for solving the problems this series addresses.
    However, this patch I think is super useful, so I've taken it mostly
    as it was in the original series.

    I have made a few minor cleanups, and I've also added some more tests.
    Any bugs should be considered mine (Andrew's), but I've left the
    original author (Michael Weghorn) in place as the GDB side changes are
    mostly their work.

    The function execv_argv::init_for_no_shell (gdb/nat/fork-inferior.c),
    is passed a single string ALLARGS containing all of the inferior
    arguments, and contains some custom code for splitting this argument
    string into a vector of separate arguments.  This function is used
    when startup-with-shell is off (which is not the default).

    The algorithm in this function was just splitting on whitespace
    characters, and ignoring any quoting, so for example:

        (gdb) set startup-with-shell off
        (gdb) set args "first arg" second_arg

    would result in three arguments ("first), (arg"), and (second_arg)
    being passed to the inferior (the parenthesis are not part of the
    parsed arguments).

    This commit replaces this custom argument splitting with a use of the
    existing gdb_argv class (which uses the libiberty buildargv function).
    This does a better job of supporting quoting and escaping, so for the
    example given above we now pass two arguments (first arg)
    and (second_arg), which is certainly what I would have expected as a
    GDB user.

    This commit changes the 'execv_argv' class accordingly and drops the
    optimization to have all the 'char *' in 'm_argv' point to a single
    string rather than allocating a separate string for each arg.  This is
    needed because we are now going to be stripping some escaping from the
    arguments, for example:

        (gdb) set startup-with-shell off
        (gdb) set args "literal \$"

    In this case we will pass the single argument (literal $) to the
    inferior, the escaping backslash will be removed.  This might seem
    strange as usually the backslash would be stripped by the shell, and
    now we have no shell.  However, I think the consistent behaviour is a
    good thing; whether we start with a shell or not the escaping will be
    removed.

    Using gdb_argv will mean that quote characters are also stripped.  If
    we consider the first example again:

        (gdb) set startup-with-shell off
        (gdb) set args "first arg" second_arg

    This is now going to pass (first arg) and (second_arg), the quotes
    have been removed.  If the user did want the original behaviour then
    they are going to have to now do this:

        (gdb) set startup-with-shell off
        (gdb) set args \"first arg\" second_arg

    or they could do this:

        (gdb) set startup-with-shell off
        (gdb) set args '"first' 'arg"' second_arg

    This commit also extends the three tests that cover inferior argument
    passing to cover the case where 'startup-with-shell' is off.  All of
    these new tests pass for native targets, but there are still problems
    when using remote targets.

    The remote target problems arise because of how escaping is handled
    while passing arguments to remote targets.  I have a larger series
    that aims to address this issue:

     
https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com

    This patch was originally part of that series, but getting a 14 patch
    series reviewed is not easy, so I've pulled this patch out on its own
    for now, and the new tests are (rather crudely) disabled for remote
    targets.

    My hope is to work through my 14 patch series posting all of the
    patches in smaller groups, which will hopefully make reviewing
    easier.  As more of that series gets merged, the remote argument
    handling will improve, before, eventually, no tests will need to be
    disabled.

    Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392

    Tested-By: Guinevere Larsen <guinevere@redhat.com>
    Reviewed-By: Keith Seitz <keiths@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #14 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=710f7df7da31dc6b81cdbdd7256005c51798260a

commit 710f7df7da31dc6b81cdbdd7256005c51798260a
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Fri Dec 15 12:46:14 2023 +0000

    gdb: remove the !startup_with_shell path from construct_inferior_arguments

    In the commit:

      commit 0df62bf09ecf242e3a932255d24ee54407b3c593
      Date:   Fri Oct 22 07:19:33 2021 +0000

          gdb: Support some escaping of args with startup-with-shell being off

    nat/fork-inferior.c was updated such that when we are starting an
    inferior without a shell we now remove escape characters.  The
    benefits of this are explained in that commit, but having made this
    change we can now make an additional change.

    Currently, in construct_inferior_arguments, when startup_with_shell is
    false we construct the inferior argument string differently than when
    startup_with_shell is true; when true we apply some escaping to
    special shell character, when false we don't.

    This commit simplifies construct_inferior_arguments by removing the
    !startup_with_shell case, and instead we now apply escaping in all
    cases.  This is fine because, thanks to the above commit the escaping
    will be correctly removed again when we call into nat/fork-inferior.c.

    We should think of construct_inferior_arguments and
    nat/fork-inferior.c as needing to cooperate in order for argument
    handling to work correctly.

    construct_inferior_arguments converts a list of separate arguments
    into a single string, and nat/fork-inferior.c splits that single
    string back into a list of arguments.  It is critical that, if
    nat/fork-inferior.c is expecting to remove a "layer" of escapes, then
    construct_inferior_arguments must add that expected "layer",
    otherwise, we end up stripping more escapes than expected.

    The great thing (I think) about the new configuration, is that GDB no
    longer cares about startup_with_shell at the point the arguments are
    being setup.  We only care about startup_with_shell at the point that
    the inferior is started.  This means that a user can set the inferior
    arguments, and then change the startup-with-shell setting, and GDB
    will do what they expect.

    Under the previous system, where construct_inferior_arguments changed
    its behaviour based on startup_with_shell, the user had to change the
    setting, and then set the arguments, otherwise, GDB might not do what
    they expect.

    There is one slight issue with this commit though, which will be
    addressed by the next commit.

    For GDB's native targets construct_inferior_arguments is reached via
    two code paths; first when GDB starts and we combine arguments from
    the command line, and second when the Python API is used to set the
    arguments from a sequence.  It's the command line argument handling
    which we are interested in.

    Consider this:

      $ gdb --args /tmp/exec '$FOO'
      (gdb) show args
      Argument list to give program being debugged when it is started is
"\$FOO".

    Notice that the argument has become \$FOO, the '$' is now quoted.

    This is because, by quoting the argument in the shell command that
    started GDB, GDB was passed a literal $FOO with no quotes.  In order
    to ensure that the inferior sees this same value, GDB added the extra
    escape character.  When GDB starts with a shell we pass \$FOO, which
    results in the inferior seeing a literal $FOO.

    But what if the user _actually_ wanted to have the shell GDB uses to
    start the inferior expand $FOO?  Well, it appears this can't be done
    from the command line, but from the GDB prompt we can just do:

      (gdb) set args $FOO
      (gdb) show args
      Argument list to give program being debugged when it is started is
"$FOO".

    And now the inferior will see the shell expanded version of $FOO.

    It might seem like we cannot achieve the same result from the GDB
    command line, however, it is possible with this trick:

      $ gdb -eiex 'set startup-with-shell off' --args /tmp/exec '$FOO'
      (gdb) show args
      Argument list to give program being debugged when it is started is
"$FOO".
      (gdb) show startup-with-shell
      Use of shell to start subprocesses is off.

    And now the $FOO is not escaped, but GDB is no longer using a shell to
    start the inferior, however, we can extend our command line like this:

      $ gdb -eiex 'set startup-with-shell off' \
            -ex 'set startup-with-shell on' \
            --args /tmp/exec '$FOO'
      (gdb) show args
      Argument list to give program being debugged when it is started is
"$FOO".
      (gdb) show startup-with-shell
      Use of shell to start subprocesses is on.

    Use an early-initialisation option to disable startup-with-shell, this
    is done before command line argument processing, then a normal
    initialisation option turns startup-with-shell back on after GDB has
    processed the command line arguments!

    Is this useful?  Yes, absolutely.  Is this a good user experience?
    Absolutely not.  And I plan to add a new command line option to
    GDB (and gdbserver) that will allow users to achieve the same
    result (this trick doesn't work in gdbserver as there's no
    early-initialisation there) without having to toggle the
    startup-with-shell option.  The new option can be found in the series
    here:

     
https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com

    The problem is that, that series is pretty long, and getting it
    reviewed is just not possible.  So instead I'm posting the individual
    patches in smaller blocks, to make reviews easier.

    So, what's the problem?  Well, by removing the !startup_with_shell
    code path from GDB, there is no longer a construct_inferior_arguments
    code path that doesn't quote inferior arguments, and so there's no
    longer a way, from the command line, to set an unquoted '$FOO' as an
    inferior argument.  Obviously, this can still be done from GDB's CLI
    prompt.

    The trick above is completely untested, so this regression isn't going
    to show up in the testsuite.

    And the breakage is only temporary.  In the next commit I'll add a fix
    which restores the above trick.

    Of course, I hope that this fix will itself, only be temporary.  Once
    the new command line options that I mentioned above are added, then
    the fix I add in the next commit can be removed, and user should start
    using the new command line option.

    After this commit a whole set of tests that were added as xfail in the
    above commit are now passing.

    A change similar to this one can be found in this series:

     
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/

    which I reviewed before writing this patch.  I don't think there's any
    one patch in that series that exactly corresponds with this patch
    though, so I've listed the author of the original series as co-author
    on this patch.

    Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392

    Tested-By: Guinevere Larsen <guinevere@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #15 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=709aefc274a2a7bd67a0f1a5ed09cac8974367fc

commit 709aefc274a2a7bd67a0f1a5ed09cac8974367fc
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Nov 9 15:28:49 2023 +0000

    gdb: add remote argument passing unit tests

    This commit adds some remote argument passing unit tests.  There are
    not many tests right now -- there are known bugs in the remote
    argument passing mechanism (see PR gdb/28392) -- but some simple cases
    are covered here, and I plan to add additional tests once I've fixed
    more of the problems with the existing argument handling code.

    The tests take an inferior argument string, this is the string that
    GDB would carry around as inferior::m_args.  This string is then split
    using gdb::remote_args::split, this gives a vector of strings, these
    are the strings that are passed over the remote protocol.  These split
    strings are validated as part of the test.

    The split strings are then combined using gdb::remote_args::join which
    gives the inferior argument string that gdbserver will use, this is
    held in server.cc as program_args, this joined string is then checked
    as part of the test.

    There are no changes to GDB's behaviour as part of this commit, other
    than adding the new tests which can be run with:

      (gdb) maintenance selftest remote-args
      Running selftest remote-args.
      Ran 1 unit tests, 0 failed

    Tested-By: Guinevere Larsen <guinevere@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #16 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=e5e76451fa82e0bc00599af96382b361c3d6ac32

commit e5e76451fa82e0bc00599af96382b361c3d6ac32
Author: Michael Weghorn <m.weghorn@posteo.de>
Date:   Fri Oct 22 07:19:29 2021 +0000

    gdb/gdbserver: add a '--no-escape-args' command line option

    This introduces a new '--no-escape-args' option for gdb and gdbserver.

    I (Andrew Burgess) have based this patch from work done in this
    series:

     
https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de/

    I have changed things slightly from the original series.  I think this
    work is close enough that I've left the original author (Michael) in
    place and added myself as co-author.  Any bugs introduced by my
    modifications to the original patch should be considered mine.  I've
    also added documentation and tests which were missing from the
    originally proposed patch.

    When the startup-with-shell option is enabled, arguments passed
    directly as 'gdb --args <args>' or 'gdbserver <args>', are by default
    escaped so that they are passed to the inferior as passed on the
    command line, no globbing or variable substitution happens within the
    shell GDB uses to start the inferior.

    For gdbserver, this is the case since commit:

      commit bea571ebd78ee29cb94adf648fbcda1e109e1be6
      Date:   Mon May 25 11:39:43 2020 -0400

          Use construct_inferior_arguments which handles special chars

    Only arguments set via 'set args <args>', 'run <args>', or through the
    Python API are not escaped in standard upstream GDB right now.

    For the 'gdb --args' case, directly setting unescaped args on gdb
    invocation is possible e.g. by using the "--eval-command='set args
    <args>'", while this possibility does not exist for gdbserver.

    This commit adds a new '--no-escape-args' command line option for GDB
    and gdbserver.  This option is used with GDB as a replacement for the
    current '--args' option, and for gdbserver this new option is a flag
    which changes how gdbserver handles inferior arguments on the command
    line.  When '--no-escape-args' is used inferior arguments passed on
    the command line will not have escaping added by GDB or gdbserver.

    For gdbserver, using this new option allows having the behaviour from
    before commit bea571ebd78ee29cb94adf648fbcda1e109e1be6, while keeping
    the default behaviour unified between GDB and GDBserver.

    For GDB the --no-escape-args option can be used as a replacement for
    --args, like this:

      shell> gdb --no-escape-args my-program arg1 arg2 arg3

    While for gdbserver, the --no-escape-args option is a flag, which can
    be used like:

      shell> gdbserver --no-escape-args --once localhost:54321 \
                 my-program arg1 arg2 arg3

    Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392

    Reviewed-By: Eli Zaretskii <eliz@gnu.org>
    Tested-By: Guinevere Larsen <guinevere@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

--- Comment #17 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Andrew Burgess <aburgess@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8e28eef6cdcbd86ad61325ce1e6bd563b0fad1e1

commit 8e28eef6cdcbd86ad61325ce1e6bd563b0fad1e1
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Nov 23 18:46:54 2023 +0000

    gdb/gdbserver: pass inferior arguments as a single string

    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 that can ever expand
    $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 improve remote argument splitting.  I think
    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 that doesn't really address
    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.

    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392

    Co-Authored-By: Michael Weghorn <m.weghorn@posteo.de>
    Reviewed-By: Eli Zaretskii <eliz@gnu.org>
    Tested-By: Guinevere Larsen <guinevere@redhat.com>
    Approved-by: Kevin Buettner <kevinb@redhat.com>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug server/28392] gdb server no longer supports argument globbing and variable substitution

[aburgess at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=28392

Andrew Burgess <aburgess at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
                 CC|                            |aburgess at redhat dot com
         Resolution|---                         |FIXED

--- Comment #18 from Andrew Burgess <aburgess at redhat dot com> ---
I think this issue is now fixed in the master branch.  You will need to start
gdbserver using the new --no-escape-args flag, like this:

  $ gdbserver --no-escape-args --startup-with-shell localhost:5006 ls
">test.txt"

This prevents the ">test.txt" being escaped by GDB to "\>test.txt".  Nothing
needs to change with how GDB is run.

-- 
You are receiving this mail because:
You are on the CC list for the bug.