Ticket #858 (closed defect: fixed)

Opened 3 years ago

Last modified 5 months ago

run-program arguments on Windows

Reported by: rme Owned by: gb
Priority: normal Milestone: Clozure CL 1.9
Component: other Version: trunk
Keywords: run-program Cc: greg@…

Description (last modified by rme) (diff)

On Windows, one creates a new process with  CreateProcess, which accepts a string parameter that contains the command line for the newly created process.

In the newly created process, C runtime code then parses this string and constructs argc and argv[]. The rules used to do this are described by  http://msdn.microsoft.com/en-us/library/a1y7w461.aspx.

We need to apply the inverse of those rules to the command and arguments given to run-program so that the the newly created process sees the same argv[] that the user provided to run-program.

Currently, we just join the all the argument strings together with #\space and call it a day.

Change History

comment:3 Changed 3 years ago by gb

  • Owner set to gb
  • Status changed from new to assigned

comment:4 Changed 3 years ago by rme

  • Milestone set to Clozure CL 1.8

comment:5 Changed 2 years ago by fare

Note that on Windows, it might make sense to expose the ability to execute a command line directly, so as to invoke any program, even those that do not follow the standard argument parsing convention. Executing a program from a list of arguments can (and must) be built on top of that, anyway. That's how I do it in xcvb-driver for allegro on Windows.

comment:6 Changed 2 years ago by fare

In other words, the ability to provide the exact, unescaped, argument that will be passed to CreateProcess? is a *feature* to be preserved. The confusing of it with the interface that on Unix passes arguments as to a C program, is a bug.

API suggestion: If the ARGS argument is T, then the COMMAND argument is passed to CreateProcess?. If the ARGS argument satisfies LISTP, then the COMMAND and ARGS arguments are properly escaped and collated before being passed to CreateProcess?.

comment:7 Changed 23 months ago by pfeilgm

  • Cc greg@… added

comment:8 Changed 23 months ago by fare

NB: Here's a permanent link to a working version of the code.

 http://common-lisp.net/gitweb?p=projects/xcvb/xcvb.git;a=blob;f=driver.lisp;h=4fb5e6701c8e82082ecab31e6bb1f946d09ede55;js=1#l1448

For the latest version, see driver.lisp in HEAD.

Once again, please both make it right and leave the ability to call CreateProcess? with an explicit string, as it's necessary for some programs that do not follow the standard C API for arguments.

comment:9 Changed 23 months ago by gb

  • Status changed from assigned to closed
  • Resolution set to fixed

(In [15397]) Windows version of RUN-PROGRAM and related functions: RUN-PROGRAM allows its "args" argument to be a SIMPLE-STRING (as well as a list of SIMPLE-STRINGs).

Use MAKE-WINDOWS-COMMAND-LINE (not just JOIN-STRINGS) to build the command-line for CreateProcess? from a cons of the RUN-PROGRAM "program name" and "args" arguments. A literal string "args" argument is passed verbatim to the command line; the program name and any arguments in a list-typed "args" arguments are processd by surrounding space/tab with double-quotes and prefixing literal double-quote characters with backslash; all other characters are passed verbatim to the command line and arguments are separated by spaces.

This seems to be at least a first approximation of the rules used by the MSVC runtime, though I haven't seen those rules written down anywhere. (The program name is processed by the OS and any quoting of it has to be done in a canonical way.)

This may fix ticket:858. How would anyone know ?

comment:10 Changed 23 months ago by fare

  • Status changed from closed to reopened
  • Resolution fixed deleted

I don't have a Windows machine anymore, but I can unearth one for testing; can you package a binary?

However, looking at the commit, you don't seem to properly escape backslash itself. It has quite elaborate quoting rules. Once again, you can freely take code from xcvb-driver, which was tested to work properly.

To test whether it works, I suggest having an executable in a path with spaces (and quotes?), and arguments with backslashes -- typically, the executed program would be CCL itself and the argument would be --eval followed by some simple expression with all the characters needing to be escaped.

Finally, I think it would be a nice feature to provide an escape so that the user could specify the raw command-line directly, and thus be able to run some programs (most importantly, CMD.EXE itself, it seems) that do not follow the C API for arguments. Once again, I suggest that either program or args being T, or program being NIL, would be a nice way to do it.

Oh, and - for backwards compatibility in xcvb-driver, how do I distinguish versions with escaping support from versions without such support?

comment:11 Changed 23 months ago by gb

  • Status changed from reopened to closed
  • Resolution set to fixed

(In [15398]) Sequences of one or more backslashes that precede a double-quote character or whitespace need to be escaped (doubled).

Fixes ticket:858 in the trunk.

Changes made to the trunk are often incorporated in the next release. We generally assume that people can call REBUILD-CCL for themselves when there aren't any bootstrapping issues involved (and there aren't here); we do often provide kernel binaries for Windows since (a) it can be tricky to install the necessary toolchain and support and (b) the available Windows toolchains are often broken.

comment:12 follow-up: ↓ 13 Changed 23 months ago by fare

The checkin looks like it handles escaping of backslash, though ideally, one would dig unit-tests from some place on the net. (If you find them, I'm a taker for the xcvb-driver-test system.)

Now, you don't provide a way to call CreateProcess? directly with a raw string anymore. Is that on purpose, or have you just not gotten to it yet? Scripting with CMD.EXE importantly requires this ability; possibly other interesting binaries, too.

Did you or can you update the checked in kernel binaries so I may test it tonight? I never installed a toolchain on Windows. Is it documented?

comment:13 in reply to: ↑ 12 ; follow-up: ↓ 14 Changed 23 months ago by gb

Replying to fare:

Now, you don't provide a way to call CreateProcess? directly with a raw string anymore.

This didn't change in r15398; I have no idea why you think that it did.

$ cat ShowParams.c
#include "stdio.h"
int main(int argc, char* argv[])
{
  int i;

  for (i = 0; i < argc; ++i)
  {
    printf("param %d = ",i);
    puts(argv[i]);
  }
  return 0;
} 
$ i686-pc-mingw32-gcc -o ShowParams.exe ShowParams.c
$ ccl
? (run-program "ShowParams.exe" '("a\\\\\" b c" "d" "e")
             :output t)
param 0 = ShowParams.exe
param 1 = a\\" b c
param 2 = d
param 3 = e
#<EXTERNAL-PROCESS (ShowParams.exe a\\" b c d ...)[NIL] (EXITED : 0) #xC5AF16E>
? (run-program "ShowParams.exe" '("a\\\\b c" "d" "e")
             :output t)
param 0 = ShowParams.exe
param 1 = a\\b c
param 2 = d
param 3 = e
#<EXTERNAL-PROCESS (ShowParams.exe a\\b c d ...)[NIL] (EXITED : 0) #xC64843E>
? (run-program "ShowParams.exe" "Simple string with embedded spaces"
             :output t)
param 0 = ShowParams.exe
param 1 = Simple
param 2 = string
param 3 = with
param 4 = embedded
param 5 = spaces
#<EXTERNAL-PROCESS (ShowParams.exe . Simple string with embedded spaces)[NIL] (EXITED : 0) #xC646906>
? 

Did you or can you update the checked in kernel binaries so I may test it tonight?

This has nothing to do with the kernel. The windows kernels in svn are able to run the images in svn, and those images are able to compile these changes and build new versions of themselves.

Windows doesn't allow a running executable file to be overwritten, so doing:

? (rebuild-ccl :clean t)

is the easiest way to generate a new image without rebuilding the kernel.

comment:14 in reply to: ↑ 13 ; follow-up: ↓ 15 Changed 23 months ago by fare

Replying to gb:

Replying to fare:

Now, you don't provide a way to call CreateProcess? directly with a raw string anymore.

This didn't change in r15398; I have no idea why you think that it did.

By failing to escape arguments, you provided a defacto interface to raw string: just put the raw string as first (program) argument, and voilà, there you were. I could explicitly quote arguments on top of that if I wanted.

Now that you always escape arguments, there is no such interface anymore, which is a shame.

I can file a separate bug for that.

Did you or can you update the checked in kernel binaries so I may test it tonight?

This has nothing to do with the kernel. The windows kernels in svn are able to run the images in svn, and those images are able to compile these changes and build new versions of themselves.

I suppose my question is whether there is a way for me to test without installing a Windows SDK. Or if not, whether you have some documentation on how to get such a SDK.

comment:15 in reply to: ↑ 14 Changed 23 months ago by gb

Replying to fare:

Replying to gb:

Replying to fare:

Now, you don't provide a way to call CreateProcess? directly with a raw string anymore.

This didn't change in r15398; I have no idea why you think that it did.

By failing to escape arguments, you provided a defacto interface to raw string: just put the raw string as first (program) argument, and voilà, there you were. I could explicitly quote arguments on top of that if I wanted.

Now that you always escape arguments, there is no such interface anymore, which is a shame.

I can file a separate bug for that.

The change in r15398 is exactly what the commit message above says it is.

It is not whatever it is that you imagine it to be.

To elaborate on what the commit messages above say:

  1. RUN-PROGRAM accepts a "program name" and "program arguments" as positional arguments and also accepts several keyword arguments.
  1. Historically, the "program arguments" argument has been a list of strings. In

r15397 in the trunk, this argument can be a single string on Windows. The program name and the elements of a list of program arguments are processed according to my understanding of the MS C quoting rules and copied to the command-line component of CreateProcess?'s argument; if the "program arguments" argument is a string, it's copied verbatim to the command line (and can therefore be used to provide arguments to a program that doesn't parse its command line according to MS C rules.)

  1. In r15397, when trying to enforce C quoting rules, sequences of backslashes that preceded double-quotes or whitespace were not themselves quoted/escaped by backslashes. In r15398, they are. This is the only change between r15397 and r15398.

This is not extremely complicated, but it does require at least a modest effort to understand. I think that you'd find that effort worthwhile, and it seems clear that whatever you're doing instead isn't working very well.

Did you or can you update the checked in kernel binaries so I may test it tonight?

This has nothing to do with the kernel. The windows kernels in svn are able to run the images in svn, and those images are able to compile these changes and build new versions of themselves.

I suppose my question is whether there is a way for me to test without installing a Windows SDK. Or if not, whether you have some documentation on how to get such a SDK.

You would only need to install a C compiler and related tools if you needed to compile C code. You don't need to compile any C code.

comment:16 Changed 5 months ago by avodonosov

Is there a way to distinguish a CCL version with this bug fixed?

I have a code with a workaround for the old behavior

  • my code itself escapes program arguments.

For the new CCL versions, where the bug is fixed, my code should not escape the arguments.

What is the best way to conditionalize such code?

comment:17 Changed 5 months ago by avodonosov

Is it safe to rely on :ccl-1.9 presence in *features* in all versions when this bug is fixed? Even in next CCL versions (1.10, 1.11 and so on)?

I assume so, because today's CCL 1.9 keeps all elder version tags in *features*: :ccl-1.2 :ccl-1.3 :ccl-1.4 :ccl-1.5 :ccl-1.6 :ccl-1.7 :ccl-1.8 :ccl-1.9

In other words, presence of :ccl-1.9 should be interpreted as "ccl version >= 1.9"

If so, then my code which wants to run-program portably on various versions of CCL,on can do the following for each argument:

   (if (and (member :windows *features*)
            (member :ccl *features*)
            (not (member :ccl-1.9 *features*)))
      (escape arument)
      argument)  
Note: See TracTickets for help on using tickets.