diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 4e2051f23f9b25..f12b225ebfccf2 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -166,6 +166,21 @@ _PyFrame_IsIncomplete(_PyInterpreterFrame *frame) frame->prev_instr < _PyCode_CODE(frame->f_code) + frame->f_code->_co_firsttraceable; } +static inline _PyInterpreterFrame * +_PyFrame_GetFirstComplete(_PyInterpreterFrame *frame) +{ + while (frame && _PyFrame_IsIncomplete(frame)) { + frame = frame->previous; + } + return frame; +} + +static inline _PyInterpreterFrame * +_PyThreadState_GetFrame(PyThreadState *tstate) +{ + return _PyFrame_GetFirstComplete(tstate->cframe->current_frame); +} + /* For use by _PyFrame_GetFrameObject Do not call directly. */ PyFrameObject * diff --git a/Lib/test/test_frame.py b/Lib/test/test_frame.py index 40c734b6e33abe..6bb0144e9b1ed7 100644 --- a/Lib/test/test_frame.py +++ b/Lib/test/test_frame.py @@ -1,4 +1,5 @@ import gc +import operator import re import sys import textwrap @@ -372,6 +373,26 @@ def run(self): ) sneaky_frame_object = sneaky_frame_object.f_back + def test_entry_frames_are_invisible_during_teardown(self): + class C: + """A weakref'able class.""" + + def f(): + """Try to find globals and locals as this frame is being cleared.""" + ref = C() + # Ignore the fact that exec(C()) is a nonsense callback. We're only + # using exec here because it tries to access the current frame's + # globals and locals. If it's trying to get those from a shim frame, + # we'll crash before raising: + return weakref.ref(ref, exec) + + with support.catch_unraisable_exception() as catcher: + # Call from C, so there is a shim frame directly above f: + weak = operator.call(f) # BOOM! + # Cool, we didn't crash. Check that the callback actually happened: + self.assertIs(catcher.unraisable.exc_type, TypeError) + self.assertIsNone(weak()) + @unittest.skipIf(_testcapi is None, 'need _testcapi') class TestCAPI(unittest.TestCase): def getframe(self): diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-12-29-04-39-38.gh-issue-100126.pfFJd-.rst b/Misc/NEWS.d/next/Core and Builtins/2022-12-29-04-39-38.gh-issue-100126.pfFJd-.rst new file mode 100644 index 00000000000000..0ec14253ceff8f --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-12-29-04-39-38.gh-issue-100126.pfFJd-.rst @@ -0,0 +1,3 @@ +Fix an issue where "incomplete" frames could be briefly visible to C code +while other frames are being torn down, possibly resulting in corruption or +hard crashes of the interpreter while running finalizers. diff --git a/Modules/_tracemalloc.c b/Modules/_tracemalloc.c index ac16626f2101ba..9826ad2935beaa 100644 --- a/Modules/_tracemalloc.c +++ b/Modules/_tracemalloc.c @@ -347,14 +347,8 @@ traceback_get_frames(traceback_t *traceback) return; } - _PyInterpreterFrame *pyframe = tstate->cframe->current_frame; - for (;;) { - while (pyframe && _PyFrame_IsIncomplete(pyframe)) { - pyframe = pyframe->previous; - } - if (pyframe == NULL) { - break; - } + _PyInterpreterFrame *pyframe = _PyThreadState_GetFrame(tstate); + while (pyframe) { if (traceback->nframe < tracemalloc_config.max_nframe) { tracemalloc_get_frame(pyframe, &traceback->frames[traceback->nframe]); assert(traceback->frames[traceback->nframe].filename != NULL); @@ -363,8 +357,7 @@ traceback_get_frames(traceback_t *traceback) if (traceback->total_nframe < UINT16_MAX) { traceback->total_nframe++; } - - pyframe = pyframe->previous; + pyframe = _PyFrame_GetFirstComplete(pyframe->previous); } } diff --git a/Modules/signalmodule.c b/Modules/signalmodule.c index 538a7e85bc950c..44a5ecf63e9d1e 100644 --- a/Modules/signalmodule.c +++ b/Modules/signalmodule.c @@ -1803,10 +1803,7 @@ _PyErr_CheckSignalsTstate(PyThreadState *tstate) */ _Py_atomic_store(&is_tripped, 0); - _PyInterpreterFrame *frame = tstate->cframe->current_frame; - while (frame && _PyFrame_IsIncomplete(frame)) { - frame = frame->previous; - } + _PyInterpreterFrame *frame = _PyThreadState_GetFrame(tstate); signal_state_t *state = &signal_global_state; for (int i = 1; i < Py_NSIG; i++) { if (!_Py_atomic_load_relaxed(&Handlers[i].tripped)) { diff --git a/Objects/frameobject.c b/Objects/frameobject.c index ebe3bfe76d5df5..39ccca70f9cbf3 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -1405,9 +1405,7 @@ PyFrame_GetBack(PyFrameObject *frame) PyFrameObject *back = frame->f_back; if (back == NULL) { _PyInterpreterFrame *prev = frame->f_frame->previous; - while (prev && _PyFrame_IsIncomplete(prev)) { - prev = prev->previous; - } + prev = _PyFrame_GetFirstComplete(prev); if (prev) { back = _PyFrame_GetFrameObject(prev); } diff --git a/Objects/genobject.c b/Objects/genobject.c index 6f4046eaa0efc8..2adb1e4bf308e4 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -903,8 +903,11 @@ _Py_MakeCoro(PyFunctionObject *func) if (origin_depth == 0) { ((PyCoroObject *)coro)->cr_origin_or_finalizer = NULL; } else { - assert(_PyEval_GetFrame()); - PyObject *cr_origin = compute_cr_origin(origin_depth, _PyEval_GetFrame()->previous); + _PyInterpreterFrame *frame = tstate->cframe->current_frame; + assert(frame); + assert(_PyFrame_IsIncomplete(frame)); + frame = _PyFrame_GetFirstComplete(frame->previous); + PyObject *cr_origin = compute_cr_origin(origin_depth, frame); ((PyCoroObject *)coro)->cr_origin_or_finalizer = cr_origin; if (!cr_origin) { Py_DECREF(coro); @@ -1286,7 +1289,7 @@ compute_cr_origin(int origin_depth, _PyInterpreterFrame *current_frame) /* First count how many frames we have */ int frame_count = 0; for (; frame && frame_count < origin_depth; ++frame_count) { - frame = frame->previous; + frame = _PyFrame_GetFirstComplete(frame->previous); } /* Now collect them */ @@ -1305,7 +1308,7 @@ compute_cr_origin(int origin_depth, _PyInterpreterFrame *current_frame) return NULL; } PyTuple_SET_ITEM(cr_origin, i, frameinfo); - frame = frame->previous; + frame = _PyFrame_GetFirstComplete(frame->previous); } return cr_origin; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index f2d78cf50913ec..e4da5b24006dd9 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -9578,13 +9578,13 @@ super_init_impl(PyObject *self, PyTypeObject *type, PyObject *obj) { /* Call super(), without args -- fill in from __class__ and first local variable on the stack. */ PyThreadState *tstate = _PyThreadState_GET(); - _PyInterpreterFrame *cframe = tstate->cframe->current_frame; - if (cframe == NULL) { + _PyInterpreterFrame *frame = _PyThreadState_GetFrame(tstate); + if (frame == NULL) { PyErr_SetString(PyExc_RuntimeError, "super(): no current frame"); return -1; } - int res = super_init_without_args(cframe, cframe->f_code, &type, &obj); + int res = super_init_without_args(frame, frame->f_code, &type, &obj); if (res < 0) { return -1; diff --git a/Python/ceval.c b/Python/ceval.c index 56cd9ad6296366..7deee76cc5b89c 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -2749,16 +2749,13 @@ _PyInterpreterFrame * _PyEval_GetFrame(void) { PyThreadState *tstate = _PyThreadState_GET(); - return tstate->cframe->current_frame; + return _PyThreadState_GetFrame(tstate); } PyFrameObject * PyEval_GetFrame(void) { _PyInterpreterFrame *frame = _PyEval_GetFrame(); - while (frame && _PyFrame_IsIncomplete(frame)) { - frame = frame->previous; - } if (frame == NULL) { return NULL; } @@ -2772,7 +2769,7 @@ PyEval_GetFrame(void) PyObject * _PyEval_GetBuiltins(PyThreadState *tstate) { - _PyInterpreterFrame *frame = tstate->cframe->current_frame; + _PyInterpreterFrame *frame = _PyThreadState_GetFrame(tstate); if (frame != NULL) { return frame->f_builtins; } @@ -2811,7 +2808,7 @@ PyObject * PyEval_GetLocals(void) { PyThreadState *tstate = _PyThreadState_GET(); - _PyInterpreterFrame *current_frame = tstate->cframe->current_frame; + _PyInterpreterFrame *current_frame = _PyThreadState_GetFrame(tstate); if (current_frame == NULL) { _PyErr_SetString(tstate, PyExc_SystemError, "frame does not exist"); return NULL; @@ -2830,7 +2827,7 @@ PyObject * PyEval_GetGlobals(void) { PyThreadState *tstate = _PyThreadState_GET(); - _PyInterpreterFrame *current_frame = tstate->cframe->current_frame; + _PyInterpreterFrame *current_frame = _PyThreadState_GetFrame(tstate); if (current_frame == NULL) { return NULL; } diff --git a/Python/frame.c b/Python/frame.c index b1525cca511224..6a287d4724051a 100644 --- a/Python/frame.c +++ b/Python/frame.c @@ -96,10 +96,7 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame) } assert(!_PyFrame_IsIncomplete(frame)); assert(f->f_back == NULL); - _PyInterpreterFrame *prev = frame->previous; - while (prev && _PyFrame_IsIncomplete(prev)) { - prev = prev->previous; - } + _PyInterpreterFrame *prev = _PyFrame_GetFirstComplete(frame->previous); frame->previous = NULL; if (prev) { assert(prev->owner != FRAME_OWNED_BY_CSTACK); diff --git a/Python/pystate.c b/Python/pystate.c index f52fc38b358689..f2f571faf401bf 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1302,10 +1302,7 @@ PyFrameObject* PyThreadState_GetFrame(PyThreadState *tstate) { assert(tstate != NULL); - _PyInterpreterFrame *f = tstate->cframe->current_frame; - while (f && _PyFrame_IsIncomplete(f)) { - f = f->previous; - } + _PyInterpreterFrame *f = _PyThreadState_GetFrame(tstate); if (f == NULL) { return NULL; } @@ -1431,9 +1428,7 @@ _PyThread_CurrentFrames(void) PyThreadState *t; for (t = i->threads.head; t != NULL; t = t->next) { _PyInterpreterFrame *frame = t->cframe->current_frame; - while (frame && _PyFrame_IsIncomplete(frame)) { - frame = frame->previous; - } + frame = _PyFrame_GetFirstComplete(frame); if (frame == NULL) { continue; } diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 3f0baf98890b44..acee794864f916 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -1884,13 +1884,10 @@ sys__getframe_impl(PyObject *module, int depth) if (frame != NULL) { while (depth > 0) { - frame = frame->previous; + frame = _PyFrame_GetFirstComplete(frame->previous); if (frame == NULL) { break; } - if (_PyFrame_IsIncomplete(frame)) { - continue; - } --depth; } }