[3.13] gh-125842: Fix sys.exit(0xffff_ffff) on Windows (GH-125896) (GH-125925)

miss-islington · colesbury · web-flow · commit 52b57eee64c1 · 2024-10-24T16:27:30.000Z
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. (cherry picked from commit ad6110a) Co-authored-by: Sam Gross <colesbury@gmail.com>
diff --git 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
@@ -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
@@ -563,6 +563,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)
 {
@@ -579,50 +603,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;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+Fix a :exc:`SystemError` when :func:`sys.exit` is called with ``0xffffffff``
	`2`	`+on Windows.`