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

Andrew Burgess
2025-09-17T08:58:05Z

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