bpo-40138: Fix Windows os.waitpid() for large exit code

[vstinner]

Fix the Windows implementation of os.waitpid() for exit code
larger than "INT_MAX >> 8". The exit status is now interpreted as an
unsigned number.

os.waitstatus_to_exitcode() now accepts wait status larger than
INT_MAX.

https://bugs.python.org/issue40138

[vstinner]

@eryksun: Would you mind to review this change?

[vstinner]

On Windows, os.waitpid() truncates the exit status if it's larger than INT_MAX >> 8. I think that Python 3.7 and 3.8 should also be fixed.

[vstinner]

cc @zooba

[eryksun]

spawnv concatenates the arguments into a command line that gets passed to CreateProcessW in the lpCommandLine parameter. No shell is involved here. Unlike Python's subprocess.list2cmdline, the CRT's spawn family does not create a command line according to MSVC rules. It simply concatenates with spaces, which leaves it up to the caller to ensure that each argument is quoted and escaped according to the rules of the target application.

You can avoid the need to escape quotes in an argument by using only single quotes, which have no special meaning in MSVC argv parsing. For example: args = ['python', '-c', f'"{code}"'], where code contains no double quote characters.

There's no need for args[0] to be a fully-qualified path with spawnv. Actually, what you're doing by setting args[0] = sys.executable is wrong. It only works because the test build's path to "python.exe" contains no spaces. If the path has spaces and isn't quoted, then C argv[0] will only include up to the first space in the path, and the rest will be in argv[1], and so on. If you want to use the full path, it has to be quoted. For example, where sys.executable is "C:\Program Files\Python38\python.exe":

>>> os.spawnv(os.P_WAIT, sys.executable, [f'"{sys.executable}"', '-V'])
Python 3.8.1
0

Failing example without quoting:

>>> os.spawnv(os.P_WAIT, sys.executable, [sys.executable, '-V'])
C:\Program: can't open file 'Files\Python38\python.exe': [Errno 2] No such file or directory
2

Preferably, just use unqualified "python":

>>> os.spawnv(os.P_WAIT, sys.executable, ['python', '-V'])
Python 3.8.1
0

[vstinner]

Alright, I didn't think about quoting sys.executable. I fixed the test.

[eryksun]

Is there any possibility that os.waitpid could simply stop shifting the status value by a byte in Windows? With the introduction of os.waitstatus_to_exitcode, scripts no longer need to make assumptions about the status value format, and the Windows implementation of waitstatus_to_exitcode could simply return the status value unchanged.

[vstinner]

Is there any possibility that os.waitpid could simply stop shifting the status value by a byte in Windows?

That would be a backward incompatible changes. If you look at changes that I made in https://bugs.python.org/issue40094 to use os.waitstatus_to_exitcode(), you can see that even in the Python stdlib, plently of code used directly "status >> 8" without even checking os.WIFEXITED().

I don't think that it's worth it to change how waitpid() encodes the exit code in the exit status. You should now use os.waitstatus_to_exitcode() to write portable code.