Re: [PATCH v3 5/5] Extend "set cwd" to work on gdbserver

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Friday, September 22 2017, Eli Zaretskii wrote:

>> From: Sergio Durigan Junior <sergiodj@redhat.com>
>> Cc: Pedro Alves <palves@redhat.com>,	Sergio Durigan Junior <sergiodj@redhat.com>
>> Date: Thu, 21 Sep 2017 18:59:26 -0400
>> 
>> This is the "natural" extension necessary for the "set cwd" command
>> (and the whole "set the inferior's cwd" logic) to work on gdbserver.
>> 
>> The idea here is to have a new remote packet, QSetWorkingDir (name
>> adopted from LLDB's extension to the RSP, as can be seen at
>> <https://raw.githubusercontent.com/llvm-mirror/lldb/master/docs/lldb-gdb-remote.txt>),
>> which sends an hex-encoded string representing the working directory
>> that gdbserver is supposed to cd into before executing the inferior.
>> The good thing is that since this feature is already implemented on
>> nat/fork-inferior.c, all gdbserver has to do is to basically implement
>> "set_inferior_cwd" and call it whenever such packet arrives.
>
> This once again raises the issue of whether to send expanded or
> unexpanded directory down the wire, and if unexpanded, then what is
> the meaning of the default "~" in the inferior.

FWIW, I decided (based on another message by Pedro) to modify the
behaviour of "set cwd" without arguments.  Before it was setting the
inferior's cwd as "~", but now it clears out whatever cwd that has been
specified by the user.

But of course, the user can still do "set cwd ~".  We do not expand
paths on the host, as I explained in another message, so gdbserver will
see "~" coming down the wire.  Then, when the inferior is to be started,
gdbserver will perform the path expansion based on what
"gdb_tilde_expand" (i.e., "glob") does.

>> +  if (inferior_cwd != NULL)
>> +    {
>> +      size_t cwdlen = strlen (inferior_cwd);
>> +
>> +      wcwd = alloca ((cwdlen + 1) * sizeof (wchar_t));
>> +      mbstowcs (wcwd, inferior_cwd, cwdlen + 1);
>> +    }
>
> no error checking of the mbstowcs conversion?

Sorry, I am not a Windows programmer.  Other places in the code also
don't check for errors.  I'd be happy to improve this code, but I refuse
to touch a Windows machine so I'm doing this all this without any
testing.  But please, feel absolutely free to point out how this code
should look like.

>>    ret = CreateProcessW (wprogram, /* image name */
>>  			wargs,    /* command line */
>>  			NULL,     /* security, not supported */
>> @@ -586,7 +595,7 @@ create_process (const char *program, char *args,
>>  			FALSE,    /* inherit handles, not supported */
>>  			flags,    /* start flags */
>>  			NULL,     /* environment, not supported */
>> -			NULL,     /* current directory, not supported */
>> +			wcwd,     /* current directory */
>>  			NULL,     /* start info, not supported */
>>  			pi);      /* proc info */
>>  #else
>> @@ -599,7 +608,7 @@ create_process (const char *program, char *args,
>>  			TRUE,     /* inherit handles */
>>  			flags,    /* start flags */
>>  			NULL,     /* environment */
>> -			NULL,     /* current directory */
>> +			inferior_cwd,     /* current directory */
>>  			&si,      /* start info */
>>  			pi);      /* proc info */
>
> Once again, this feeds CreateProcess with an unexpanded directory,
> which AFAIU will not work if the directory includes "~".

You're right; I've changed that now.

>> +static void
>> +extended_remote_set_inferior_cwd (struct remote_state *rs)
>> +{
>> +  if (packet_support (PACKET_QSetWorkingDir) != PACKET_DISABLE)
>> +    {
>> +      const char *inferior_cwd = get_inferior_cwd ();
>> +
>> +      if (inferior_cwd != NULL)
>> +	{
>> +	  std::string hexpath = bin2hex ((const gdb_byte *) inferior_cwd,
>> +					 strlen (inferior_cwd));
>> +
>
> Shouldn't this do some encoding conversion, from the GDB charset to
> the target charset, before encoding in hex?

I don't know.  There's nothing related to charset on gdb/gdbserver/, but
then again I don't know if we've ever encountered a case that demanded
conversions.  I can investigate this a bit more.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/