Skip to content

Commit b6be6c5

Browse files
committed
gh-156204: address review feedback
1 parent d0d8595 commit b6be6c5

5 files changed

Lines changed: 53 additions & 24 deletions

File tree

Doc/c-api/exceptions.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,13 @@ Querying the error indicator
486486
of a subclass. If *exc* is a tuple, all exception types in the tuple (and
487487
recursively in subtuples) are searched for a match.
488488
489+
If the subtuples are nested deeply enough to risk exhausting the C stack,
490+
the search is abandoned, a :exc:`RecursionError` is set, and the function
491+
returns false.
492+
493+
.. versionchanged:: next
494+
Deeply nested subtuples previously crashed the interpreter.
495+
489496
490497
.. c:function:: PyObject *PyErr_GetRaisedException(void)
491498

Lib/test/test_exceptions.py

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2779,23 +2779,28 @@ def test_except_star_invalid_exception_type(self):
27792779
pass
27802780

27812781
@cpython_only
2782+
@unittest.skipIf(_testcapi is None, "requires _testcapi")
2783+
def test_given_exception_matches_nested_tuple(self):
2784+
# Nested tuples are searched recursively.
2785+
self.assertTrue(
2786+
_testcapi.err_givenexceptionmatches(ValueError(), ((ValueError,),)))
2787+
self.assertFalse(
2788+
_testcapi.err_givenexceptionmatches(TypeError(), ((ValueError,),)))
2789+
2790+
@cpython_only
2791+
@unittest.skipIf(_testcapi is None, "requires _testcapi")
27822792
@support.skip_emscripten_stack_overflow()
27832793
@support.skip_wasi_stack_overflow()
2794+
@support.run_with_limited_c_stack(depth=500_000)
27842795
def test_given_exception_matches_deeply_nested_tuple(self):
2785-
ctypes = import_module('ctypes')
2786-
lib = ctypes.pythonapi
2787-
lib.PyErr_GivenExceptionMatches.argtypes = [ctypes.py_object, ctypes.py_object]
2788-
lib.PyErr_GivenExceptionMatches.restype = ctypes.c_int
2789-
2790-
tup = (1, ValueError)
2791-
for _ in range(50_000):
2792-
tup = (1, tup)
2793-
2794-
# PyErr_GivenExceptionMatches should handle deep recursion safely without SIGSEGV
2795-
res = lib.PyErr_GivenExceptionMatches(TypeError(), tup)
2796-
self.assertEqual(res, 0)
2797-
if lib.PyErr_Occurred():
2798-
lib.PyErr_Clear()
2796+
# gh-156204: PyErr_GivenExceptionMatches() used to exhaust the C stack
2797+
# and crash the interpreter on deeply nested tuples of exception types.
2798+
tup = (ValueError,)
2799+
for _ in range(500_000):
2800+
tup = (tup,)
2801+
2802+
with self.assertRaises(RecursionError):
2803+
_testcapi.err_givenexceptionmatches(TypeError(), tup)
27992804

28002805

28012806
class PEP626Tests(unittest.TestCase):
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
Fix unhandled recursion in :c:func:`PyErr_GivenExceptionMatches` when
1+
Fix unhandled recursion error in :c:func:`PyErr_GivenExceptionMatches` when
22
evaluating deeply nested exception tuples, preventing crashes caused by
33
stack exhaustion.

Modules/_testcapi/exceptions.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,24 @@ err_restore(PyObject *self, PyObject *args) {
5454
return NULL;
5555
}
5656

57+
static PyObject *
58+
err_givenexceptionmatches(PyObject *Py_UNUSED(module), PyObject *args)
59+
{
60+
PyObject *err, *exc;
61+
if (!PyArg_ParseTuple(args, "OO", &err, &exc)) {
62+
return NULL;
63+
}
64+
assert(!PyErr_Occurred());
65+
int res = PyErr_GivenExceptionMatches(err, exc);
66+
/* PyErr_GivenExceptionMatches() has no failure return value, but it can
67+
* set RecursionError on a deeply nested tuple; report that to the caller.
68+
*/
69+
if (res == 0 && PyErr_Occurred()) {
70+
return NULL;
71+
}
72+
return PyBool_FromLong(res);
73+
}
74+
5775
/*[clinic input]
5876
_testcapi.exception_print
5977
exception as exc: object
@@ -544,6 +562,7 @@ static PyTypeObject PyRecursingInfinitelyError_Type = {
544562

545563
static PyMethodDef test_methods[] = {
546564
{"err_restore", err_restore, METH_VARARGS},
565+
{"err_givenexceptionmatches", err_givenexceptionmatches, METH_VARARGS},
547566
{"err_writeunraisable", err_writeunraisable, METH_VARARGS},
548567
{"err_formatunraisable", err_formatunraisable, METH_VARARGS},
549568
_TESTCAPI_ERR_SET_RAISED_METHODDEF

Python/errors.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -338,19 +338,17 @@ PyErr_GivenExceptionMatches(PyObject *err, PyObject *exc)
338338
if (Py_EnterRecursiveCall(" in PyErr_GivenExceptionMatches")) {
339339
return 0;
340340
}
341-
Py_ssize_t i, n;
342-
n = PyTuple_GET_SIZE(exc);
343-
for (i = 0; i < n; i++) {
341+
int res = 0;
342+
Py_ssize_t n = PyTuple_GET_SIZE(exc);
343+
for (Py_ssize_t i = 0; i < n; i++) {
344344
/* Test recursively */
345-
if (PyErr_GivenExceptionMatches(
346-
err, PyTuple_GET_ITEM(exc, i)))
347-
{
348-
Py_LeaveRecursiveCall();
349-
return 1;
345+
if (PyErr_GivenExceptionMatches(err, PyTuple_GET_ITEM(exc, i))) {
346+
res = 1;
347+
break;
350348
}
351349
}
352350
Py_LeaveRecursiveCall();
353-
return 0;
351+
return res;
354352
}
355353
/* err might be an instance, so check its class. */
356354
if (PyExceptionInstance_Check(err))

0 commit comments

Comments
 (0)