[RFA/commit/Windows] run program with space in path to exe.

[RFA/commit/Windows] run program with space in path to exe.

[Joel Brobecker]

The following works...

    % gdb c:\path to exe\foo.exe
    (gdb) start

... unless a file or directory called "c:\path" or "c:\path to" exists.
This is what happens in the latter case:

    (gdb) start
    [...]
    Error creating process C:\path to exe\foo.exe (error 193).

This is because we are calling CreateProcess (et al) without specifying
the lpApplicationName, so Windows determines the name of the executable
using the second argument, which is the entire command line.  This
command line is a space-separated list of tokens, so the space in
the path to the executable which potentially creates an ambiguity.
The ambiguity is automatically resolved unless we're in the situation
above.

The solution, as suggested by the MSDN documentation for CreateProcess
is to quote the executable name.

gdb/ChangeLog:

        * windows-nat.c (windows_create_inferior) [!__CYGWIN__]:
        New local variable args_len.
        Quote the name of the executable when computing the command line.

Tested on x86-windows. Fixes the problem, with no regression.

A few remarks on the patch:

  1. As shown in the ChangeLog entry, this only deal with the MinGW
     case.  I think we have the same issue with cygwin, except it is
     a little more complicated, because there are two cases: startup
     with shell, vs startup without shell, multiplied by wide characters
     vs non-wide characters.

     It'd be nice if someone who uses the cygwin version were to fix
     the problem there as well. I don't mind writing a patch, but it
     will be just barely tested, since I do not have a testing
     environment for cygwin. I'm also waiting to see if this patch
     is accepted.

  2. It seemed much simpler to use xsnprintf rather than strcat to
     compute the command line, especially now that it has a few more
     elements in it.  So I used that instead.

  3. Theoretically, I have a feeling that we're breaking the case
     where the executable name contains a double quote in it.
     But, I think that's not something anyone would do, and I don't
     think I'd want to complexify the current code just to handle
     this theoretical case, particularly since it didn't work anyway!

         $ gdb -q 'foo"bar.exe'
         foo"bar.exe: No such file or directory.

     So, I don't think anyone is going to complain...

Any objections this patch?

Thanks,
-- 
Joel

---
 gdb/windows-nat.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 905d4bf..5eb8f67 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -2036,6 +2036,7 @@ windows_create_inferior (struct target_ops *ops, char *exec_file,
   char shell[__PMAX]; /* Path to shell */
   char *toexec;
   char *args;
+  size_t args_len;
   HANDLE tty;
   char *w32env;
   char *temp;
@@ -2188,10 +2189,13 @@ windows_create_inferior (struct target_ops *ops, char *exec_file,
     }
 #else
   toexec = exec_file;
-  args = alloca (strlen (toexec) + strlen (allargs) + 2);
-  strcpy (args, toexec);
-  strcat (args, " ");
-  strcat (args, allargs);
+  /* Build the command line, a space-separated list of tokens where
+     the first token is the name of the module to be executed.
+     To avoid ambiguities introduced by spaces in the module name,
+     we quote it.  */
+  args_len = strlen (toexec) + 2 /* quotes */ + strlen (allargs) + 2;
+  args = alloca (args_len);
+  xsnprintf (args, args_len, "\"%s\" %s", toexec, allargs);
 
   flags |= DEBUG_ONLY_THIS_PROCESS;
 
-- 
1.7.9.5

Re: [RFA/commit/Windows] run program with space in path to exe.

[Eli Zaretskii]

> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Joel Brobecker <brobecker@adacore.com>
> Date: Fri, 19 Oct 2012 17:28:45 -0700
> 
> The following works...
> 
>     % gdb c:\path to exe\foo.exe
>     (gdb) start
> 
> ... unless a file or directory called "c:\path" or "c:\path to" exists.
> This is what happens in the latter case:
> 
>     (gdb) start
>     [...]
>     Error creating process C:\path to exe\foo.exe (error 193).
> 
> This is because we are calling CreateProcess (et al) without specifying
> the lpApplicationName, so Windows determines the name of the executable
> using the second argument, which is the entire command line.  This
> command line is a space-separated list of tokens, so the space in
> the path to the executable which potentially creates an ambiguity.
> The ambiguity is automatically resolved unless we're in the situation
> above.

Does it work if you say

  % gdb "\"c:\path to exe\foo.exe\""

instead?

>   3. Theoretically, I have a feeling that we're breaking the case
>      where the executable name contains a double quote in it.

This is impossible, at least in the MinGW case: Windows file names
cannot include the quote character.  See

  http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx

But what happens if the program name is already quoted?  A user can do
that if she realizes the problem in advance, certainly if the program
name is specified at the GDB prompt, as in 'file "c:\foo bar\my.exe"'.
I think we should detect this case and not quote it again.

Also, what about the arguments to the program?  Don't they have the
same issue when you use --args on the GDB command line?

Re: [RFA/commit/Windows] run program with space in path to exe.

[Joel Brobecker]

> Does it work if you say
> 
>   % gdb "\"c:\path to exe\foo.exe\""
> 
> instead?

It doesn't, because GDB thinks that the name of the executable includes
those quotes.

> This is impossible, at least in the MinGW case: Windows file names
> cannot include the quote character.  See
> 
>   http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx

OK, great. I wasn't sure whether this was allowed or not. I was able
to create a file with a double-quote, but that was under cygwin.

> But what happens if the program name is already quoted?  A user can do
> that if she realizes the problem in advance, certainly if the program
> name is specified at the GDB prompt, as in 'file "c:\foo bar\my.exe"'.
> I think we should detect this case and not quote it again.

As per the above - this time, it's GDB that punts, so we never even
get to the point where we can run the program.

> Also, what about the arguments to the program?  Don't they have the
> same issue when you use --args on the GDB command line?

The arguments are always quoted properly by the generic portion of
GDB.  No problem there.

-- 
Joel

Re: [RFA/commit/Windows] run program with space in path to exe.

[Eli Zaretskii]

> Date: Sat, 20 Oct 2012 09:29:36 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > This is impossible, at least in the MinGW case: Windows file names
> > cannot include the quote character.  See
> > 
> >   http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx
> 
> OK, great. I wasn't sure whether this was allowed or not. I was able
> to create a file with a double-quote, but that was under cygwin.

Cygwin employs some dark corners of NTFS to provide Posix-like
features.  A Cygwin GDB will indeed need to cope with quotes as part
of file names in this context.

> > But what happens if the program name is already quoted?  A user can do
> > that if she realizes the problem in advance, certainly if the program
> > name is specified at the GDB prompt, as in 'file "c:\foo bar\my.exe"'.
> > I think we should detect this case and not quote it again.
> 
> As per the above - this time, it's GDB that punts, so we never even
> get to the point where we can run the program.
> 
> > Also, what about the arguments to the program?  Don't they have the
> > same issue when you use --args on the GDB command line?
> 
> The arguments are always quoted properly by the generic portion of
> GDB.  No problem there.

Then I guess you can go ahead and commit the changes.  Thanks.

Checked in: [RFA/commit/Windows] run program with space in path to exe.

[Joel Brobecker]

> Then I guess you can go ahead and commit the changes.  Thanks.

Thanks, Eli. Now checked in.

-- 
Joel

Re: Checked in: [RFA/commit/Windows] run program with space in path to exe.

[Pedro Alves]

On 10/24/2012 02:42 PM, Joel Brobecker wrote:
>> Then I guess you can go ahead and commit the changes.  Thanks.
> 
> Thanks, Eli. Now checked in.

Doesn't gdbserver need the same treatment?

-- 
Pedro Alves

Re: Checked in: [RFA/commit/Windows] run program with space in path to exe.

[Joel Brobecker]

> Doesn't gdbserver need the same treatment?

It doesn't, because it provides the "image name" in addition to the
command line. Although, now that I look at the code, there might
be a different bug. The program is going to start fine, but I am guessing
that argc/argv is going to be miscomputed. I'll put it on my list...

-- 
Joel

Re: Checked in: [RFA/commit/Windows] run program with space in path to exe.

[Pedro Alves]

On 10/24/2012 03:45 PM, Joel Brobecker wrote:
>> Doesn't gdbserver need the same treatment?
> 
> It doesn't, because it provides the "image name" in addition to the
> command line.

Ah.  The "image name" is a mandatory argument on Windows CE, not optional.
Are you saying that that would be an alternative fix for gdb?  Or is there a
downside?  (Wondering in the sake of convergence in the code bases.)

> Although, now that I look at the code, there might
> be a different bug. The program is going to start fine, but I am guessing
> that argc/argv is going to be miscomputed. I'll put it on my list...

Thanks.

-- 
Pedro Alves

Re: Checked in: [RFA/commit/Windows] run program with space in path to exe.

[Joel Brobecker]

> Ah.  The "image name" is a mandatory argument on Windows CE, not optional.
> Are you saying that that would be an alternative fix for gdb?  Or is there a
> downside?  (Wondering in the sake of convergence in the code bases.)

We can provide the image name, but we still need to quote it in
the command line, because otherwise, argv is improperly computed.
For instance, instead of being:

  ["/path/to/space dir/exe", "arg1"]

The program would see:

  ["/path/to/space", "dir/exe", "arg1"]

/me wishes that CreateProcess was taking a list of arguments, rather
than a string of space-separated tokens....

-- 
Joel

Re: Checked in: [RFA/commit/Windows] run program with space in path to exe.

[Pedro Alves]

On 10/24/2012 04:01 PM, Joel Brobecker wrote:
>> Ah.  The "image name" is a mandatory argument on Windows CE, not optional.
>> Are you saying that that would be an alternative fix for gdb?  Or is there a
>> downside?  (Wondering in the sake of convergence in the code bases.)
> 
> We can provide the image name, but we still need to quote it in
> the command line, because otherwise, argv is improperly computed.
> For instance, instead of being:
> 
>   ["/path/to/space dir/exe", "arg1"]
> 
> The program would see:
> 
>   ["/path/to/space", "dir/exe", "arg1"]
> 

I see, thanks.

> /me wishes that CreateProcess was taking a list of arguments, rather
> than a string of space-separated tokens....

:-)

-- 
Pedro Alves

Re: Checked in: [RFA/commit/Windows] run program with space in path to exe.

[Christopher Faylor]

On Wed, Oct 24, 2012 at 11:01:51AM -0400, Joel Brobecker wrote:
>> Ah.  The "image name" is a mandatory argument on Windows CE, not optional.
>> Are you saying that that would be an alternative fix for gdb?  Or is there a
>> downside?  (Wondering in the sake of convergence in the code bases.)
>
>We can provide the image name, but we still need to quote it in
>the command line, because otherwise, argv is improperly computed.
>For instance, instead of being:
>
>  ["/path/to/space dir/exe", "arg1"]
>
>The program would see:
>
>  ["/path/to/space", "dir/exe", "arg1"]
>
>/me wishes that CreateProcess was taking a list of arguments, rather
>than a string of space-separated tokens....

Yeah, tell me about it.  We actually pass a real argv list in Cygwin.
The "one long command line" method is rather dumb.  Much of the Windows
API is not that bad but this is, IMO, one glaring flaw.

cgf