Skip to content

Commit 3e36e42

Browse files
maurycymiss-islington
authored andcommitted
gh-151436: Fix missing tstate->last_profiled_frame updates (GH-151437)
(cherry picked from commit a8d74c0) Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com>
1 parent 8fe5897 commit 3e36e42

8 files changed

Lines changed: 33 additions & 8 deletions

File tree

Include/internal/pycore_interpframe.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,20 @@ _PyThreadState_GetFrame(PyThreadState *tstate)
282282
return _PyFrame_GetFirstComplete(tstate->current_frame);
283283
}
284284

285+
// Update last_profiled_frame for remote profiler frame caching.
286+
// Only update if we're removing the exact frame that was last profiled.
287+
// This avoids corrupting the cache when transient frames (called and returned
288+
// between profiler samples) update last_profiled_frame to addresses the
289+
// profiler never saw.
290+
#define _PyThreadState_UpdateLastProfiledFrame(tstate, frame, previous) \
291+
do { \
292+
PyThreadState *tstate_ = (tstate); \
293+
_PyInterpreterFrame *frame_ = (frame); \
294+
if (tstate_->last_profiled_frame == frame_) { \
295+
tstate_->last_profiled_frame = (previous); \
296+
} \
297+
} while (0)
298+
285299
/* For use by _PyFrame_GetFrameObject
286300
Do not call directly. */
287301
PyAPI_FUNC(PyFrameObject *)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix skewed stack trackes in the Tachyon profiler when caching is enabled and
2+
when generators and coroutines are profiled, by updating
3+
``tstate->last_profiled_frame`` at every frame-removal site. The issue resulted
4+
in total erasure of some callers. Patch by Maurycy Pawłowski-Wieroński.

Modules/_testinternalcapi/test_cases.c.h

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Objects/genobject.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ gen_clear_frame(PyGenObject *gen)
168168
{
169169
assert(FT_ATOMIC_LOAD_INT8_RELAXED(gen->gi_frame_state) == FRAME_CLEARED);
170170
_PyInterpreterFrame *frame = &gen->gi_iframe;
171+
_PyThreadState_UpdateLastProfiledFrame(_PyThreadState_GET(), frame, frame->previous);
171172
frame->previous = NULL;
172173
_PyFrame_ClearExceptCode(frame);
173174
_PyErr_ClearExcState(&gen->gi_exc_state);
@@ -681,6 +682,7 @@ _gen_throw(PyGenObject *gen, int close_on_genexit,
681682
'yield from' or awaiting on with 'await'. */
682683
ret = _gen_throw((PyGenObject *)yf, close_on_genexit,
683684
typ, val, tb);
685+
_PyThreadState_UpdateLastProfiledFrame(tstate, frame, prev);
684686
tstate->current_frame = prev;
685687
frame->previous = NULL;
686688
}
@@ -701,6 +703,7 @@ _gen_throw(PyGenObject *gen, int close_on_genexit,
701703
frame->previous = prev;
702704
tstate->current_frame = frame;
703705
ret = PyObject_CallFunctionObjArgs(meth, typ, val, tb, NULL);
706+
_PyThreadState_UpdateLastProfiledFrame(tstate, frame, prev);
704707
tstate->current_frame = prev;
705708
frame->previous = NULL;
706709
Py_DECREF(meth);

Python/bytecodes.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1859,6 +1859,7 @@ dummy_func(
18591859
gen->gi_exc_state.previous_item = NULL;
18601860
_Py_LeaveRecursiveCallPy(tstate);
18611861
_PyInterpreterFrame *gen_frame = frame;
1862+
_PyThreadState_UpdateLastProfiledFrame(tstate, gen_frame, gen_frame->previous);
18621863
frame = tstate->current_frame = frame->previous;
18631864
gen_frame->previous = NULL;
18641865
((_PyThreadStateImpl *)tstate)->generator_return_kind = GENERATOR_YIELD;
@@ -5880,6 +5881,7 @@ dummy_func(
58805881
gen_frame->owner = FRAME_OWNED_BY_GENERATOR;
58815882
_Py_LeaveRecursiveCallPy(tstate);
58825883
_PyInterpreterFrame *prev = frame->previous;
5884+
_PyThreadState_UpdateLastProfiledFrame(tstate, frame, prev);
58835885
_PyThreadState_PopFrame(tstate, frame);
58845886
frame = tstate->current_frame = prev;
58855887
LOAD_IP(frame->return_offset);

Python/ceval.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1988,15 +1988,8 @@ clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame)
19881988
void
19891989
_PyEval_FrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame * frame)
19901990
{
1991-
// Update last_profiled_frame for remote profiler frame caching.
19921991
// By this point, tstate->current_frame is already set to the parent frame.
1993-
// Only update if we're popping the exact frame that was last profiled.
1994-
// This avoids corrupting the cache when transient frames (called and returned
1995-
// between profiler samples) update last_profiled_frame to addresses the
1996-
// profiler never saw.
1997-
if (tstate->last_profiled_frame != NULL && tstate->last_profiled_frame == frame) {
1998-
tstate->last_profiled_frame = tstate->current_frame;
1999-
}
1992+
_PyThreadState_UpdateLastProfiledFrame(tstate, frame, tstate->current_frame);
20001993

20011994
if (frame->owner == FRAME_OWNED_BY_THREAD) {
20021995
clear_thread_frame(tstate, frame);
@@ -2022,6 +2015,7 @@ _PyEvalFramePushAndInit(PyThreadState *tstate, _PyStackRef func,
20222015
_PyFrame_Initialize(tstate, frame, func, locals, code, 0, previous);
20232016
if (initialize_locals(tstate, func_obj, frame->localsplus, args, argcount, kwnames)) {
20242017
assert(frame->owner == FRAME_OWNED_BY_THREAD);
2018+
_PyThreadState_UpdateLastProfiledFrame(tstate, frame, tstate->current_frame);
20252019
clear_thread_frame(tstate, frame);
20262020
return NULL;
20272021
}

Python/executor_cases.c.h

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/generated_cases.c.h

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)