From ad6110a93ffa82cae71af6c78692de065d3871b5 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 24 Oct 2024 12:03:50 -0400 Subject: [PATCH] gh-125842: Fix `sys.exit(0xffff_ffff)` on Windows (#125896) On Windows, `long` is a signed 32-bit integer so it can't represent `0xffff_ffff` without overflow. Windows exit codes are unsigned 32-bit integers, so if a child process exits with `-1`, it will be represented as `0xffff_ffff`. Also fix a number of other possible cases where `_Py_HandleSystemExit` could return with an exception set, leading to a `SystemError` (or fatal error in debug builds) later on during shutdown. --- Lib/test/test_sys.py | 14 ++++ ...-10-23-17-24-23.gh-issue-125842.m3EF9E.rst | 2 + Python/pythonrun.c | 80 +++++++++++-------- 3 files changed, 63 insertions(+), 33 deletions(-) create mode 100644 Misc/NEWS.d/next/Windows/2024-10-23-17-24-23.gh-issue-125842.m3EF9E.rst diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 9689ef8e96e072..c0862d7d15f39e 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -206,6 +206,20 @@ def test_exit(self): self.assertEqual(out, b'') self.assertEqual(err, b'') + # gh-125842: Windows uses 32-bit unsigned integers for exit codes + # so a -1 exit code is sometimes interpreted as 0xffff_ffff. + rc, out, err = assert_python_failure('-c', 'import sys; sys.exit(0xffff_ffff)') + self.assertIn(rc, (-1, 0xff, 0xffff_ffff)) + self.assertEqual(out, b'') + self.assertEqual(err, b'') + + # Overflow results in a -1 exit code, which may be converted to 0xff + # or 0xffff_ffff. + rc, out, err = assert_python_failure('-c', 'import sys; sys.exit(2**128)') + self.assertIn(rc, (-1, 0xff, 0xffff_ffff)) + self.assertEqual(out, b'') + self.assertEqual(err, b'') + # call with integer argument with self.assertRaises(SystemExit) as cm: sys.exit(42) diff --git a/Misc/NEWS.d/next/Windows/2024-10-23-17-24-23.gh-issue-125842.m3EF9E.rst b/Misc/NEWS.d/next/Windows/2024-10-23-17-24-23.gh-issue-125842.m3EF9E.rst new file mode 100644 index 00000000000000..63644721d57f5b --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2024-10-23-17-24-23.gh-issue-125842.m3EF9E.rst @@ -0,0 +1,2 @@ +Fix a :exc:`SystemError` when :func:`sys.exit` is called with ``0xffffffff`` +on Windows. diff --git a/Python/pythonrun.c b/Python/pythonrun.c index fc0f11bc4e8af4..8b57018321c070 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -564,6 +564,30 @@ PyRun_SimpleStringFlags(const char *command, PyCompilerFlags *flags) return _PyRun_SimpleStringFlagsWithName(command, NULL, flags); } +static int +parse_exit_code(PyObject *code, int *exitcode_p) +{ + if (PyLong_Check(code)) { + // gh-125842: Use a long long to avoid an overflow error when `long` + // is 32-bit. We still truncate the result to an int. + int exitcode = (int)PyLong_AsLongLong(code); + if (exitcode == -1 && PyErr_Occurred()) { + // On overflow or other error, clear the exception and use -1 + // as the exit code to match historical Python behavior. + PyErr_Clear(); + *exitcode_p = -1; + return 1; + } + *exitcode_p = exitcode; + return 1; + } + else if (code == Py_None) { + *exitcode_p = 0; + return 1; + } + return 0; +} + int _Py_HandleSystemExit(int *exitcode_p) { @@ -580,50 +604,40 @@ _Py_HandleSystemExit(int *exitcode_p) fflush(stdout); - int exitcode = 0; - PyObject *exc = PyErr_GetRaisedException(); - if (exc == NULL) { - goto done; - } - assert(PyExceptionInstance_Check(exc)); + assert(exc != NULL && PyExceptionInstance_Check(exc)); - /* The error code should be in the `code' attribute. */ PyObject *code = PyObject_GetAttr(exc, &_Py_ID(code)); - if (code) { + if (code == NULL) { + // If the exception has no 'code' attribute, print the exception below + PyErr_Clear(); + } + else if (parse_exit_code(code, exitcode_p)) { + Py_DECREF(code); + Py_CLEAR(exc); + return 1; + } + else { + // If code is not an int or None, print it below Py_SETREF(exc, code); - if (exc == Py_None) { - goto done; - } } - /* If we failed to dig out the 'code' attribute, - * just let the else clause below print the error. - */ - if (PyLong_Check(exc)) { - exitcode = (int)PyLong_AsLong(exc); + PyThreadState *tstate = _PyThreadState_GET(); + PyObject *sys_stderr = _PySys_GetAttr(tstate, &_Py_ID(stderr)); + if (sys_stderr != NULL && sys_stderr != Py_None) { + if (PyFile_WriteObject(exc, sys_stderr, Py_PRINT_RAW) < 0) { + PyErr_Clear(); + } } else { - PyThreadState *tstate = _PyThreadState_GET(); - PyObject *sys_stderr = _PySys_GetAttr(tstate, &_Py_ID(stderr)); - /* We clear the exception here to avoid triggering the assertion - * in PyObject_Str that ensures it won't silently lose exception - * details. - */ - PyErr_Clear(); - if (sys_stderr != NULL && sys_stderr != Py_None) { - PyFile_WriteObject(exc, sys_stderr, Py_PRINT_RAW); - } else { - PyObject_Print(exc, stderr, Py_PRINT_RAW); - fflush(stderr); + if (PyObject_Print(exc, stderr, Py_PRINT_RAW) < 0) { + PyErr_Clear(); } - PySys_WriteStderr("\n"); - exitcode = 1; + fflush(stderr); } - -done: + PySys_WriteStderr("\n"); Py_CLEAR(exc); - *exitcode_p = exitcode; + *exitcode_p = 1; return 1; }