From 968d0ba1c5d9442de8f6ee14933eb8c12a856196 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 7 Jul 2022 17:00:31 +0200 Subject: [PATCH 1/2] gh-94215: Fix reference count issue in exception_unwind Zero-cost exception handling did not take ``frame_setlineno()`` into account. It can pop off stacks. This can lead to a crash. Exception unwinding now re-calculates the stack pointer. Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> --- Lib/test/test_pdb.py | 2 +- .../2022-07-07-17-07-00.gh-issue-94215.cXltGH.rst | 4 ++++ Python/ceval.c | 5 +++++ 3 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-07-07-17-07-00.gh-issue-94215.cXltGH.rst diff --git a/Lib/test/test_pdb.py b/Lib/test/test_pdb.py index 968a32fce47b69..1a494be8503f24 100644 --- a/Lib/test/test_pdb.py +++ b/Lib/test/test_pdb.py @@ -2085,7 +2085,7 @@ def test_issue42383(self): expected = '(Pdb) The correct file was executed' self.assertEqual(stdout.split('\n')[6].rstrip('\r'), expected) - @unittest.skip("test crashes, see gh-94215") + # @unittest.skip("test crashes, see gh-94215") def test_gh_94215_crash(self): script = """\ def func(): diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-07-07-17-07-00.gh-issue-94215.cXltGH.rst b/Misc/NEWS.d/next/Core and Builtins/2022-07-07-17-07-00.gh-issue-94215.cXltGH.rst new file mode 100644 index 00000000000000..f181304b1a94f8 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-07-07-17-07-00.gh-issue-94215.cXltGH.rst @@ -0,0 +1,4 @@ +Fix a reference counting bug in exception unwinding code for zero-cost +exception handling. The code did not take into account that +``frame_setlineno()`` can pop off stacks, too. +Patch by Irit Katriel and Christian Heimes. diff --git a/Python/ceval.c b/Python/ceval.c index cdb14d84c0670d..083f8818077df5 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5795,6 +5795,11 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int /* Pop remaining stack entries. */ PyObject **stackbase = _PyFrame_Stackbase(frame); + if (frame->stacktop != -1) { + // frame_setlineno() may have popped off additional stacks in + // frame_stack_pop(). Re-calculate stack pointer. + stack_pointer = _PyFrame_GetStackPointer(frame); + } while (stack_pointer > stackbase) { PyObject *o = POP(); Py_XDECREF(o); From cee2887a1d3c1e9e44580c0b0793d3432e20d0b7 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Thu, 7 Jul 2022 22:31:02 +0200 Subject: [PATCH 2/2] Let's try Brandt's patch --- Python/ceval.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 083f8818077df5..e23bfda7254b10 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5683,6 +5683,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int err = maybe_call_line_trace(tstate->c_tracefunc, tstate->c_traceobj, tstate, frame, instr_prev); + stack_pointer = _PyFrame_GetStackPointer(frame); + frame->stacktop = -1; if (err) { /* trace function raised an exception */ next_instr++; @@ -5690,9 +5692,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int } /* Reload possibly changed frame fields */ next_instr = frame->prev_instr; - - stack_pointer = _PyFrame_GetStackPointer(frame); - frame->stacktop = -1; } } } @@ -5795,11 +5794,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int /* Pop remaining stack entries. */ PyObject **stackbase = _PyFrame_Stackbase(frame); - if (frame->stacktop != -1) { - // frame_setlineno() may have popped off additional stacks in - // frame_stack_pop(). Re-calculate stack pointer. - stack_pointer = _PyFrame_GetStackPointer(frame); - } while (stack_pointer > stackbase) { PyObject *o = POP(); Py_XDECREF(o);