[Python-checkins] GH-100126: Skip incomplete frames in more places (GH-100613)
brandtbucher
webhook-mailer at python.org
Mon Jan 9 15:20:10 EST 2023
https://github.com/python/cpython/commit/61762b93871419b34f02d83cee5ca0d94d4a2903
commit: 61762b93871419b34f02d83cee5ca0d94d4a2903
branch: main
author: Brandt Bucher <brandtbucher at microsoft.com>
committer: brandtbucher <brandtbucher at gmail.com>
date: 2023-01-09T12:20:04-08:00
summary:
GH-100126: Skip incomplete frames in more places (GH-100613)
files:
A Misc/NEWS.d/next/Core and Builtins/2022-12-29-04-39-38.gh-issue-100126.pfFJd-.rst
M Include/internal/pycore_frame.h
M Lib/test/test_frame.py
M Modules/_tracemalloc.c
M Modules/signalmodule.c
M Objects/frameobject.c
M Objects/genobject.c
M Objects/typeobject.c
M Python/ceval.c
M Python/frame.c
M Python/pystate.c
M Python/sysmodule.c
diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h
index 4e2051f23f9b..f12b225ebfcc 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 40c734b6e33a..6bb0144e9b1e 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 000000000000..0ec14253ceff
--- /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 ac16626f2101..9826ad2935be 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 538a7e85bc95..44a5ecf63e9d 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 ebe3bfe76d5d..39ccca70f9cb 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 6f4046eaa0ef..2adb1e4bf308 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 f2d78cf50913..e4da5b24006d 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 56cd9ad62963..7deee76cc5b8 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 b1525cca5112..6a287d472405 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 f52fc38b3586..f2f571faf401 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 3f0baf98890b..acee794864f9 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;
}
}
More information about the Python-checkins
mailing list