Re: [PATCH v3 5/6] [PR gdbserver/25893]: Use construct_inferior_arguments which handles special chars

Michael Weghorn
2020-05-13T09:58:11Z

On 13/05/2020 02.54, Simon Marchi wrote:
> On 2020-05-12 8:52 p.m., Simon Marchi wrote:
>> On 2020-05-12 11:42 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 (linux-low, lyn-low), 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".
>>>
>>> Since construct_inferior_args has been extended to do
>>> proper quoting for Windows shells in commit
>>> 5d60742e2dd3c9b475dce54b56043a358751bbb8
>>> ("Fix quoting of special characters for the MinGW build.",
>>> 2012-06-12), use it for the Windows case as well.
>>> (I could not test that case myself, though.)
>>>
>>> Adapt handling of empty args in function 'handle_v_run'
>>> in gdbserver/server.cc to just insert an empty string
>>> for an empty arg, since that one is now properly handled
>>> in 'construct_inferior_arguments' already (and inserting
>>> a "''" string in 'handle_v_run' would otherwise
>>> cause that one to be treated as a string literally
>>> containing two quote characters, which
>>> 'construct_inferior_args' would preserve by adding
>>> extra escaping).
>>>
>>> This makes gdbserver properly handle program args containing special
>>> characters (like spaces), 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").
>>>
>>> 2020-05-12 Michael Weghorn <m.weghorn@posteo.de>
>>>
>>> PR gdbserver/25893
>>> * linux-low.cc (linux_process_target::create_inferior),
>>> lynx-low.cc (lynx_process_target::create_inferior),
>>> win32-low.cc (win32_process_target::create_inferior): Use
>>> construct_inferior_arguments instead of stringify_argv
>>> to get string representation which properly escapes
>>> special characters.
>>> * server.cc (handle_v_run): Just pass empty program arg
>>> as such, since any further processing is now handled via
>>> construct_inferior_arguments
>>
>> This patch LGTM.
>>
>> Simon
>>
>
> I forgot to mention, I built this on mingw-w64 and confirmed that it
> fixed the same problem. It also handles empty arguments correctly, both
> on gdbserver's command line and "run".
>
> Unfortunately, I wasn't able to run my test case, but that's because of
> separate issues.

Thanks a lot for testing it actually works and all reviewing!

Michael