[Python-checkins] bpo-40138: Fix Windows os.waitpid() for large exit code (GH-19637)

Wed Apr 22 10:30:43 EDT 2020

https://github.com/python/cpython/commit/9bee32b34e4fb3e67a88bf14d38153851d4c4598
commit: 9bee32b34e4fb3e67a88bf14d38153851d4c4598
branch: master
author: Victor Stinner <vstinner at python.org>
committer: GitHub <noreply at github.com>
date: 2020年04月22日T16:30:35+02:00
summary:
bpo-40138: Fix Windows os.waitpid() for large exit code (GH-19637)
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.
files:
A Misc/NEWS.d/next/Library/2020-04-22-00-05-10.bpo-40138.i_oGqa.rst
M Lib/test/test_os.py
M Modules/clinic/posixmodule.c.h
M Modules/posixmodule.c

diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
index 73dc064d5ff75..74aef47243428 100644
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -2789,40 +2789,66 @@ def test_getppid(self):
 # We are the parent of our subprocess
 self.assertEqual(int(stdout), os.getpid())
 
+ def check_waitpid(self, code, exitcode, callback=None):
+ if sys.platform == 'win32':
+ # On Windows, os.spawnv() simply joins arguments with spaces:
+ # arguments need to be quoted
+ args = [f'"{sys.executable}"', '-c', f'"{code}"']
+ else:
+ args = [sys.executable, '-c', code]
+ pid = os.spawnv(os.P_NOWAIT, sys.executable, args)
+
+ if callback is not None:
+ callback(pid)
+
+ # don't use support.wait_process() to test directly os.waitpid()
+ # and os.waitstatus_to_exitcode()
+ pid2, status = os.waitpid(pid, 0)
+ self.assertEqual(os.waitstatus_to_exitcode(status), exitcode)
+ self.assertEqual(pid2, pid)
+
 def test_waitpid(self):
- args = [sys.executable, '-c', 'pass']
- # Add an implicit test for PyUnicode_FSConverter().
- pid = os.spawnv(os.P_NOWAIT, FakePath(args[0]), args)
- support.wait_process(pid, exitcode=0)
+ self.check_waitpid(code='pass', exitcode=0)
 
 def test_waitstatus_to_exitcode(self):
 exitcode = 23
- filename = support.TESTFN
- self.addCleanup(support.unlink, filename)
+ code = f'import sys; sys.exit({exitcode})'
+ self.check_waitpid(code, exitcode=exitcode)
 
- with open(filename, "w") as fp:
- print(f'import sys; sys.exit({exitcode})', file=fp)
- fp.flush()
- args = [sys.executable, filename]
- pid = os.spawnv(os.P_NOWAIT, args[0], args)
+ with self.assertRaises(TypeError):
+ os.waitstatus_to_exitcode(0.0)
 
- pid2, status = os.waitpid(pid, 0)
- self.assertEqual(os.waitstatus_to_exitcode(status), exitcode)
- self.assertEqual(pid2, pid)
+ @unittest.skipUnless(sys.platform == 'win32', 'win32-specific test')
+ def test_waitpid_windows(self):
+ # bpo-40138: test os.waitpid() and os.waitstatus_to_exitcode()
+ # with exit code larger than INT_MAX.
+ STATUS_CONTROL_C_EXIT = 0xC000013A
+ code = f'import _winapi; _winapi.ExitProcess({STATUS_CONTROL_C_EXIT})'
+ self.check_waitpid(code, exitcode=STATUS_CONTROL_C_EXIT)
+
+ @unittest.skipUnless(sys.platform == 'win32', 'win32-specific test')
+ def test_waitstatus_to_exitcode_windows(self):
+ max_exitcode = 2 ** 32 - 1
+ for exitcode in (0, 1, 5, max_exitcode):
+ self.assertEqual(os.waitstatus_to_exitcode(exitcode << 8),
+ exitcode)
+
+ # invalid values
+ with self.assertRaises(ValueError):
+ os.waitstatus_to_exitcode((max_exitcode + 1) << 8)
+ with self.assertRaises(OverflowError):
+ os.waitstatus_to_exitcode(-1)
 
 # Skip the test on Windows
 @unittest.skipUnless(hasattr(signal, 'SIGKILL'), 'need signal.SIGKILL')
 def test_waitstatus_to_exitcode_kill(self):
+ code = f'import time; time.sleep({support.LONG_TIMEOUT})'
 signum = signal.SIGKILL
- args = [sys.executable, '-c',
- f'import time; time.sleep({support.LONG_TIMEOUT})']
- pid = os.spawnv(os.P_NOWAIT, args[0], args)
 
- os.kill(pid, signum)
+ def kill_process(pid):
+ os.kill(pid, signum)
 
- pid2, status = os.waitpid(pid, 0)
- self.assertEqual(os.waitstatus_to_exitcode(status), -signum)
- self.assertEqual(pid2, pid)
+ self.check_waitpid(code, exitcode=-signum, callback=kill_process)
 
 
 class SpawnTests(unittest.TestCase):
@@ -2884,6 +2910,10 @@ def test_spawnv(self):
 exitcode = os.spawnv(os.P_WAIT, args[0], args)
 self.assertEqual(exitcode, self.exitcode)
 
+ # Test for PyUnicode_FSConverter()
+ exitcode = os.spawnv(os.P_WAIT, FakePath(args[0]), args)
+ self.assertEqual(exitcode, self.exitcode)
+
 @requires_os_func('spawnve')
 def test_spawnve(self):
 args = self.create_args(with_env=True)
diff --git a/Misc/NEWS.d/next/Library/2020-04-22-00-05-10.bpo-40138.i_oGqa.rst b/Misc/NEWS.d/next/Library/2020-04-22-00-05-10.bpo-40138.i_oGqa.rst
new file mode 100644
index 0000000000000..ad5faf3865751
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-04-22-00-05-10.bpo-40138.i_oGqa.rst
@@ -0,0 +1,2 @@
+Fix the Windows implementation of :func:`os.waitpid` for exit code larger than
+``INT_MAX >> 8``. The exit status is now interpreted as an unsigned number.
diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h
index 9a605e4841a00..a2b4566443b51 100644
--- a/Modules/clinic/posixmodule.c.h
+++ b/Modules/clinic/posixmodule.c.h
@@ -8839,7 +8839,7 @@ PyDoc_STRVAR(os_waitstatus_to_exitcode__doc__,
 {"waitstatus_to_exitcode", (PyCFunction)(void(*)(void))os_waitstatus_to_exitcode, METH_FASTCALL|METH_KEYWORDS, os_waitstatus_to_exitcode__doc__},
 
 static PyObject *
-os_waitstatus_to_exitcode_impl(PyObject *module, int status);
+os_waitstatus_to_exitcode_impl(PyObject *module, PyObject *status_obj);
 
 static PyObject *
 os_waitstatus_to_exitcode(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames)
@@ -8848,22 +8848,14 @@ os_waitstatus_to_exitcode(PyObject *module, PyObject *const *args, Py_ssize_t na
 static const char * const _keywords[] = {"status", NULL};
 static _PyArg_Parser _parser = {NULL, _keywords, "waitstatus_to_exitcode", 0};
 PyObject *argsbuf[1];
- int status;
+ PyObject *status_obj;
 
 args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 1, 1, 0, argsbuf);
 if (!args) {
 goto exit;
 }
- if (PyFloat_Check(args[0])) {
- PyErr_SetString(PyExc_TypeError,
- "integer argument expected, got float" );
- goto exit;
- }
- status = _PyLong_AsInt(args[0]);
- if (status == -1 && PyErr_Occurred()) {
- goto exit;
- }
- return_value = os_waitstatus_to_exitcode_impl(module, status);
+ status_obj = args[0];
+ return_value = os_waitstatus_to_exitcode_impl(module, status_obj);
 
 exit:
 return return_value;
@@ -9426,4 +9418,4 @@ os_waitstatus_to_exitcode(PyObject *module, PyObject *const *args, Py_ssize_t na
 #ifndef OS_WAITSTATUS_TO_EXITCODE_METHODDEF
 #define OS_WAITSTATUS_TO_EXITCODE_METHODDEF
 #endif /* !defined(OS_WAITSTATUS_TO_EXITCODE_METHODDEF) */
-/*[clinic end generated code: output=545c08f76d7a6286 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=ba73b68f1c435ff6 input=a9049054013a1b77]*/
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 2157cbbe5d9b5..3386be0fbc85a 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -7972,8 +7972,10 @@ os_waitpid_impl(PyObject *module, intptr_t pid, int options)
 if (res < 0)
 return (!async_err) ? posix_error() : NULL;
 
+ unsigned long long ustatus = (unsigned int)status;
+
 /* shift the status left a byte so this is more like the POSIX waitpid */
- return Py_BuildValue(_Py_PARSE_INTPTR "i", res, status << 8);
+ return Py_BuildValue(_Py_PARSE_INTPTR "K", res, ustatus << 8);
 }
 #endif
 
@@ -13829,7 +13831,7 @@ os__remove_dll_directory_impl(PyObject *module, PyObject *cookie)
 /*[clinic input]
 os.waitstatus_to_exitcode
 
- status: int
+ status as status_obj: object
 
 Convert a wait status to an exit code.
 
@@ -13847,10 +13849,20 @@ This function must not be called if WIFSTOPPED(status) is true.
 [clinic start generated code]*/
 
 static PyObject *
-os_waitstatus_to_exitcode_impl(PyObject *module, int status)
-/*[clinic end generated code: output=c7c2265731f79b7a input=edfa5ca5006276fb]*/
+os_waitstatus_to_exitcode_impl(PyObject *module, PyObject *status_obj)
+/*[clinic end generated code: output=db50b1b0ba3c7153 input=7fe2d7fdaea3db42]*/
 {
+ if (PyFloat_Check(status_obj)) {
+ PyErr_SetString(PyExc_TypeError,
+ "integer argument expected, got float" );
+ return NULL;
+ }
 #ifndef MS_WINDOWS
+ int status = _PyLong_AsInt(status_obj);
+ if (status == -1 && PyErr_Occurred()) {
+ return NULL;
+ }
+
 WAIT_TYPE wait_status;
 WAIT_STATUS_INT(wait_status) = status;
 int exitcode;
@@ -13889,8 +13901,19 @@ os_waitstatus_to_exitcode_impl(PyObject *module, int status)
 #else
 /* Windows implementation: see os.waitpid() implementation
 which uses _cwait(). */
- int exitcode = (status >> 8);
- return PyLong_FromLong(exitcode);
+ unsigned long long status = PyLong_AsUnsignedLongLong(status_obj);
+ if (status == (unsigned long long)-1 && PyErr_Occurred()) {
+ return NULL;
+ }
+
+ unsigned long long exitcode = (status >> 8);
+ /* ExitProcess() accepts an UINT type:
+ reject exit code which doesn't fit in an UINT */
+ if (exitcode > UINT_MAX) {
+ PyErr_Format(PyExc_ValueError, "invalid exit code: %llu", exitcode);
+ return NULL;
+ }
+ return PyLong_FromUnsignedLong((unsigned long)exitcode);
 #endif
 }
 #endif
