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

[Andrew Burgess <aburgess@redhat.com>]

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