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

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

[Andrew Burgess]

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
$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
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
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>
---
 gdb/NEWS                                      |   7 +
 gdb/doc/gdb.texinfo                           |  25 +++
 gdb/remote.c                                  |  25 ++-
 gdb/testsuite/gdb.base/args.exp               |  47 ++++--
 gdb/testsuite/gdb.base/inferior-args.exp      |  31 +++-
 gdb/testsuite/gdb.base/startup-with-shell.exp | 146 ++++++++++--------
 gdbserver/server.cc                           |  20 ++-
 gdbserver/server.h                            |   5 +
 8 files changed, 221 insertions(+), 85 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index d3699de1653..22eb17ae34d 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -163,6 +163,13 @@ qXfer:threads:read
   should print as the target ID of the thread, for example in the
   "info threads" command or when switching to the thread.
 
+single-inf-arg in qSupported
+  The new single-inf-arg feature within the qSupported packet allows
+  GDB to inform the stub that it would like to send the inferior
+  arguments as a single string within the vRun packet.  The stub can
+  reply with the single-inf-arg feature to indicate that it is able to
+  accept arguments as a single string.
+
 * MI changes
 
 ** The =library-unloaded event now includes the 'ranges' field, which
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 05f550233fe..70445f79ec4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -43723,6 +43723,12 @@ Packets
 (e.g.@: the last program run).  The program is created in the stopped
 state.
 
+If @value{GDBN} sent the @samp{single-inf-arg} feature in the
+@samp{qSupported} packet (@pxref{single-inf-arg}), and the stub replied
+with @samp{single-inf-arg+}, then there will only be a single
+@var{argument} string, which includes all inferior arguments,
+separated with whitespace.
+
 @c FIXME:  What about non-stop mode?
 
 This packet is only available in extended mode (@pxref{extended mode}).
@@ -45106,6 +45112,14 @@ General Query Packets
 
 New packets should be written to support @samp{E.@var{errtext}}
 regardless of this feature being true or not.
+
+@anchor{single-inf-arg}
+@item single-inf-arg
+This feature indicates that @value{GDBN} would like to send the
+inferior arguments as a single string within the @samp{vRun} packet.
+@value{GDBN} will not send the arguments as a single string unless the
+stub also reports that it supports this behaviour by including
+@samp{single-inf-arg+} in its @samp{qSupported} reply.
 @end table
 
 Stubs should ignore any unknown values for
@@ -45409,6 +45423,11 @@ General Query Packets
 @tab @samp{-}
 @tab No
 
+@item @samp{single-inf-arg}
+@tab No
+@tab @samp{-}
+@tab No
+
 @end multitable
 
 These are the currently defined stub features, in more detail:
@@ -45658,6 +45677,12 @@ General Query Packets
 
 @item binary-upload
 The remote stub supports the @samp{x} packet (@pxref{x packet}).
+
+@item single-inf-arg
+The remote stub would like to receive the inferior arguments as a
+single string within the @samp{vRun} packet.  The stub should only
+send this feature if @value{GDBN} sent @samp{single-inf-arg+} in the
+@samp{qSupported} packet.
 @end table
 
 @item qSymbol::
diff --git a/gdb/remote.c b/gdb/remote.c
index 1c49cdf0fc0..faad850463b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -400,6 +400,10 @@ enum {
      errors, and so they should not need to check for this feature.  */
   PACKET_accept_error_message,
 
+  /* Not really a packet; this indicates support for sending the vRun
+     inferior arguments as a single string.  */
+  PACKET_vRun_single_argument,
+
   PACKET_MAX
 };
 
@@ -825,6 +829,11 @@ struct remote_features
   bool remote_memory_tagging_p () const
   { return packet_support (PACKET_memory_tagging_feature) == PACKET_ENABLE; }
 
+  /* Returns true if there is support for sending vRun inferior arguments
+     as a single string.  */
+  bool remote_vrun_single_arg_p () const
+  { return packet_support (PACKET_vRun_single_argument) == PACKET_ENABLE; }
+
   /* Reset all packets back to "unknown support".  Called when opening a
      new connection to a remote target.  */
   void reset_all_packet_configs_support ();
@@ -5863,6 +5872,8 @@ static const struct protocol_feature remote_protocol_features[] = {
   { "error-message", PACKET_ENABLE, remote_supported_packet,
     PACKET_accept_error_message },
   { "binary-upload", PACKET_DISABLE, remote_supported_packet, PACKET_x },
+  { "single-inf-arg", PACKET_DISABLE, remote_supported_packet,
+    PACKET_vRun_single_argument },
 };
 
 static char *remote_support_xml;
@@ -5974,6 +5985,10 @@ remote_target::remote_query_supported ()
 	  != AUTO_BOOLEAN_FALSE)
 	remote_query_supported_append (&q, "memory-tagging+");
 
+      if (m_features.packet_set_cmd_state (PACKET_vRun_single_argument)
+	  != AUTO_BOOLEAN_FALSE)
+	remote_query_supported_append (&q, "single-inf-arg+");
+
       /* Keep this one last to work around a gdbserver <= 7.10 bug in
 	 the qSupported:xmlRegisters=i386 handling.  */
       if (remote_support_xml != NULL
@@ -10836,7 +10851,11 @@ remote_target::extended_remote_run (const std::string &args)
 
   if (!args.empty ())
     {
-      std::vector<std::string> split_args = gdb::remote_args::split (args);
+      std::vector<std::string> split_args;
+      if (!m_features.remote_vrun_single_arg_p ())
+	split_args = gdb::remote_args::split (args);
+      else
+	split_args.push_back (args);
 
       for (const auto &a : split_args)
 	{
@@ -16539,6 +16558,10 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (PACKET_accept_error_message,
 			 "error-message", "error-message", 0);
 
+  add_packet_config_cmd (PACKET_vRun_single_argument,
+			 "single-inferior-argument-feature",
+			 "single-inferior-argument-feature", 0);
+
   /* Assert that we've registered "set remote foo-packet" commands
      for all packet configs.  */
   {
diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp
index 33952e4bd76..fa44a5a7fd8 100644
--- a/gdb/testsuite/gdb.base/args.exp
+++ b/gdb/testsuite/gdb.base/args.exp
@@ -72,31 +72,48 @@ proc args_test { name arglist {re_list {}} } {
     }
 }
 
-# Test that the --args are processed correctly.
+# Run all the tests.
+proc run_all_tests {} {
+    # Test that the --args are processed correctly.
 
-args_test basic {{1} {3}}
+    args_test basic {{1} {3}}
 
-# Test that the --args are processed correctly even if one of them is
-# empty.
+    # Test that the --args are processed correctly even if one of them is
+    # empty.
 
-args_test "one empty" {{1} {} {3}}
+    args_test "one empty" {{1} {} {3}}
 
-# Try with 2 empty args.
+    # Try with 2 empty args.
 
-args_test "two empty" {{1} {} {} 3}
+    args_test "two empty" {{1} {} {} 3}
 
-# Try with arguments containing literal single quotes.
+    # Try with arguments containing literal single quotes.
 
-args_test "one empty with single quotes" {{1} {''} {3}}
+    args_test "one empty with single quotes" {{1} {''} {3}}
 
-args_test "two empty with single quotes" {{1} {''} {''} {3}}
+    args_test "two empty with single quotes" {{1} {''} {''} {3}}
 
-# Try with arguments containing literal newlines.
+    # Try with arguments containing literal newlines.
 
-args_test "one newline" {{1} "\n" {3}} {1 \\\\n 3}
+    args_test "one newline" {{1} "\n" {3}} {1 \\\\n 3}
 
-args_test "two newlines" {{1} "\n" "\n" {3}} {1 \\\\n \\\\n 3}
+    args_test "two newlines" {{1} "\n" "\n" {3}} {1 \\\\n \\\\n 3}
 
-args_test "lone single quote" {{1} \' {3}}
+    args_test "lone single quote" {{1} \' {3}}
 
-args_test "lone double quote" {{1} \" {3}} {1 \\\\\" 3}
+    args_test "lone double quote" {{1} \" {3}} {1 \\\\\" 3}
+}
+
+run_all_tests
+
+# For extended-remote targets, disable the packet which passes
+# inferior arguments as a single string.  This changes how the vRun
+# (extended-remote only) packet works.
+if {[target_info gdb_protocol] == "extended-remote"} {
+    with_test_prefix "single-inferior-arg disabled" {
+	save_vars { GDBFLAGS } {
+	    append GDBFLAGS " -ex \"set remote single-inferior-argument-feature-packet off\""
+	    run_all_tests
+	}
+    }
+}
diff --git a/gdb/testsuite/gdb.base/inferior-args.exp b/gdb/testsuite/gdb.base/inferior-args.exp
index 9406c7882f0..db49e326552 100644
--- a/gdb/testsuite/gdb.base/inferior-args.exp
+++ b/gdb/testsuite/gdb.base/inferior-args.exp
@@ -211,14 +211,31 @@ lappend test_desc_list [list "test four" \
 			    [list "$hex \"'\"" \
 				 "$hex \"\\\\\"\""]]
 
-foreach desc $test_desc_list {
-    lassign $desc name stub_suitable args re_list
-    with_test_prefix $name {
-	foreach_with_prefix set_method { "start" "starti" "run" "set args" } {
-	    foreach_with_prefix startup_with_shell { on off } {
-		do_test $set_method $startup_with_shell $args $re_list \
-		    $stub_suitable
+# Run all tests in the global TEST_DESC_LIST.
+proc run_all_tests {} {
+    foreach desc $::test_desc_list {
+	lassign $desc name stub_suitable args re_list
+	with_test_prefix $name {
+	    foreach_with_prefix set_method { "start" "starti" "run" "set args" } {
+		foreach_with_prefix startup_with_shell { on off } {
+		    do_test $set_method $startup_with_shell $args $re_list \
+			$stub_suitable
+		}
 	    }
 	}
     }
 }
+
+run_all_tests
+
+# For extended-remote targets, disable the packet which passes
+# inferior arguments as a single string.  This changes how the vRun
+# (extended-remote only) packet works.
+if {[target_info gdb_protocol] == "extended-remote"} {
+    with_test_prefix "single-inferior-arg disabled" {
+	save_vars { GDBFLAGS } {
+	   append GDBFLAGS " -ex \"set remote single-inferior-argument-feature-packet off\""
+	   run_all_tests
+       }
+    }
+}
diff --git a/gdb/testsuite/gdb.base/startup-with-shell.exp b/gdb/testsuite/gdb.base/startup-with-shell.exp
index 80dfdf3db83..1a6c5b94383 100644
--- a/gdb/testsuite/gdb.base/startup-with-shell.exp
+++ b/gdb/testsuite/gdb.base/startup-with-shell.exp
@@ -91,73 +91,97 @@ proc run_test_same { args re testname } {
     run_test $args $re $re $testname
 }
 
-# The regexp to match a single '\' character.
-set bs "\\\\"
+# Run the actual tests
+proc run_all_tests { { is_remote_with_split_args false } } {
+    # The regexp to match a single '\' character.
+    set bs "\\\\"
 
-# Are we using 'remote' or 'extended-remote' protocol?
-set is_remote_p [gdb_protocol_is_remote]
+    run_test "$::unique_file_dir/*.unique-extension" \
+	"\"$::unique_file\"" \
+	"\"$::unique_file_dir/\\\*\.unique-extension\"" \
+	"arg is glob" \
+	$is_remote_with_split_args
 
-## Run the actual tests
+    run_test_same "$::unique_file_dir/\\*.unique-extension" \
+	"\"$::unique_file_dir/\\\*\.unique-extension\"" \
+	"arg is escaped glob"
 
-run_test "$unique_file_dir/*.unique-extension" \
-    "\"$unique_file\"" \
-    "\"$unique_file_dir/\\\*\.unique-extension\"" \
-    "arg is glob" \
-    $is_remote_p
+    save_vars { ::env(TEST) } {
+	set ::env(TEST) "1234"
+	run_test "\$TEST" \
+	    "\"1234\"" \
+	    "\"\\\$TEST\"" \
+	    "arg is shell variable" \
+	    $is_remote_with_split_args
 
-run_test_same "$unique_file_dir/\\*.unique-extension" \
-    "\"$unique_file_dir/\\\*\.unique-extension\"" \
-    "arg is escaped glob"
+	run_test_same "\\\$TEST" \
+	    "\"\\\$TEST\"" \
+	    "arg is escaped shell variable"
+    }
 
-save_vars { env(TEST) } {
-    set env(TEST) "1234"
-    run_test "\$TEST" \
-	"\"1234\"" \
-	"\"\\\$TEST\"" \
-	"arg is shell variable" \
-	$is_remote_p
+    run_test "\$(echo foo)" \
+	"\"foo\"" \
+	"\"\\\$\\(echo\"" \
+	"arg is parameter expansion, command execution" \
+	$is_remote_with_split_args
 
-    run_test_same "\\\$TEST" \
-	"\"\\\$TEST\"" \
-	"arg is escaped shell variable"
+    run_test "\$((2 + 3))" \
+	"\"5\"" \
+	"\"\\\$\\(\\(2\"" \
+	"arg is parameter expansion, expression evaluation" \
+	$is_remote_with_split_args
+
+    run_test_same "\"\\a\"" \
+	"\"${bs}${bs}a\"" \
+	"retain backslash in double quote arg"
+
+    run_test_same "'\\a'" \
+	"\"${bs}${bs}a\"" \
+	"retain backslash in single quote arg"
+
+    run_test_same "\"\\\$\"" \
+	"\"\\\$\"" \
+	"'\$' can be escaped in double quote arg"
+
+    run_test_same "'\\\$'" \
+	"\"${bs}${bs}\\\$\"" \
+	"'\$' is not escaped in single quote arg"
+
+    run_test_same "\"\\`\"" \
+	"\"\\`\"" \
+	"'`' can be escaped in double quote arg"
+
+    run_test_same "'\\`'" \
+	"\"${bs}${bs}`\"" \
+	"'`' is not escaped in single quote arg"
+
+    run_test_same "\"\\\"\"" \
+	"\"${bs}\"\"" \
+	"'\"' can be escaped in double quote arg"
+
+    run_test_same "'\\\"'" \
+	"\"${bs}${bs}${bs}\"\"" \
+	"'\"' is not escaped in single quote arg"
+
+    run_test_same "\"\\\\\"" \
+	"\"${bs}${bs}\"" \
+	"'\\' can be escaped in double quote arg"
+
+    run_test_same "'\\\\'" \
+	"\"${bs}${bs}${bs}${bs}\"" \
+	"'\\' is not escaped in single quote arg"
 }
 
-run_test_same "\"\\a\"" \
-    "\"${bs}${bs}a\"" \
-    "retain backslash in double quote arg"
+run_all_tests
 
-run_test_same "'\\a'" \
-    "\"${bs}${bs}a\"" \
-    "retain backslash in single quote arg"
-
-run_test_same "\"\\\$\"" \
-    "\"\\\$\"" \
-    "'\$' can be escaped in double quote arg"
-
-run_test_same "'\\\$'" \
-    "\"${bs}${bs}\\\$\"" \
-    "'\$' is not escaped in single quote arg"
-
-run_test_same "\"\\`\"" \
-    "\"\\`\"" \
-    "'`' can be escaped in double quote arg"
-
-run_test_same "'\\`'" \
-    "\"${bs}${bs}`\"" \
-    "'`' is not escaped in single quote arg"
-
-run_test_same "\"\\\"\"" \
-    "\"${bs}\"\"" \
-    "'\"' can be escaped in double quote arg"
-
-run_test_same "'\\\"'" \
-    "\"${bs}${bs}${bs}\"\"" \
-    "'\"' is not escaped in single quote arg"
-
-run_test_same "\"\\\\\"" \
-    "\"${bs}${bs}\"" \
-    "'\\' can be escaped in double quote arg"
-
-run_test_same "'\\\\'" \
-    "\"${bs}${bs}${bs}${bs}\"" \
-    "'\\' is not escaped in single quote arg"
+# For extended-remote targets, disable the packet which passes
+# inferior arguments as a single string.  This changes how the vRun
+# (extended-remote only) packet works.
+if {[target_info gdb_protocol] == "extended-remote"} {
+    with_test_prefix "single-inferior-arg disabled" {
+	save_vars { GDBFLAGS } {
+	    append GDBFLAGS " -ex \"set remote single-inferior-argument-feature-packet off\""
+	    run_all_tests true
+	}
+    }
+}
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index ab69400f97b..2bb6e6b74ad 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -2747,6 +2747,8 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
 		}
 	      else if (feature == "error-message+")
 		cs.error_message_supported = true;
+	      else if (feature == "single-inf-arg+")
+		cs.single_inferior_argument = true;
 	      else
 		{
 		  /* Move the unknown features all together.  */
@@ -2874,6 +2876,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
       if (target_supports_memory_tagging ())
 	strcat (own_buf, ";memory-tagging+");
 
+      if (cs.single_inferior_argument)
+	strcat (own_buf, ";single-inf-arg+");
+
       /* Reinitialize components as needed for the new connection.  */
       hostio_handle_new_gdb_connection ();
       target_handle_new_gdb_connection ();
@@ -3466,7 +3471,20 @@ handle_v_run (char *own_buf)
   else
     program_path.set (new_program_name.get ());
 
-  program_args = gdb::remote_args::join (new_argv.get ());
+  if (cs.single_inferior_argument)
+    {
+      if (new_argv.get ().size () > 1)
+	{
+	  write_enn (own_buf);
+	  return;
+	}
+      else if (new_argv.get ().size () == 1)
+	program_args = std::string (new_argv.get ()[0]);
+      else
+	program_args.clear ();
+    }
+  else
+    program_args = gdb::remote_args::join (new_argv.get ());
 
   try
     {
diff --git a/gdbserver/server.h b/gdbserver/server.h
index 5609584f716..652accb8064 100644
--- a/gdbserver/server.h
+++ b/gdbserver/server.h
@@ -196,6 +196,11 @@ struct client_state
      are not supported with qRcmd and m packets, but are still supported
      everywhere else.  This is for backward compatibility reasons.  */
   bool error_message_supported = false;
+
+  /* If true then we've agreed that the debugger will send all inferior
+     arguments as a single string.  When false the debugger will attempt
+     to split the inferior arguments before sending them.  */
+  bool single_inferior_argument = false;
 };
 
 client_state &get_client_state ();

base-commit: 4e16a470492e412b6652d1e992f49edbc8e90894
-- 
2.47.1

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

[Kevin Buettner]

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>

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

[Andrew Burgess]

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

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

[Luis]

[-- 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 --]

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

[Andrew Burgess]

Luis <luis.machado.foss@gmail.com> writes:

> 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?

Not intentionally, I'll take a look now.

Thanks,
Andrew

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

[Andrew Burgess]

Luis <luis.machado.foss@gmail.com> writes:

> 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

And to debug it, I did:

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

It's GDB all the way down!

Anyway.

The patch below fixes this problem.  I still need to write a test for
this and write a commit message.  I'll try to do that later today, or it
might have to be first thing tomorrow.

Good news is this bug is not in GDB 17 branch.

Thanks,
Andrew

---

diff --git i/gdb/main.c w/gdb/main.c
index 04c33638bec..f3049600a06 100644
--- i/gdb/main.c
+++ w/gdb/main.c
@@ -866,9 +866,14 @@ captured_main_1 (struct captured_main_args *context)
       {
 	int option_index;
 
+	/* If the previous argument was --args or --no-escape-args, then
+	   stop argument processing.  */
+	if (set_args != NO_ARGS)
+	  break;
+
 	c = getopt_long_only (argc, argv, "",
 			      long_options, &option_index);
-	if (c == EOF || set_args != NO_ARGS)
+	if (c == EOF)
 	  break;
 
 	/* Long option that takes an argument.  */

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

[Andrew Burgess]

Luis <luis.machado.foss@gmail.com> writes:

> 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?

Here's a proper fix, with tests.

Let me know what you think.

Thanks,
Andrew

---

commit 5dfd9f9da130c5deae9ac81fcda1bd18f8951eac
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Sep 16 17:02:01 2025 +0100

    gdb: fix --args handling when inferior argument have dash
    
    After the commit:
    
      commit e5e76451fa82e0bc00599af96382b361c3d6ac32
      Date:   Fri Oct 22 07:19:29 2021 +0000
    
          gdb/gdbserver: add a '--no-escape-args' command line option
    
    Inferior argument handling on the GDB command line was broken:
    
      $ gdb --args /bin/ls --foo
      ./gdb/gdb: unrecognized option '--foo'
      ./gdb/gdb: `--args' specified but no program specified
    
    Before the above patch the definition of the '--args' argument in the
    long_options array (in captured_main_1) was such that the
    getopt_long_only call would directly set the 'set_args' variable to
    true if '--args' was seen.
    
    This meant that, immediately after the getopt_long_only call, we could
    inspect set_args and break out of the argument processing loop if
    needed.
    
    After the above patch '--args' (and the new '--no-escape-args') no
    longer set set_args directly via the getopt_long_only call.  Instead
    the getopt_long_only call returns an OPT_* enum value, which we then
    use in the following switch statement in order to set the set_args
    variable.
    
    What this means is that, immediately after the getopt_long_only call,
    set_args no longer (immediately) indicates if --args was seen.  After
    the switch statement, when set_args has been updated, we go around the
    argument processing loop again and call getopt_long_only once more.
    
    This extra getopt_long_only call will, if it finds another argument
    that starts with a dash, update the global optind to point to this
    option.  At this point things have gone wrong, GDB has now lost track
    of the argument containing the program name the user wanted us to
    start.  This leads to GDB existing with the above error.
    
    The solution is to move the check of set_args to either before the
    getopt_long_only call, or to after the switch statement.  I chose to
    move it earlier as this keeps all the loop exiting checks near the
    beginning.
    
    I've added more tests that cover this issue.

diff --git a/gdb/main.c b/gdb/main.c
index 04c33638bec..f3049600a06 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -866,9 +866,14 @@ captured_main_1 (struct captured_main_args *context)
       {
 	int option_index;
 
+	/* If the previous argument was --args or --no-escape-args, then
+	   stop argument processing.  */
+	if (set_args != NO_ARGS)
+	  break;
+
 	c = getopt_long_only (argc, argv, "",
 			      long_options, &option_index);
-	if (c == EOF || set_args != NO_ARGS)
+	if (c == EOF)
 	  break;
 
 	/* Long option that takes an argument.  */
diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp
index 573543cc1f8..03e5e57ee20 100644
--- a/gdb/testsuite/gdb.base/args.exp
+++ b/gdb/testsuite/gdb.base/args.exp
@@ -186,6 +186,18 @@ proc run_all_tests {} {
 	set ::env(TEST) "ABCD"
 	args_test "shell variable" {{$TEST}} {\\$TEST} {{ABCD}}
     }
+
+    # At one point we had a bug where, if the last inferior argument,
+    # appearing after --args, looked like an option GDB might be able
+    # to process, e.g. started with a dash, then GDB would try to
+    # process it.  This would mean all leave GDB in a broken state,
+    # and so GDB would fail to start.
+    foreach_with_prefix flag { args no-escape-args } {
+	args_test "with trailing GDB flag" [list "--${flag}"]
+    }
+    args_test "with trailing GDB option and value" [list "--ex" "start"]
+    args_test "with trailing double dash option" [list "--foo"]
+    args_test "with trailing single dash option" [list "-foo"]
 }
 
 run_all_tests

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

[Luis]

Hi Andrew,

On 16/09/2025 17:21, Andrew Burgess wrote:
> Luis <luis.machado.foss@gmail.com> writes:
> 
>> 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?
> 
> Here's a proper fix, with tests.

Excellent. Thanks for the quick fix. I thought I was misremembering 
things a bit.

Let me give it a go.

> 
> Let me know what you think.
> 
> Thanks,
> Andrew
> 
> ---
> 
> commit 5dfd9f9da130c5deae9ac81fcda1bd18f8951eac
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Tue Sep 16 17:02:01 2025 +0100
> 
>      gdb: fix --args handling when inferior argument have dash
>      
>      After the commit:
>      
>        commit e5e76451fa82e0bc00599af96382b361c3d6ac32
>        Date:   Fri Oct 22 07:19:29 2021 +0000
>      
>            gdb/gdbserver: add a '--no-escape-args' command line option
>      
>      Inferior argument handling on the GDB command line was broken:
>      
>        $ gdb --args /bin/ls --foo
>        ./gdb/gdb: unrecognized option '--foo'
>        ./gdb/gdb: `--args' specified but no program specified
>      
>      Before the above patch the definition of the '--args' argument in the
>      long_options array (in captured_main_1) was such that the
>      getopt_long_only call would directly set the 'set_args' variable to
>      true if '--args' was seen.
>      
>      This meant that, immediately after the getopt_long_only call, we could
>      inspect set_args and break out of the argument processing loop if
>      needed.
>      
>      After the above patch '--args' (and the new '--no-escape-args') no
>      longer set set_args directly via the getopt_long_only call.  Instead
>      the getopt_long_only call returns an OPT_* enum value, which we then
>      use in the following switch statement in order to set the set_args
>      variable.
>      
>      What this means is that, immediately after the getopt_long_only call,
>      set_args no longer (immediately) indicates if --args was seen.  After
>      the switch statement, when set_args has been updated, we go around the
>      argument processing loop again and call getopt_long_only once more.
>      
>      This extra getopt_long_only call will, if it finds another argument
>      that starts with a dash, update the global optind to point to this
>      option.  At this point things have gone wrong, GDB has now lost track
>      of the argument containing the program name the user wanted us to
>      start.  This leads to GDB existing with the above error.
>      
>      The solution is to move the check of set_args to either before the
>      getopt_long_only call, or to after the switch statement.  I chose to
>      move it earlier as this keeps all the loop exiting checks near the
>      beginning.
>      
>      I've added more tests that cover this issue.
> 
> diff --git a/gdb/main.c b/gdb/main.c
> index 04c33638bec..f3049600a06 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -866,9 +866,14 @@ captured_main_1 (struct captured_main_args *context)
>         {
>   	int option_index;
>   
> +	/* If the previous argument was --args or --no-escape-args, then
> +	   stop argument processing.  */
> +	if (set_args != NO_ARGS)
> +	  break;
> +
>   	c = getopt_long_only (argc, argv, "",
>   			      long_options, &option_index);
> -	if (c == EOF || set_args != NO_ARGS)
> +	if (c == EOF)
>   	  break;
>   
>   	/* Long option that takes an argument.  */
> diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp
> index 573543cc1f8..03e5e57ee20 100644
> --- a/gdb/testsuite/gdb.base/args.exp
> +++ b/gdb/testsuite/gdb.base/args.exp
> @@ -186,6 +186,18 @@ proc run_all_tests {} {
>   	set ::env(TEST) "ABCD"
>   	args_test "shell variable" {{$TEST}} {\\$TEST} {{ABCD}}
>       }
> +
> +    # At one point we had a bug where, if the last inferior argument,
> +    # appearing after --args, looked like an option GDB might be able
> +    # to process, e.g. started with a dash, then GDB would try to
> +    # process it.  This would mean all leave GDB in a broken state,
> +    # and so GDB would fail to start.
> +    foreach_with_prefix flag { args no-escape-args } {
> +	args_test "with trailing GDB flag" [list "--${flag}"]
> +    }
> +    args_test "with trailing GDB option and value" [list "--ex" "start"]
> +    args_test "with trailing double dash option" [list "--foo"]
> +    args_test "with trailing single dash option" [list "-foo"]
>   }
>   
>   run_all_tests
> 

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

[Luis]

On 16/09/2025 17:21, Andrew Burgess wrote:
> Luis <luis.machado.foss@gmail.com> writes:
> 
>> 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?
> 
> Here's a proper fix, with tests.
> 
> Let me know what you think.
> 
> Thanks,
> Andrew
> 
> ---
> 
> commit 5dfd9f9da130c5deae9ac81fcda1bd18f8951eac
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Tue Sep 16 17:02:01 2025 +0100
> 
>      gdb: fix --args handling when inferior argument have dash
>      
>      After the commit:
>      
>        commit e5e76451fa82e0bc00599af96382b361c3d6ac32
>        Date:   Fri Oct 22 07:19:29 2021 +0000
>      
>            gdb/gdbserver: add a '--no-escape-args' command line option
>      
>      Inferior argument handling on the GDB command line was broken:
>      
>        $ gdb --args /bin/ls --foo
>        ./gdb/gdb: unrecognized option '--foo'
>        ./gdb/gdb: `--args' specified but no program specified
>      
>      Before the above patch the definition of the '--args' argument in the
>      long_options array (in captured_main_1) was such that the
>      getopt_long_only call would directly set the 'set_args' variable to
>      true if '--args' was seen.
>      
>      This meant that, immediately after the getopt_long_only call, we could
>      inspect set_args and break out of the argument processing loop if
>      needed.
>      
>      After the above patch '--args' (and the new '--no-escape-args') no
>      longer set set_args directly via the getopt_long_only call.  Instead
>      the getopt_long_only call returns an OPT_* enum value, which we then
>      use in the following switch statement in order to set the set_args
>      variable.
>      
>      What this means is that, immediately after the getopt_long_only call,
>      set_args no longer (immediately) indicates if --args was seen.  After
>      the switch statement, when set_args has been updated, we go around the
>      argument processing loop again and call getopt_long_only once more.
>      
>      This extra getopt_long_only call will, if it finds another argument
>      that starts with a dash, update the global optind to point to this
>      option.  At this point things have gone wrong, GDB has now lost track
>      of the argument containing the program name the user wanted us to
>      start.  This leads to GDB existing with the above error.

s/existing/exiting?

>      
>      The solution is to move the check of set_args to either before the
>      getopt_long_only call, or to after the switch statement.  I chose to
>      move it earlier as this keeps all the loop exiting checks near the
>      beginning.
>      
>      I've added more tests that cover this issue.
> 
> diff --git a/gdb/main.c b/gdb/main.c
> index 04c33638bec..f3049600a06 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -866,9 +866,14 @@ captured_main_1 (struct captured_main_args *context)
>         {
>   	int option_index;
>   
> +	/* If the previous argument was --args or --no-escape-args, then
> +	   stop argument processing.  */
> +	if (set_args != NO_ARGS)
> +	  break;
> +
>   	c = getopt_long_only (argc, argv, "",
>   			      long_options, &option_index);
> -	if (c == EOF || set_args != NO_ARGS)
> +	if (c == EOF)
>   	  break;
>   
>   	/* Long option that takes an argument.  */
> diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp
> index 573543cc1f8..03e5e57ee20 100644
> --- a/gdb/testsuite/gdb.base/args.exp
> +++ b/gdb/testsuite/gdb.base/args.exp
> @@ -186,6 +186,18 @@ proc run_all_tests {} {
>   	set ::env(TEST) "ABCD"
>   	args_test "shell variable" {{$TEST}} {\\$TEST} {{ABCD}}
>       }
> +
> +    # At one point we had a bug where, if the last inferior argument,
> +    # appearing after --args, looked like an option GDB might be able
> +    # to process, e.g. started with a dash, then GDB would try to
> +    # process it.  This would mean all leave GDB in a broken state,

Maybe a typo in "mean all". It doesn´t sound clear to me. Maybe you 
meant "we'll"?

If you think it is worth it, we could mention the specific example of 
the invocation that ran into issues.

> +    # and so GDB would fail to start.
> +    foreach_with_prefix flag { args no-escape-args } {
> +	args_test "with trailing GDB flag" [list "--${flag}"]
> +    }
> +    args_test "with trailing GDB option and value" [list "--ex" "start"]
> +    args_test "with trailing double dash option" [list "--foo"]
> +    args_test "with trailing single dash option" [list "-foo"]
>   }
>   
>   run_all_tests
> 

Thanks. The above fixed it for me.

Approved-By: Luis Machado <luis.machado.foss@gmail.com>
Tested-By: Luis Machado <luis.machado.foss@gmail.com>

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

[Andrew Burgess]

Luis <luis.machado.foss@gmail.com> writes:

> On 16/09/2025 17:21, Andrew Burgess wrote:
>> Luis <luis.machado.foss@gmail.com> writes:
>> 
>>> 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?
>> 
>> Here's a proper fix, with tests.
>> 
>> Let me know what you think.
>> 
>> Thanks,
>> Andrew
>> 
>> ---
>> 
>> commit 5dfd9f9da130c5deae9ac81fcda1bd18f8951eac
>> Author: Andrew Burgess <aburgess@redhat.com>
>> Date:   Tue Sep 16 17:02:01 2025 +0100
>> 
>>      gdb: fix --args handling when inferior argument have dash
>>      
>>      After the commit:
>>      
>>        commit e5e76451fa82e0bc00599af96382b361c3d6ac32
>>        Date:   Fri Oct 22 07:19:29 2021 +0000
>>      
>>            gdb/gdbserver: add a '--no-escape-args' command line option
>>      
>>      Inferior argument handling on the GDB command line was broken:
>>      
>>        $ gdb --args /bin/ls --foo
>>        ./gdb/gdb: unrecognized option '--foo'
>>        ./gdb/gdb: `--args' specified but no program specified
>>      
>>      Before the above patch the definition of the '--args' argument in the
>>      long_options array (in captured_main_1) was such that the
>>      getopt_long_only call would directly set the 'set_args' variable to
>>      true if '--args' was seen.
>>      
>>      This meant that, immediately after the getopt_long_only call, we could
>>      inspect set_args and break out of the argument processing loop if
>>      needed.
>>      
>>      After the above patch '--args' (and the new '--no-escape-args') no
>>      longer set set_args directly via the getopt_long_only call.  Instead
>>      the getopt_long_only call returns an OPT_* enum value, which we then
>>      use in the following switch statement in order to set the set_args
>>      variable.
>>      
>>      What this means is that, immediately after the getopt_long_only call,
>>      set_args no longer (immediately) indicates if --args was seen.  After
>>      the switch statement, when set_args has been updated, we go around the
>>      argument processing loop again and call getopt_long_only once more.
>>      
>>      This extra getopt_long_only call will, if it finds another argument
>>      that starts with a dash, update the global optind to point to this
>>      option.  At this point things have gone wrong, GDB has now lost track
>>      of the argument containing the program name the user wanted us to
>>      start.  This leads to GDB existing with the above error.
>
> s/existing/exiting?
>
>>      
>>      The solution is to move the check of set_args to either before the
>>      getopt_long_only call, or to after the switch statement.  I chose to
>>      move it earlier as this keeps all the loop exiting checks near the
>>      beginning.
>>      
>>      I've added more tests that cover this issue.
>> 
>> diff --git a/gdb/main.c b/gdb/main.c
>> index 04c33638bec..f3049600a06 100644
>> --- a/gdb/main.c
>> +++ b/gdb/main.c
>> @@ -866,9 +866,14 @@ captured_main_1 (struct captured_main_args *context)
>>         {
>>   	int option_index;
>>   
>> +	/* If the previous argument was --args or --no-escape-args, then
>> +	   stop argument processing.  */
>> +	if (set_args != NO_ARGS)
>> +	  break;
>> +
>>   	c = getopt_long_only (argc, argv, "",
>>   			      long_options, &option_index);
>> -	if (c == EOF || set_args != NO_ARGS)
>> +	if (c == EOF)
>>   	  break;
>>   
>>   	/* Long option that takes an argument.  */
>> diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp
>> index 573543cc1f8..03e5e57ee20 100644
>> --- a/gdb/testsuite/gdb.base/args.exp
>> +++ b/gdb/testsuite/gdb.base/args.exp
>> @@ -186,6 +186,18 @@ proc run_all_tests {} {
>>   	set ::env(TEST) "ABCD"
>>   	args_test "shell variable" {{$TEST}} {\\$TEST} {{ABCD}}
>>       }
>> +
>> +    # At one point we had a bug where, if the last inferior argument,
>> +    # appearing after --args, looked like an option GDB might be able
>> +    # to process, e.g. started with a dash, then GDB would try to
>> +    # process it.  This would mean all leave GDB in a broken state,
>
> Maybe a typo in "mean all". It doesn´t sound clear to me. Maybe you 
> meant "we'll"?
>
> If you think it is worth it, we could mention the specific example of 
> the invocation that ran into issues.

I made the fixes you suggested, and pushed this patch.

Thanks,
Andrew