bpo-30697: Fix PyErr_NormalizeException() when no memory (GH-2327)

8 files changed, 203 insertions, 53 deletions

diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst
index 17e4e0a..d3c3c1f 100644
--- a/Doc/whatsnew/3.7.rst
+++ b/Doc/whatsnew/3.7.rst
@@ -440,6 +440,11 @@ Build and C API Changes
   download a copy of 32-bit Python for this purpose.  (Contributed by Zachary
   Ware in :issue:`30450`.)
 
+* The ``PyExc_RecursionErrorInst`` singleton that was part of the public API
+  has been removed as its members being never cleared may cause a segfault
+  during finalization of the interpreter. Contributed by Xavier de Gaye in
+  :issue:`22898` and :issue:`30697`.
+
 * Support for building ``--without-threads`` is removed.
   (Contributed by Antoine Pitrou in :issue:`31370`.).
 
diff --git a/Include/pyerrors.h b/Include/pyerrors.h
index 9418201..f289471 100644
--- a/Include/pyerrors.h
+++ b/Include/pyerrors.h
@@ -220,8 +220,6 @@ PyAPI_DATA(PyObject *) PyExc_IOError;
 PyAPI_DATA(PyObject *) PyExc_WindowsError;
 #endif
 
-PyAPI_DATA(PyObject *) PyExc_RecursionErrorInst;
-
 /* Predefined warning categories */
 PyAPI_DATA(PyObject *) PyExc_Warning;
 PyAPI_DATA(PyObject *) PyExc_UserWarning;
diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py
index cef8d44..8dceb5c 100644
--- a/Lib/test/test_exceptions.py
+++ b/Lib/test/test_exceptions.py
@@ -10,8 +10,8 @@ import errno
 
 from test.support import (TESTFN, captured_stderr, check_impl_detail,
                           check_warnings, cpython_only, gc_collect, run_unittest,
-                          no_tracing, unlink, import_module, script_helper)
-
+                          no_tracing, unlink, import_module, script_helper,
+                          SuppressCrashReport)
 class NaiveException(Exception):
     def __init__(self, x):
         self.x = x
@@ -936,6 +936,105 @@ class ExceptionTests(unittest.TestCase):
         self.assertIsInstance(v, RecursionError, type(v))
         self.assertIn("maximum recursion depth exceeded", str(v))
 
+    @cpython_only
+    def test_recursion_normalizing_exception(self):
+        # Issue #22898.
+        # Test that a RecursionError is raised when tstate->recursion_depth is
+        # equal to recursion_limit in PyErr_NormalizeException() and check
+        # that a ResourceWarning is printed.
+        # Prior to #22898, the recursivity of PyErr_NormalizeException() was
+        # controled by tstate->recursion_depth and a PyExc_RecursionErrorInst
+        # singleton was being used in that case, that held traceback data and
+        # locals indefinitely and would cause a segfault in _PyExc_Fini() upon
+        # finalization of these locals.
+        code = """if 1:
+            import sys
+            from _testcapi import get_recursion_depth
+
+            class MyException(Exception): pass
+
+            def setrecursionlimit(depth):
+                while 1:
+                    try:
+                        sys.setrecursionlimit(depth)
+                        return depth
+                    except RecursionError:
+                        # sys.setrecursionlimit() raises a RecursionError if
+                        # the new recursion limit is too low (issue #25274).
+                        depth += 1
+
+            def recurse(cnt):
+                cnt -= 1
+                if cnt:
+                    recurse(cnt)
+                else:
+                    generator.throw(MyException)
+
+            def gen():
+                f = open(%a, mode='rb', buffering=0)
+                yield
+
+            generator = gen()
+            next(generator)
+            recursionlimit = sys.getrecursionlimit()
+            depth = get_recursion_depth()
+            try:
+                # Upon the last recursive invocation of recurse(),
+                # tstate->recursion_depth is equal to (recursion_limit - 1)
+                # and is equal to recursion_limit when _gen_throw() calls
+                # PyErr_NormalizeException().
+                recurse(setrecursionlimit(depth + 2) - depth - 1)
+            finally:
+                sys.setrecursionlimit(recursionlimit)
+                print('Done.')
+        """ % __file__
+        rc, out, err = script_helper.assert_python_failure("-Wd", "-c", code)
+        # Check that the program does not fail with SIGABRT.
+        self.assertEqual(rc, 1)
+        self.assertIn(b'RecursionError', err)
+        self.assertIn(b'ResourceWarning', err)
+        self.assertIn(b'Done.', out)
+
+    @cpython_only
+    def test_recursion_normalizing_infinite_exception(self):
+        # Issue #30697. Test that a RecursionError is raised when
+        # PyErr_NormalizeException() maximum recursion depth has been
+        # exceeded.
+        code = """if 1:
+            import _testcapi
+            try:
+                raise _testcapi.RecursingInfinitelyError
+            finally:
+                print('Done.')
+        """
+        rc, out, err = script_helper.assert_python_failure("-c", code)
+        self.assertEqual(rc, 1)
+        self.assertIn(b'RecursionError: maximum recursion depth exceeded '
+                      b'while normalizing an exception', err)
+        self.assertIn(b'Done.', out)
+
+    @cpython_only
+    def test_recursion_normalizing_with_no_memory(self):
+        # Issue #30697. Test that in the abort that occurs when there is no
+        # memory left and the size of the Python frames stack is greater than
+        # the size of the list of preallocated MemoryError instances, the
+        # Fatal Python error message mentions MemoryError.
+        code = """if 1:
+            import _testcapi
+            class C(): pass
+            def recurse(cnt):
+                cnt -= 1
+                if cnt:
+                    recurse(cnt)
+                else:
+                    _testcapi.set_nomemory(0)
+                    C()
+            recurse(16)
+        """
+        with SuppressCrashReport():
+            rc, out, err = script_helper.assert_python_failure("-c", code)
+            self.assertIn(b'Fatal Python error: Cannot recover from '
+                          b'MemoryErrors while normalizing exceptions.', err)
 
     @cpython_only
     def test_MemoryError(self):
diff --git a/Misc/NEWS.d/next/C API/2017-06-30-11-58-01.bpo-30697.Q3T_8n.rst b/Misc/NEWS.d/next/C API/2017-06-30-11-58-01.bpo-30697.Q3T_8n.rst
new file mode 100644
index 0000000..ccd9605
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2017-06-30-11-58-01.bpo-30697.Q3T_8n.rst
@@ -0,0 +1,4 @@
+The `PyExc_RecursionErrorInst` singleton is removed and
+`PyErr_NormalizeException()` does not use it anymore. This singleton is
+persistent and its members being never cleared may cause a segfault during
+finalization of the interpreter. See also issue #22898.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 0e1e772..899c3d9 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -4940,6 +4940,61 @@ static PyTypeObject awaitType = {
 };
 
 
+static int recurse_infinitely_error_init(PyObject *, PyObject *, PyObject *);
+
+static PyTypeObject PyRecursingInfinitelyError_Type = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    "RecursingInfinitelyError",   /* tp_name */
+    sizeof(PyBaseExceptionObject), /* tp_basicsize */
+    0,                          /* tp_itemsize */
+    0,                          /* tp_dealloc */
+    0,                          /* tp_print */
+    0,                          /* tp_getattr */
+    0,                          /* tp_setattr */
+    0,                          /* tp_reserved */
+    0,                          /* tp_repr */
+    0,                          /* tp_as_number */
+    0,                          /* tp_as_sequence */
+    0,                          /* tp_as_mapping */
+    0,                          /* tp_hash */
+    0,                          /* tp_call */
+    0,                          /* tp_str */
+    0,                          /* tp_getattro */
+    0,                          /* tp_setattro */
+    0,                          /* tp_as_buffer */
+    Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */
+    "Instantiating this exception starts infinite recursion.", /* tp_doc */
+    0,                          /* tp_traverse */
+    0,                          /* tp_clear */
+    0,                          /* tp_richcompare */
+    0,                          /* tp_weaklistoffset */
+    0,                          /* tp_iter */
+    0,                          /* tp_iternext */
+    0,                          /* tp_methods */
+    0,                          /* tp_members */
+    0,                          /* tp_getset */
+    0,                          /* tp_base */
+    0,                          /* tp_dict */
+    0,                          /* tp_descr_get */
+    0,                          /* tp_descr_set */
+    0,                          /* tp_dictoffset */
+    (initproc)recurse_infinitely_error_init, /* tp_init */
+    0,                          /* tp_alloc */
+    0,                          /* tp_new */
+};
+
+static int
+recurse_infinitely_error_init(PyObject *self, PyObject *args, PyObject *kwds)
+{
+    PyObject *type = (PyObject *)&PyRecursingInfinitelyError_Type;
+
+    /* Instantiating this exception starts infinite recursion. */
+    Py_INCREF(type);
+    PyErr_SetObject(type, NULL);
+    return -1;
+}
+
+
 static struct PyModuleDef _testcapimodule = {
     PyModuleDef_HEAD_INIT,
     "_testcapi",
@@ -4981,6 +5036,14 @@ PyInit__testcapi(void)
     Py_INCREF(&awaitType);
     PyModule_AddObject(m, "awaitType", (PyObject *)&awaitType);
 
+    PyRecursingInfinitelyError_Type.tp_base = (PyTypeObject *)PyExc_Exception;
+    if (PyType_Ready(&PyRecursingInfinitelyError_Type) < 0) {
+        return NULL;
+    }
+    Py_INCREF(&PyRecursingInfinitelyError_Type);
+    PyModule_AddObject(m, "RecursingInfinitelyError",
+                       (PyObject *)&PyRecursingInfinitelyError_Type);
+
     PyModule_AddObject(m, "CHAR_MAX", PyLong_FromLong(CHAR_MAX));
     PyModule_AddObject(m, "CHAR_MIN", PyLong_FromLong(CHAR_MIN));
     PyModule_AddObject(m, "UCHAR_MAX", PyLong_FromLong(UCHAR_MAX));
diff --git a/Objects/exceptions.c b/Objects/exceptions.c
index 42b3fc7..637d766 100644
--- a/Objects/exceptions.c
+++ b/Objects/exceptions.c
@@ -2409,12 +2409,6 @@ SimpleExtendsException(PyExc_Warning, ResourceWarning,
 
 
 
-/* Pre-computed RecursionError instance for when recursion depth is reached.
-   Meant to be used when normalizing the exception for exceeding the recursion
-   depth will cause its own infinite recursion.
-*/
-PyObject *PyExc_RecursionErrorInst = NULL;
-
 #define PRE_INIT(TYPE) \
     if (!(_PyExc_ ## TYPE.tp_flags & Py_TPFLAGS_READY)) { \
         if (PyType_Ready(&_PyExc_ ## TYPE) < 0) \
@@ -2674,37 +2668,11 @@ _PyExc_Init(PyObject *bltinmod)
     ADD_ERRNO(TimeoutError, ETIMEDOUT)
 
     preallocate_memerrors();
-
-    if (!PyExc_RecursionErrorInst) {
-        PyExc_RecursionErrorInst = BaseException_new(&_PyExc_RecursionError, NULL, NULL);
-        if (!PyExc_RecursionErrorInst)
-            Py_FatalError("Cannot pre-allocate RecursionError instance for "
-                            "recursion errors");
-        else {
-            PyBaseExceptionObject *err_inst =
-                (PyBaseExceptionObject *)PyExc_RecursionErrorInst;
-            PyObject *args_tuple;
-            PyObject *exc_message;
-            exc_message = PyUnicode_FromString("maximum recursion depth exceeded");
-            if (!exc_message)
-                Py_FatalError("cannot allocate argument for RecursionError "
-                                "pre-allocation");
-            args_tuple = PyTuple_Pack(1, exc_message);
-            if (!args_tuple)
-                Py_FatalError("cannot allocate tuple for RecursionError "
-                                "pre-allocation");
-            Py_DECREF(exc_message);
-            if (BaseException_init(err_inst, args_tuple, NULL))
-                Py_FatalError("init of pre-allocated RecursionError failed");
-            Py_DECREF(args_tuple);
-        }
-    }
 }
 
 void
 _PyExc_Fini(void)
 {
-    Py_CLEAR(PyExc_RecursionErrorInst);
     free_preallocated_memerrors();
     Py_CLEAR(errnomap);
 }
diff --git a/PC/python3.def b/PC/python3.def
index 1d089ec..a4c0415 100644
--- a/PC/python3.def
+++ b/PC/python3.def
@@ -224,7 +224,6 @@ EXPORTS
   PyExc_PermissionError=python37.PyExc_PermissionError DATA
   PyExc_ProcessLookupError=python37.PyExc_ProcessLookupError DATA
   PyExc_RecursionError=python37.PyExc_RecursionError DATA
-  PyExc_RecursionErrorInst=python37.PyExc_RecursionErrorInst DATA
   PyExc_ReferenceError=python37.PyExc_ReferenceError DATA
   PyExc_ResourceWarning=python37.PyExc_ResourceWarning DATA
   PyExc_RuntimeError=python37.PyExc_RuntimeError DATA
diff --git a/Python/errors.c b/Python/errors.c
index 51fc791..13ebb19 100644
--- a/Python/errors.c
+++ b/Python/errors.c
@@ -218,20 +218,24 @@ PyErr_ExceptionMatches(PyObject *exc)
 }
 
 
+#ifndef Py_NORMALIZE_RECURSION_LIMIT
+#define Py_NORMALIZE_RECURSION_LIMIT 32
+#endif
+
 /* Used in many places to normalize a raised exception, including in
    eval_code2(), do_raise(), and PyErr_Print()
 
    XXX: should PyErr_NormalizeException() also call
             PyException_SetTraceback() with the resulting value and tb?
 */
-void
-PyErr_NormalizeException(PyObject **exc, PyObject **val, PyObject **tb)
+static void
+PyErr_NormalizeExceptionEx(PyObject **exc, PyObject **val,
+                           PyObject **tb, int recursion_depth)
 {
     PyObject *type = *exc;
     PyObject *value = *val;
     PyObject *inclass = NULL;
     PyObject *initial_tb = NULL;
-    PyThreadState *tstate = NULL;
 
     if (type == NULL) {
         /* There was no exception, so nothing to do. */
@@ -293,6 +297,10 @@ PyErr_NormalizeException(PyObject **exc, PyObject **val, PyObject **tb)
 finally:
     Py_DECREF(type);
     Py_DECREF(value);
+    if (recursion_depth + 1 == Py_NORMALIZE_RECURSION_LIMIT) {
+        PyErr_SetString(PyExc_RecursionError, "maximum recursion depth "
+                        "exceeded while normalizing an exception");
+    }
     /* If the new exception doesn't set a traceback and the old
        exception had a traceback, use the old traceback for the
        new exception.  It's better than nothing.
@@ -305,20 +313,26 @@ finally:
         else
             Py_DECREF(initial_tb);
     }
-    /* normalize recursively */
-    tstate = PyThreadState_GET();
-    if (++tstate->recursion_depth > Py_GetRecursionLimit()) {
-        --tstate->recursion_depth;
-        /* throw away the old exception and use the recursion error instead */
-        Py_INCREF(PyExc_RecursionError);
-        Py_SETREF(*exc, PyExc_RecursionError);
-        Py_INCREF(PyExc_RecursionErrorInst);
-        Py_SETREF(*val, PyExc_RecursionErrorInst);
-        /* just keeping the old traceback */
-        return;
+    /* Normalize recursively.
+     * Abort when Py_NORMALIZE_RECURSION_LIMIT has been exceeded and the
+     * corresponding RecursionError could not be normalized.*/
+    if (++recursion_depth > Py_NORMALIZE_RECURSION_LIMIT) {
+        if (PyErr_GivenExceptionMatches(*exc, PyExc_MemoryError)) {
+            Py_FatalError("Cannot recover from MemoryErrors "
+                          "while normalizing exceptions.");
+        }
+        else {
+            Py_FatalError("Cannot recover from the recursive normalization "
+                          "of an exception.");
+        }
     }
-    PyErr_NormalizeException(exc, val, tb);
-    --tstate->recursion_depth;
+    PyErr_NormalizeExceptionEx(exc, val, tb, recursion_depth);
+}
+
+void
+PyErr_NormalizeException(PyObject **exc, PyObject **val, PyObject **tb)
+{
+    PyErr_NormalizeExceptionEx(exc, val, tb, 0);
 }