GH-100126: Skip incomplete frames in more places (GH-100613)

Author: brandtbucher

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 *

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):

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.

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);
     }
 }
 

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)) {

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);
         }

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;

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;

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;
     }

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);

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;
             }

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;
         }
     }