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

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

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>