Re: [PATCH 3/4] [PR gdbserver/25893]: Use construct_inferior_arguments which handles special chars

[Simon Marchi <simark@simark.ca>]

On 2020-05-07 4:09 p.m., Simon Marchi wrote:
> On 2020-04-29 7:16 a.m., Michael Weghorn via Gdb-patches wrote:
>> Use the construct_inferior_arguments function instead of
>> stringify_argv to construct a string from the program
>> arguments in those places where that one is then passed
>> to fork_inferior, since construct_inferior_arguments
>> properly takes care of special characters, while
>> stringify_argv does not.
>> Using construct_inferior_arguments seems "natural", since its
>> documentation also mentions that it "does the
>> same shell processing as fork_inferior".
>>
>> This makes gdbserver properly handle program args containing special
>> characters, e.g. (example from PR25893)
>>
>>   $ gdbserver localhost:50505 myprogram "hello world"
>>
>> now properly handles "hello world" as a single arg, not two separate
>> ones ("hello", "world").
>>
>> I'm not sure regarding the two remaining uses of function stringify_argv
>> (in win32_process_target::create_inferior and
>> nto_process_target::create_inferior) whose result is not passed to
>> fork_inferior and don't know how the arg processing in the creation
>> of processes on those targets exactly works, so left them unchanged
>> for now.
> 
> I'd say, do the change the best you can for win32 and nto.  We'll be
> able to test win32.  Regarding nto, it hasn't seen patches for years,
> it most likely doesn't even build.  And the toolchain not being easily
> available, we can't do much about it.  I think we should actually
> consider removing this port, unless somebody steps up to maintain it.
> Same for lynx.
> 
> This will allow removing stringify_argv, and avoid having duplicate
> broken functionality of construct_inferior_arguments.

While trying out stuff to review patch 4, I noticed a crash introduced by this patch.

In one terminal, run gdbserver with:

$ ./gdbserver --once --multi :1234

In another, run GDB with:

$ ./gdb /bin/ls -ex 'set pagination off' -ex 'set remote exec-file /bin/ls' -ex 'target extended-remote :1234' -ex 'run hello'

Doing so segfaults gdbserver for me.  Could you look into it?

The commands above try to run a new process through gdbserver.  The args are passed
to GDB using the remote protocol.  You can add -ex 'set debug remote 1' before the run
part to see the exchanged packets, the args are hex-encoded in the vRun packet.

Simon