[00/16] Inferior argument (inc for remote targets) changes

  This series relates to bug PR gdb/28392.  For background, check out
this series:

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

which is a previous attempt to address this bug.

It might also be worth reading:

  https://inbox.sourceware.org/gdb-patches/2b98ca58e47638b4760d86bd6e1fa9a9a79fa2ad.1695817255.git.aburgess@redhat.com

which is a previous series of mine which covers some of the work from
the original patch series.

This series does include many ideas taken from the original patch
series (thanks for Michael (original series author) for his work).  I
think this iteration does include some additional fixes beyond the
original series, also there are more tests and documentation changes
in this version.

There is one small problem: patch #1 is within libiberty.  I have
posted this to the gcc list here:

  https://inbox.sourceware.org/gcc-patches/24a8d878590403540bc9b579ba58805985a4d2f7.1701881419.git.aburgess@redhat.com/

However, GCC is currently in stage 4 of its release cycle, so I'm not
expecting to see that patch merged before April, I've expanded on this
more within the patch #1 email.

Still, there's plenty here to comment on, and I figure between now and
April I can address any feedback that's given.

Thanks,
Andrew

---

Andrew Burgess (14):
  libiberty/buildargv: POSIX behaviour for backslash handling
  gdb/testsuite: add some xfail in gdb.base/startup-with-shell.exp
  gdb: remove the !startup_with_shell path from
    construct_inferior_arguments
  gdbserver: convert program_args to a single string
  gdbsupport: have construct_inferior_arguments take an escape function
  gdbsupport: split escape_shell_characters in two
  gdb: move remote arg splitting and joining into gdbsupport/
  gdb/python: change escaping rules when setting arguments
  gdb: add remote argument passing self tests
  gdb/gdbserver: pass inferior arguments as a single string
  gdb: allow 'set args' and run commands to contain newlines
  gdb/gdbserver: remove some uses of free_vector_argv
  gdb: new maintenance command to help debug remote argument issues
  gdb/gdbserver: rework argument splitting and joining

Michael Weghorn (2):
  gdb: Support some escaping of args with startup-with-shell being off
  gdb/gdbserver: add a '--no-escape-args' command line option

 gdb/Makefile.in                               |   1 +
 gdb/NEWS                                      |  41 +++
 gdb/doc/gdb.texinfo                           | 191 ++++++++++++-
 gdb/doc/python.texi                           |   7 +-
 gdb/infcmd.c                                  | 128 ++++++++-
 gdb/inferior.c                                |   8 -
 gdb/inferior.h                                |   7 +-
 gdb/main.c                                    |  30 +-
 gdb/nat/fork-inferior.c                       |  84 ++----
 gdb/python/py-inferior.c                      |   7 +-
 gdb/remote.c                                  |  94 ++++++-
 gdb/testsuite/gdb.base/args.exp               | 137 ++++++---
 gdb/testsuite/gdb.base/inferior-args.exp      | 215 +++++++++++++--
 .../gdb.base/maint-test-remote-args.exp       |  40 +++
 gdb/testsuite/gdb.base/startup-with-shell.exp | 143 ++++++++--
 gdb/testsuite/gdb.python/py-inferior.exp      |  36 ++-
 gdb/testsuite/gdb.server/inferior-args.c      |  27 ++
 gdb/testsuite/gdb.server/inferior-args.exp    | 157 +++++++++++
 gdb/unittests/remote-arg-selftests.c          | 172 ++++++++++++
 gdbserver/linux-low.cc                        |   5 +-
 gdbserver/linux-low.h                         |   2 +-
 gdbserver/netbsd-low.cc                       |   6 +-
 gdbserver/netbsd-low.h                        |   2 +-
 gdbserver/server.cc                           |  74 +++--
 gdbserver/server.h                            |   5 +
 gdbserver/target.h                            |   6 +-
 gdbserver/win32-low.cc                        |   7 +-
 gdbserver/win32-low.h                         |   2 +-
 gdbsupport/Makefile.am                        |   1 +
 gdbsupport/Makefile.in                        |  13 +-
 gdbsupport/common-inferior.cc                 | 207 +++++++++-----
 gdbsupport/common-inferior.h                  |  29 +-
 gdbsupport/remote-args.cc                     | 260 ++++++++++++++++++
 gdbsupport/remote-args.h                      |  44 +++
 libiberty/argv.c                              |   8 +-
 libiberty/testsuite/test-expandargv.c         |  34 +++
 36 files changed, 1927 insertions(+), 303 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/maint-test-remote-args.exp
 create mode 100644 gdb/testsuite/gdb.server/inferior-args.c
 create mode 100644 gdb/testsuite/gdb.server/inferior-args.exp
 create mode 100644 gdb/unittests/remote-arg-selftests.c
 create mode 100644 gdbsupport/remote-args.cc
 create mode 100644 gdbsupport/remote-args.h


base-commit: b7a5722ebdd24a0d15d56e96d30a649ea1d7b0ee
  

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>, Michael Weghorn <m.weghorn@posteo.de>
> Date: Tue,  9 Jan 2024 14:26:23 +0000
> 
> This series relates to bug PR gdb/28392.  For background, check out
> this series:
> 
>   https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de
> 
> which is a previous attempt to address this bug.
> 
> It might also be worth reading:
> 
>   https://inbox.sourceware.org/gdb-patches/2b98ca58e47638b4760d86bd6e1fa9a9a79fa2ad.1695817255.git.aburgess@redhat.com
> 
> which is a previous series of mine which covers some of the work from
> the original patch series.
> 
> This series does include many ideas taken from the original patch
> series (thanks for Michael (original series author) for his work).  I
> think this iteration does include some additional fixes beyond the
> original series, also there are more tests and documentation changes
> in this version.
> 
> There is one small problem: patch #1 is within libiberty.  I have
> posted this to the gcc list here:
> 
>   https://inbox.sourceware.org/gcc-patches/24a8d878590403540bc9b579ba58805985a4d2f7.1701881419.git.aburgess@redhat.com/
> 
> However, GCC is currently in stage 4 of its release cycle, so I'm not
> expecting to see that patch merged before April, I've expanded on this
> more within the patch #1 email.
> 
> Still, there's plenty here to comment on, and I figure between now and
> April I can address any feedback that's given.

Thanks.  It is hard to follow all these changes especially as they are
broken into so many pieces, but I cannot avoid asking: what does this
mean for the quoting/escaping of arguments by GDB as a native debugger
on MS-Windows?

For starters, AFAIK on Windows there's no such thing as "startup with
shell", as the inferior is never invoked via the Windows shell.  So
what does this feature mean and how (and whether) will it work in a
native GDB running on Windows debugging Windows programs?  Likewise
with the --no-escape-args command-line option?

And this text:

> When starting GDB it is possible to set an inferior argument that
> contains a newline, for example:
> 
>   shell> gdb --args my.app "abc
>   > def"
>   ...
>   (gdb) show args
>   Argument list to give program being debugged when it is started is "abc'
>   'def".
> 
> However, once GDB is started, the only way to install an argument
> containing a newline is to use the Python API.
> 
> This commit changes that.
> 
> After this commit 'set args' as well as 'run', 'start', and 'starti',
> will now accept multi-line inferior arguments, e.g.:
> 
>   (gdb) set args "abc
>   > def"
>   (gdb) show args
>   Argument list to give program being debugged when it is started is ""abc
>   def"".
> 
> And also:
> 
>   (gdb) run "abc
>   > def"
>   ... etc ...

seems to say that quoting arguments can cause the inferior's
command-line arguments to include a newline, but AFAIK there's no way
to make that work on MS-Windows.  If that is true, we should at least
document the lack of support for this on Windows.

Thanks, and apologies in advance if my questions make no sense.
  

Hi Andrew,

On 2024-01-09 15:26, Andrew Burgess wrote:
> This series relates to bug PR gdb/28392.  For background, check out
> this series:
> 
>    https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de
> 
> which is a previous attempt to address this bug.
> 
> It might also be worth reading:
> 
>    https://inbox.sourceware.org/gdb-patches/2b98ca58e47638b4760d86bd6e1fa9a9a79fa2ad.1695817255.git.aburgess@redhat.com
> 
> which is a previous series of mine which covers some of the work from
> the original patch series.
> 
> This series does include many ideas taken from the original patch
> series (thanks for Michael (original series author) for his work).  I
> think this iteration does include some additional fixes beyond the
> original series, also there are more tests and documentation changes
> in this version.

Thanks a lot for looking into that series and coming up with an improved 
solution for the underlying problem!

Best,
Michael
  

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrew Burgess <aburgess@redhat.com>
>> Cc: Andrew Burgess <aburgess@redhat.com>, Michael Weghorn <m.weghorn@posteo.de>
>> Date: Tue,  9 Jan 2024 14:26:23 +0000
>> 
>> This series relates to bug PR gdb/28392.  For background, check out
>> this series:
>> 
>>   https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de
>> 
>> which is a previous attempt to address this bug.
>> 
>> It might also be worth reading:
>> 
>>   https://inbox.sourceware.org/gdb-patches/2b98ca58e47638b4760d86bd6e1fa9a9a79fa2ad.1695817255.git.aburgess@redhat.com
>> 
>> which is a previous series of mine which covers some of the work from
>> the original patch series.
>> 
>> This series does include many ideas taken from the original patch
>> series (thanks for Michael (original series author) for his work).  I
>> think this iteration does include some additional fixes beyond the
>> original series, also there are more tests and documentation changes
>> in this version.
>> 
>> There is one small problem: patch #1 is within libiberty.  I have
>> posted this to the gcc list here:
>> 
>>   https://inbox.sourceware.org/gcc-patches/24a8d878590403540bc9b579ba58805985a4d2f7.1701881419.git.aburgess@redhat.com/
>> 
>> However, GCC is currently in stage 4 of its release cycle, so I'm not
>> expecting to see that patch merged before April, I've expanded on this
>> more within the patch #1 email.
>> 
>> Still, there's plenty here to comment on, and I figure between now and
>> April I can address any feedback that's given.
>
> Thanks.  It is hard to follow all these changes especially as they are
> broken into so many pieces, but I cannot avoid asking: what does this
> mean for the quoting/escaping of arguments by GDB as a native debugger
> on MS-Windows?
>
> For starters, AFAIK on Windows there's no such thing as "startup with
> shell", as the inferior is never invoked via the Windows shell.  So
> what does this feature mean and how (and whether) will it work in a
> native GDB running on Windows debugging Windows programs?  Likewise
> with the --no-escape-args command-line option?
>
> And this text:
>
>> When starting GDB it is possible to set an inferior argument that
>> contains a newline, for example:
>> 
>>   shell> gdb --args my.app "abc
>>   > def"
>>   ...
>>   (gdb) show args
>>   Argument list to give program being debugged when it is started is "abc'
>>   'def".
>> 
>> However, once GDB is started, the only way to install an argument
>> containing a newline is to use the Python API.
>> 
>> This commit changes that.
>> 
>> After this commit 'set args' as well as 'run', 'start', and 'starti',
>> will now accept multi-line inferior arguments, e.g.:
>> 
>>   (gdb) set args "abc
>>   > def"
>>   (gdb) show args
>>   Argument list to give program being debugged when it is started is ""abc
>>   def"".
>> 
>> And also:
>> 
>>   (gdb) run "abc
>>   > def"
>>   ... etc ...
>
> seems to say that quoting arguments can cause the inferior's
> command-line arguments to include a newline, but AFAIK there's no way
> to make that work on MS-Windows.  If that is true, we should at least
> document the lack of support for this on Windows.
>
> Thanks, and apologies in advance if my questions make no sense.

The question makes perfect sense.

My hope is that the situation on Windows is no better or worse than it
ever was.

I'm reluctant to start writing documentation saying feature X does or
does not work on Windows and why though as I don't have access to a
Windows machine on which I can test any of this.

Specifically on the question of passing a newline within an argument, I
guess (having taken a quick peek at the window-nat.c file) that
embedding a newline within the argument string would cause problems?

I believe it's already possible today to inject newline characters via
the Python API.  Maybe the window-nat.c code should be checking for
invalid characters and throw an error if the user tries to start an
inferior.  I guess the other option would be to check the arguments at
the moment the user sets them ... but this isn't going to work if the
user sets the arguments before connecting to a remote target, so I
suspect it makes more sense to detect invalid argument characters just
prior to starting the inferior and giving an error at that point.

I agree that platform specific limitations should be documented.  I'm
willing to help however I can to get that done.  But I'm a little
uncomfortable trying to document something I can't test.  I'll see if I
can source a Windows laptop, maybe I can build/test in that setup.

Thanks,
Andrew
  

On 1/9/24 06:26, Andrew Burgess wrote:
> This series relates to bug PR gdb/28392.  For background, check out
> this series:
> 
>    https://inbox.sourceware.org/gdb-patches/20211022071933.3478427-1-m.weghorn@posteo.de

Having run through this code in years gone by, this series is long
overdue, so thank you for that.

> Still, there's plenty here to comment on, and I figure between now and
> April I can address any feedback that's given.

Re: libiberty, I am sure you'll make something work. :-)

FWIW, I've regression tested the series on our internal testing harness
and found no regressions on Fedora 39 (x86_64, aarch64, ppc64le, s390x).

I only have comments for a handful of the patches, and most of those
comments are pretty trivial. Feel free to add my Reviewed-by.

Keith

> Andrew Burgess (14):
>    libiberty/buildargv: POSIX behaviour for backslash handling
>    gdb/testsuite: add some xfail in gdb.base/startup-with-shell.exp
>    gdb: remove the !startup_with_shell path from
>      construct_inferior_arguments
>    gdbserver: convert program_args to a single string
>    gdbsupport: have construct_inferior_arguments take an escape function
>    gdbsupport: split escape_shell_characters in two
>    gdb: move remote arg splitting and joining into gdbsupport/
>    gdb/python: change escaping rules when setting arguments
>    gdb: add remote argument passing self tests
>    gdb/gdbserver: pass inferior arguments as a single string
>    gdb: allow 'set args' and run commands to contain newlines
>    gdb/gdbserver: remove some uses of free_vector_argv
>    gdb: new maintenance command to help debug remote argument issues
>    gdb/gdbserver: rework argument splitting and joining
> 
> Michael Weghorn (2):
>    gdb: Support some escaping of args with startup-with-shell being off
>    gdb/gdbserver: add a '--no-escape-args' command line option
> 
>   gdb/Makefile.in                               |   1 +
>   gdb/NEWS                                      |  41 +++
>   gdb/doc/gdb.texinfo                           | 191 ++++++++++++-
>   gdb/doc/python.texi                           |   7 +-
>   gdb/infcmd.c                                  | 128 ++++++++-
>   gdb/inferior.c                                |   8 -
>   gdb/inferior.h                                |   7 +-
>   gdb/main.c                                    |  30 +-
>   gdb/nat/fork-inferior.c                       |  84 ++----
>   gdb/python/py-inferior.c                      |   7 +-
>   gdb/remote.c                                  |  94 ++++++-
>   gdb/testsuite/gdb.base/args.exp               | 137 ++++++---
>   gdb/testsuite/gdb.base/inferior-args.exp      | 215 +++++++++++++--
>   .../gdb.base/maint-test-remote-args.exp       |  40 +++
>   gdb/testsuite/gdb.base/startup-with-shell.exp | 143 ++++++++--
>   gdb/testsuite/gdb.python/py-inferior.exp      |  36 ++-
>   gdb/testsuite/gdb.server/inferior-args.c      |  27 ++
>   gdb/testsuite/gdb.server/inferior-args.exp    | 157 +++++++++++
>   gdb/unittests/remote-arg-selftests.c          | 172 ++++++++++++
>   gdbserver/linux-low.cc                        |   5 +-
>   gdbserver/linux-low.h                         |   2 +-
>   gdbserver/netbsd-low.cc                       |   6 +-
>   gdbserver/netbsd-low.h                        |   2 +-
>   gdbserver/server.cc                           |  74 +++--
>   gdbserver/server.h                            |   5 +
>   gdbserver/target.h                            |   6 +-
>   gdbserver/win32-low.cc                        |   7 +-
>   gdbserver/win32-low.h                         |   2 +-
>   gdbsupport/Makefile.am                        |   1 +
>   gdbsupport/Makefile.in                        |  13 +-
>   gdbsupport/common-inferior.cc                 | 207 +++++++++-----
>   gdbsupport/common-inferior.h                  |  29 +-
>   gdbsupport/remote-args.cc                     | 260 ++++++++++++++++++
>   gdbsupport/remote-args.h                      |  44 +++
>   libiberty/argv.c                              |   8 +-
>   libiberty/testsuite/test-expandargv.c         |  34 +++
>   36 files changed, 1927 insertions(+), 303 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.base/maint-test-remote-args.exp
>   create mode 100644 gdb/testsuite/gdb.server/inferior-args.c
>   create mode 100644 gdb/testsuite/gdb.server/inferior-args.exp
>   create mode 100644 gdb/unittests/remote-arg-selftests.c
>   create mode 100644 gdbsupport/remote-args.cc
>   create mode 100644 gdbsupport/remote-args.h
> 
> 
> base-commit: b7a5722ebdd24a0d15d56e96d30a649ea1d7b0ee
  

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: gdb-patches@sourceware.org, m.weghorn@posteo.de
> Date: Sat, 20 Jan 2024 22:46:02 +0000
> 
> The question makes perfect sense.
> 
> My hope is that the situation on Windows is no better or worse than it
> ever was.
> 
> I'm reluctant to start writing documentation saying feature X does or
> does not work on Windows and why though as I don't have access to a
> Windows machine on which I can test any of this.

How about saying in the manual that for the embedded newlines to work,
the inferior should be started via a Posix shell?  This should be
enough to hint Windows users this will not work for them.

> I believe it's already possible today to inject newline characters via
> the Python API.  Maybe the window-nat.c code should be checking for
> invalid characters and throw an error if the user tries to start an
> inferior.

If doing that produces some error message from the inferior or the
APIs we use to start it, that should be enbough.

> I agree that platform specific limitations should be documented.  I'm
> willing to help however I can to get that done.  But I'm a little
> uncomfortable trying to document something I can't test.  I'll see if I
> can source a Windows laptop, maybe I can build/test in that setup.

Maybe someone else can test that, and we could then amend the
documentation as needed?  IOW, I see no reason to delay installing
your patch series due to this.

Thanks.
  

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andrew Burgess <aburgess@redhat.com>
>> Cc: gdb-patches@sourceware.org, m.weghorn@posteo.de
>> Date: Sat, 20 Jan 2024 22:46:02 +0000
>> 
>> The question makes perfect sense.
>> 
>> My hope is that the situation on Windows is no better or worse than it
>> ever was.
>> 
>> I'm reluctant to start writing documentation saying feature X does or
>> does not work on Windows and why though as I don't have access to a
>> Windows machine on which I can test any of this.
>
> How about saying in the manual that for the embedded newlines to work,
> the inferior should be started via a Posix shell?  This should be
> enough to hint Windows users this will not work for them.

That makes sense.  I other feedback to address, so I'll add some
suitable words in this area and draw your attention to it on V2.

Thanks,
Andrew


>
>> I believe it's already possible today to inject newline characters via
>> the Python API.  Maybe the window-nat.c code should be checking for
>> invalid characters and throw an error if the user tries to start an
>> inferior.
>
> If doing that produces some error message from the inferior or the
> APIs we use to start it, that should be enbough.
>
>> I agree that platform specific limitations should be documented.  I'm
>> willing to help however I can to get that done.  But I'm a little
>> uncomfortable trying to document something I can't test.  I'll see if I
>> can source a Windows laptop, maybe I can build/test in that setup.
>
> Maybe someone else can test that, and we could then amend the
> documentation as needed?  IOW, I see no reason to delay installing
> your patch series due to this.
>
> Thanks.