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

diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h
index 4e2051f23f9b252b5edf172b4228c53a5b6cada6..f12b225ebfccf24d3676cae3885c66428fd2e854 100644 (file)
--- 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 40c734b6e33abeb57730e2a890e3295fe3d2ab8c..6bb0144e9b1ed75f0f745da1875da3f65872e954 100644 (file)
--- 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 @@ class TestIncompleteFrameAreInvisible(unittest.TestCase):
             )
             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 (file)
index 0000000..0ec1425
--- /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 ac16626f2101baddd7ed145bf9cc4fb000273f99..9826ad2935beaac140e91a290219a4aef8423508 100644 (file)
--- 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 538a7e85bc950cfd9f3548eee4da77a3a985871c..44a5ecf63e9d1ee1d763e900c9f66ea1fbfcaa99 100644 (file)
--- 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 ebe3bfe76d5df56fe6f76869037a6fdbfe7fd639..39ccca70f9cbf3744537c8bb2ebdbbfa29080364 100644 (file)
--- 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 6f4046eaa0efc89275d15512e37eaf747b80607b..2adb1e4bf308e46fa0e8dfdc7dd6fafc7a7dee24 100644 (file)
--- 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 f2d78cf50913ecb933f47eb3684d6ed1d27c9fa8..e4da5b24006dd9ccfdafad8063fee1528b2e10ef 100644 (file)
--- 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 56cd9ad6296366545570bbc65528dabe8937f4c1..7deee76cc5b89c0ebb01049098648a2968a11040 100644 (file)
--- 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 b1525cca511224aa5105defde38f67e21e531040..6a287d4724051ab11c2e3479cbd1cec6815a247a 100644 (file)
--- 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 f52fc38b358689cb44c64c2e1b9d34b836349ce4..f2f571faf401bf57d321f03a05688ca567422cd5 100644 (file)
--- 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 3f0baf98890b44772621a40eba07ade7e5bc4d06..acee794864f916fcc443d98720222dc4dd488791 100644 (file)
--- 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;
         }
     }