gdbserver: fix handling of single quote arguments

  I noticed that passing arguments containing single quotes to gdbserver
didn't work correctly:

  gdb -ex 'set sysroot' --args /tmp/show-args
  Reading symbols from /tmp/show-args...
  (gdb) target extended-remote | gdbserver --once --multi - /tmp/show-args
  Remote debugging using | gdbserver --once --multi - /tmp/show-args
  stdin/stdout redirected
  Process /tmp/show-args created; pid = 176054
  Remote debugging using stdio
  Reading symbols from /lib64/ld-linux-x86-64.so.2...
  (No debugging symbols found in /lib64/ld-linux-x86-64.so.2)
  0x00007ffff7fd3110 in _start () from /lib64/ld-linux-x86-64.so.2
  (gdb) set args \'
  (gdb) r
  The program being debugged has been started already.
  Start it from the beginning? (y or n) y
  Starting program: /tmp/show-args \'
  stdin/stdout redirected
  Process /tmp/show-args created; pid = 176088
  2 args are:
    /tmp/show-args
    \'
  Done.
  [Inferior 1 (process 176088) exited normally]
  (gdb) target native
  Done.  Use the "run" command to start a process.
  (gdb) run
  Starting program: /tmp/show-args \'
  2 args are:
    /tmp/show-args
    '
  Done.
  [Inferior 1 (process 176095) exited normally]
  (gdb) q

The 'shows-args' program used here just prints the arguments passed to
the inferior.

Notice that when starting the inferior using the extended-remote
target the second argument is "\'", while when running using native
target the argument is "'".  The second of these is correct, the \'
used with the "set args" command is just to show GDB that the single
quote is not opening an argument string.

It turns out that the extra backslash is injected on the gdbserver
side when gdbserver processes the arguments that GDB passes it, the
code that does this was added as part of this much larger commit:

  commit 2090129c36c7e582943b7d300968d19b46160d84
  Date:   Thu Dec 22 21:11:11 2016 -0500

      Share fork_inferior et al with gdbserver

In this commit I propose removing the specific code that adds what I
believe is a stray backslash.  I've extended an existing test to cover
this case, and I now see identical behaviour when using an
extended-remote target as with the native target.
---
 gdb/testsuite/gdb.base/inferior-args.exp | 7 +++++--
 gdbserver/server.cc                      | 6 ------
 2 files changed, 5 insertions(+), 8 deletions(-)


base-commit: f586e3409b752748bf213520c2dbb0b44e0005d8
  

On Sep 27 2023, Andrew Burgess via Gdb-patches wrote:

> In this commit I propose removing the specific code that adds what I
> believe is a stray backslash.  I've extended an existing test to cover
> this case, and I now see identical behaviour when using an
> extended-remote target as with the native target.

It seems like arguments containing newlines, and a trailing empty
argument are also mishandled.
  

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> Some fixes for passing arguments to gdbserver.

This all looks good to me.  Thanks for working on this.

https://sourceware.org/bugzilla/show_bug.cgi?id=28392
points to this series:
https://sourceware.org/pipermail/gdb-patches/2021-October/182723.html

We really do need a patch tracker... the current situation is not good.

Anyway I wonder how your patches compare.

Approved-By: Tom Tromey <tom@tromey.com>

Tom
  

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> Some fixes for passing arguments to gdbserver.
>
> This all looks good to me.  Thanks for working on this.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=28392
> points to this series:
> https://sourceware.org/pipermail/gdb-patches/2021-October/182723.html
>
> We really do need a patch tracker... the current situation is not good.
>
> Anyway I wonder how your patches compare.

So I took a look at this older series, and got it compiling.  It is a
super-set of my changes.

On the whole the older series looks really promising, however, it does
change the vRun remote packet, which I don't think is acceptable as it
was initially suggested[1].

I think this series from me does still offer some value, so I'm going to
go ahead and merge it, though I've acknowledged the author of the original
series with a Co-Authored-By tag, though I had not read that series when
I created my work.

When I get a chance, I'll try to revisit the older series and see what
can be done with it.

[1] All arguments are now passed as a single string within the vRun
rather than a vector of arguments.  I don't think this is going to be
acceptable.  If this really is the only way to solve the quoting problem
then, at a minimum, we'd need a feature flag to indicate this change in
behaviour.


>
> Approved-By: Tom Tromey <tom@tromey.com>
>

I removed the stray 'verbose' output from patch #4, and pushed this
series.

Thanks,
Andrew
  

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> [1] All arguments are now passed as a single string within the vRun
Andrew> rather than a vector of arguments.  I don't think this is going to be
Andrew> acceptable.  If this really is the only way to solve the quoting problem
Andrew> then, at a minimum, we'd need a feature flag to indicate this change in
Andrew> behaviour.

Yeah, I think we should avoid this change if at all possible.

Tom