From 4bf4356a06a359fb07c259bdaac927c5874779c1 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 2 Dec 2020 13:30:55 +0000 Subject: [PATCH] bpo-42500: Fix recursion in or after except (GH-23568) * Use counter, rather boolean state when handling soft overflows. (cherry picked from commit 4e7a69bdb63a104587759d7784124492dcdd496e) --- Include/cpython/pystate.h | 3 +- Include/internal/pycore_ceval.h | 16 ------ Lib/test/test_exceptions.py | 52 ++++++++++++++++++- Lib/test/test_sys.py | 46 +++------------- .../2020-11-30-14-27-29.bpo-42500.excVKU.rst | 2 + Python/ceval.c | 23 ++++---- Python/errors.c | 3 ++ Python/pystate.c | 2 +- Python/sysmodule.c | 4 +- 9 files changed, 76 insertions(+), 75 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-11-30-14-27-29.bpo-42500.excVKU.rst diff --git a/Include/cpython/pystate.h b/Include/cpython/pystate.h index f292da1d3c6c5e..29f6bf5a662dcd 100644 --- a/Include/cpython/pystate.h +++ b/Include/cpython/pystate.h @@ -58,8 +58,7 @@ struct _ts { /* Borrowed reference to the current frame (it can be NULL) */ PyFrameObject *frame; int recursion_depth; - char overflowed; /* The stack has overflowed. Allow 50 more calls - to handle the runtime error. */ + int recursion_headroom; /* Allow 50 more calls to handle any errors. */ char recursion_critical; /* The current calls must not cause a stack overflow. */ int stackcheck_counter; diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index 18c8f027af16e6..e7ace9bd01c2a9 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -90,24 +90,8 @@ static inline int _Py_EnterRecursiveCall_inline(const char *where) { #define Py_EnterRecursiveCall(where) _Py_EnterRecursiveCall_inline(where) -/* Compute the "lower-water mark" for a recursion limit. When - * Py_LeaveRecursiveCall() is called with a recursion depth below this mark, - * the overflowed flag is reset to 0. */ -static inline int _Py_RecursionLimitLowerWaterMark(int limit) { - if (limit > 200) { - return (limit - 50); - } - else { - return (3 * (limit >> 2)); - } -} - static inline void _Py_LeaveRecursiveCall(PyThreadState *tstate) { tstate->recursion_depth--; - int limit = tstate->interp->ceval.recursion_limit; - if (tstate->recursion_depth < _Py_RecursionLimitLowerWaterMark(limit)) { - tstate->overflowed = 0; - } } static inline void _Py_LeaveRecursiveCall_inline(void) { diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py index 8d125b57ad6d5a..9c09bdc317dacf 100644 --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -1043,7 +1043,7 @@ def gen(): # 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) + recurse(setrecursionlimit(depth + 2) - depth) finally: sys.setrecursionlimit(recursionlimit) print('Done.') @@ -1073,6 +1073,54 @@ def test_recursion_normalizing_infinite_exception(self): b'while normalizing an exception', err) self.assertIn(b'Done.', out) + + def test_recursion_in_except_handler(self): + + def set_relative_recursion_limit(n): + depth = 1 + while True: + try: + sys.setrecursionlimit(depth) + except RecursionError: + depth += 1 + else: + break + sys.setrecursionlimit(depth+n) + + def recurse_in_except(): + try: + 1/0 + except: + recurse_in_except() + + def recurse_after_except(): + try: + 1/0 + except: + pass + recurse_after_except() + + def recurse_in_body_and_except(): + try: + recurse_in_body_and_except() + except: + recurse_in_body_and_except() + + recursionlimit = sys.getrecursionlimit() + try: + set_relative_recursion_limit(10) + for func in (recurse_in_except, recurse_after_except, recurse_in_body_and_except): + with self.subTest(func=func): + try: + func() + except RecursionError: + pass + else: + self.fail("Should have raised a RecursionError") + finally: + sys.setrecursionlimit(recursionlimit) + + @cpython_only def test_recursion_normalizing_with_no_memory(self): # Issue #30697. Test that in the abort that occurs when there is no @@ -1109,7 +1157,7 @@ def raiseMemError(): except MemoryError as e: tb = e.__traceback__ else: - self.fail("Should have raises a MemoryError") + self.fail("Should have raised a MemoryError") return traceback.format_tb(tb) tb1 = raiseMemError() diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index d79151355eab1d..fc6c1ef1acd7cd 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -219,7 +219,7 @@ def test_recursionlimit_recovery(self): def f(): f() try: - for depth in (10, 25, 50, 75, 100, 250, 1000): + for depth in (50, 75, 100, 250, 1000): try: sys.setrecursionlimit(depth) except RecursionError: @@ -229,17 +229,17 @@ def f(): # Issue #5392: test stack overflow after hitting recursion # limit twice - self.assertRaises(RecursionError, f) - self.assertRaises(RecursionError, f) + with self.assertRaises(RecursionError): + f() + with self.assertRaises(RecursionError): + f() finally: sys.setrecursionlimit(oldlimit) @test.support.cpython_only def test_setrecursionlimit_recursion_depth(self): # Issue #25274: Setting a low recursion limit must be blocked if the - # current recursion depth is already higher than the "lower-water - # mark". Otherwise, it may not be possible anymore to - # reset the overflowed flag to 0. + # current recursion depth is already higher than limit. from _testinternalcapi import get_recursion_depth @@ -260,42 +260,10 @@ def set_recursion_limit_at_depth(depth, limit): sys.setrecursionlimit(1000) for limit in (10, 25, 50, 75, 100, 150, 200): - # formula extracted from _Py_RecursionLimitLowerWaterMark() - if limit > 200: - depth = limit - 50 - else: - depth = limit * 3 // 4 - set_recursion_limit_at_depth(depth, limit) + set_recursion_limit_at_depth(limit, limit) finally: sys.setrecursionlimit(oldlimit) - # The error message is specific to CPython - @test.support.cpython_only - def test_recursionlimit_fatalerror(self): - # A fatal error occurs if a second recursion limit is hit when recovering - # from a first one. - code = textwrap.dedent(""" - import sys - - def f(): - try: - f() - except RecursionError: - f() - - sys.setrecursionlimit(%d) - f()""") - with test.support.SuppressCrashReport(): - for i in (50, 1000): - sub = subprocess.Popen([sys.executable, '-c', code % i], - stderr=subprocess.PIPE) - err = sub.communicate()[1] - self.assertTrue(sub.returncode, sub.returncode) - self.assertIn( - b"Fatal Python error: _Py_CheckRecursiveCall: " - b"Cannot recover from stack overflow", - err) - def test_getwindowsversion(self): # Raise SkipTest if sys doesn't have getwindowsversion attribute test.support.get_attribute(sys, "getwindowsversion") diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-11-30-14-27-29.bpo-42500.excVKU.rst b/Misc/NEWS.d/next/Core and Builtins/2020-11-30-14-27-29.bpo-42500.excVKU.rst new file mode 100644 index 00000000000000..2462a8e1fabefc --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-11-30-14-27-29.bpo-42500.excVKU.rst @@ -0,0 +1,2 @@ +Improve handling of exceptions near recursion limit. Converts a number of +Fatal Errors in RecursionErrors. diff --git a/Python/ceval.c b/Python/ceval.c index 91e879e8042044..b7176dc0f6b8a2 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -793,23 +793,22 @@ _Py_CheckRecursiveCall(PyThreadState *tstate, const char *where) _Py_CheckRecursionLimit = recursion_limit; } #endif - if (tstate->recursion_critical) - /* Somebody asked that we don't check for recursion. */ - return 0; - if (tstate->overflowed) { + if (tstate->recursion_headroom) { if (tstate->recursion_depth > recursion_limit + 50) { /* Overflowing while handling an overflow. Give up. */ Py_FatalError("Cannot recover from stack overflow."); } - return 0; } - if (tstate->recursion_depth > recursion_limit) { - --tstate->recursion_depth; - tstate->overflowed = 1; - _PyErr_Format(tstate, PyExc_RecursionError, - "maximum recursion depth exceeded%s", - where); - return -1; + else { + if (tstate->recursion_depth > recursion_limit) { + tstate->recursion_headroom++; + _PyErr_Format(tstate, PyExc_RecursionError, + "maximum recursion depth exceeded%s", + where); + tstate->recursion_headroom--; + --tstate->recursion_depth; + return -1; + } } return 0; } diff --git a/Python/errors.c b/Python/errors.c index 87af39d527a512..d8c2d8b9236ed6 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -290,12 +290,14 @@ _PyErr_NormalizeException(PyThreadState *tstate, PyObject **exc, PyObject **val, PyObject **tb) { int recursion_depth = 0; + tstate->recursion_headroom++; PyObject *type, *value, *initial_tb; restart: type = *exc; if (type == NULL) { /* There was no exception, so nothing to do. */ + tstate->recursion_headroom--; return; } @@ -347,6 +349,7 @@ _PyErr_NormalizeException(PyThreadState *tstate, PyObject **exc, } *exc = type; *val = value; + tstate->recursion_headroom--; return; error: diff --git a/Python/pystate.c b/Python/pystate.c index 9beefa8e20c444..71aac6f695d6e8 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -576,7 +576,7 @@ new_threadstate(PyInterpreterState *interp, int init) tstate->frame = NULL; tstate->recursion_depth = 0; - tstate->overflowed = 0; + tstate->recursion_headroom = 0; tstate->recursion_critical = 0; tstate->stackcheck_counter = 0; tstate->tracing = 0; diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 3e4115fe8e1f93..4c7f7b67fa837b 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -1160,7 +1160,6 @@ static PyObject * sys_setrecursionlimit_impl(PyObject *module, int new_limit) /*[clinic end generated code: output=35e1c64754800ace input=b0f7a23393924af3]*/ { - int mark; PyThreadState *tstate = _PyThreadState_GET(); if (new_limit < 1) { @@ -1178,8 +1177,7 @@ sys_setrecursionlimit_impl(PyObject *module, int new_limit) Reject too low new limit if the current recursion depth is higher than the new low-water mark. Otherwise it may not be possible anymore to reset the overflowed flag to 0. */ - mark = _Py_RecursionLimitLowerWaterMark(new_limit); - if (tstate->recursion_depth >= mark) { + if (tstate->recursion_depth >= new_limit) { _PyErr_Format(tstate, PyExc_RecursionError, "cannot set the recursion limit to %i at " "the recursion depth %i: the limit is too low",