From 56d1f5ca32892c7643eb8cee49c40c1644f1abfe Mon Sep 17 00:00:00 2001 From: xdegaye Date: Thu, 26 Oct 2017 15:09:06 +0200 Subject: [PATCH] bpo-30697: Fix PyErr_NormalizeException() when no memory (GH-2327) --- Doc/whatsnew/3.7.rst | 5 + Include/pyerrors.h | 2 - Lib/test/test_exceptions.py | 103 +++++++++++++++++- .../2017-06-30-11-58-01.bpo-30697.Q3T_8n.rst | 4 + Modules/_testcapimodule.c | 63 +++++++++++ Objects/exceptions.c | 32 ------ PC/python3.def | 1 - Python/errors.c | 46 +++++--- 8 files changed, 203 insertions(+), 53 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2017-06-30-11-58-01.bpo-30697.Q3T_8n.rst diff --git a/Doc/whatsnew/3.7.rst b/Doc/whatsnew/3.7.rst index 17e4e0a8813..d3c3c1f7537 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 94182012246..f289471be20 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 cef8d44ffaa..8dceb5c8e79 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 00000000000..ccd9605d247 --- /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 0e1e772b5c1..899c3d93701 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 42b3fc7bb1b..637d7660d35 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 1d089ec28b1..a4c0415242b 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 51fc791f45b..13ebb192c54 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); }