rschupp / PAR-Packer

(perl) Generate stand-alone executables, perl scripts and PAR files https://metacpan.org/pod/PAR::Packer
Other
48 stars 13 forks source link

Fix argument list passed to spawnvpe() call on Windows #2

Closed steve-m-hay closed 7 years ago

steve-m-hay commented 7 years ago

The first argument in the list should be the program name (or path) being spawned (i.e. the same as the second argument passed to spawnvpe() itself: my_perl, in this case); the remainder of the argument list are the arguments for the spawned process. See: https://msdn.microsoft.com/en-us/library/20y988d2.aspx#Anchor_1 https://msdn.microsoft.com/en-us/library/h565xwht.aspx#Anchor_0

Also, we can simply use spawnvp() rather than spawnvpe() since the environment being passed to the spawned process is only the current process's environ, which the spawned process inherits anyway in the case of spawnvp(). See: https://msdn.microsoft.com/en-us/library/20y988d2.aspx#Anchor_2

This also avoids referencing environ, which stdlib.h #defines as _environ, which is not exposed if _CRT_BEST_PRACTICES_USAGE is #defined (in the Windows 10 SDK used by VS2015).

rschupp commented 7 years ago

I agree on dropping the explicit environ argument. But regarding argv, you do (in Perl terms) unshift(argv, my_perl) Shouldn't that simply be argv[0] = my_perl? Have you checked by e.g. packing pp -o show_argv.exe -E "say qq[@ARGV]" that ./show_argv foo bar quux will just print foo bar quux and not an additional string before that?

steve-m-hay commented 7 years ago

argv[0] = my_perl would not be correct if I understand things correctly. What (I believe) we're doing here is launching my_perl and passing it all the arguments of the boot program, including the boot program's own name as an additional argument because that's what my_perl -- the custom Perl interpreter -- is going to be running.

So if the boot program is called boot.exe and the custom Perl interpreter is called parl.exe, and the following command is what's running:

boot.exe foo bar quux

(in which boot.exe's argv[0] is "boot.exe", argv[1] is "foo", etc) then what we're trying to do is to launch parl.exe like this:

parl.exe boot.exe foo bar quux

Thus, my_perl needs prepending to the full argv, rather than overwriting argv[0]. If we overwrote argv[0] then the command we'd be spawning would be:

parl.exe foo bar quux

which isn't right.

A weird fact, however, is that it works! I can only surmise this is because parl.exe must be reading its arguments from the PAR_ARG* environment variables that have been set up, in which the correct value still exists in PAR_ARGV_0.

In fact, the following also works (i.e. your show_argv.exe example correctly prints out all the arguments, despite them all apparently having been thrown away!), proving that the spawned program must actually be getting its arguments from the environment:

my_argv = (char**)malloc(2 * sizeof(char**));
my_argv[0] = my_perl;
my_argv[1] = NULL;
rc = spawnvp(P_WAIT, my_perl, (const char* const*)my_argv);

So, in short, I believe my patch is correct, and I've double-checked that your show_argv.exe does the right thing, printing out just foo bar quux, without anything else before it.

Alternatively, feel free to go with some variation of the four-line code snippet above since that works equally well (and is arguably less misleading about where the arguments are coming from!). I guess it could be more simply rewritten as:

argv[0] = my_perl;
argv[1] = NULL;
rc = spawnvp(P_WAIT, my_perl, (const char* const*)argv);
rschupp commented 7 years ago

WRT PARARGV*: PAR::Packer never fails to surprise with weird stuff for no reason - maybe there once was one, but it's lost in the mists of time. I just removed this wart with no apparent consequences. BTW: he intention always was to call parl.exe with the original arguments. The name of the packed executable is passed by other means.

steve-m-hay commented 7 years ago

Apologies for not replying to your earlier comment sooner.

The argument list given to spawnvp*() must start with the program being spawned (parl.exe). See https://msdn.microsoft.com/en-us/library/h565xwht.aspx#Anchor_0 ("The argument argv[0] is usually a pointer to a path in real mode or to the program name in protected mode, and argv[1] through argv[n] are pointers to the character strings forming the new argument list.")

Originally, I thought that the current program's name (boot.exe) was needed as an argument too, hence I prepended parl.exe on the front of the argument list. Now that the boot.exe name doesn't need to be passed through, we simply need to overwrite it with the parl.exe name instead.

So I think the current code isn't quite correct. Your previous suggestion of argv[0] = my_perl is now what's required.

rschupp commented 7 years ago

So I think the current code isn't quite correct. Your previous suggestion of argv[0] = my_perl is now what's required.

Done. Though I dunno whether anyone checks argv[0] - on Linux definitely not