Skip to content

Commit f387c6f

Browse files
committed
gh-138122: Add incomplete sample detection and fix generator frame unwinding
Add PyThreadState.entry_frame to track the bottommost frame in each thread. The profiler validates unwinding reached this frame, rejecting incomplete samples caused by race conditions or errors. Also fix unwinding to skip suspended generator frames which have NULL previous pointers.
1 parent 6462322 commit f387c6f

File tree

6 files changed

+61
-7
lines changed

6 files changed

+61
-7
lines changed

Include/cpython/pystate.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,11 @@ struct _ts {
135135
/* Pointer to currently executing frame. */
136136
struct _PyInterpreterFrame *current_frame;
137137

138+
/* Pointer to the entry/bottommost frame of the current call stack.
139+
* This is the frame that was entered when starting execution.
140+
* Used by profiling/sampling to detect incomplete stack traces. */
141+
struct _PyInterpreterFrame *entry_frame;
142+
138143
Py_tracefunc c_profilefunc;
139144
Py_tracefunc c_tracefunc;
140145
PyObject *c_profileobj;

Include/internal/pycore_debug_offsets.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ typedef struct _Py_DebugOffsets {
102102
uint64_t next;
103103
uint64_t interp;
104104
uint64_t current_frame;
105+
uint64_t entry_frame;
105106
uint64_t thread_id;
106107
uint64_t native_thread_id;
107108
uint64_t datastack_chunk;
@@ -272,6 +273,7 @@ typedef struct _Py_DebugOffsets {
272273
.next = offsetof(PyThreadState, next), \
273274
.interp = offsetof(PyThreadState, interp), \
274275
.current_frame = offsetof(PyThreadState, current_frame), \
276+
.entry_frame = offsetof(PyThreadState, entry_frame), \
275277
.thread_id = offsetof(PyThreadState, thread_id), \
276278
.native_thread_id = offsetof(PyThreadState, native_thread_id), \
277279
.datastack_chunk = offsetof(PyThreadState, datastack_chunk), \
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
Fix profiler stack unwinding to skip generator frames with NULL previous
2+
pointers. Generator frames at yield points are suspended objects that don't
3+
link to callers via normal frame chains, causing incomplete stack traces.
4+
The profiler now correctly skips these frames during stack unwinding.
5+
6+
Add incomplete sample detection to prevent corrupted profiling data. The
7+
interpreter now tracks the entry frame (bottommost frame) in each thread's
8+
``PyThreadState.entry_frame``, which the profiler uses to validate that
9+
stack unwinding reached the expected bottom. Samples that fail to unwind
10+
completely (due to race conditions, memory corruption, or other errors)
11+
are now rejected rather than being included as spurious single-frame stacks.

Modules/_remote_debugging_module.c

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2254,15 +2254,22 @@ is_frame_valid(
22542254
}
22552255

22562256
void* frame = (void*)frame_addr;
2257+
char owner = GET_MEMBER(char, frame, unwinder->debug_offsets.interpreter_frame.owner);
22572258

2258-
if (GET_MEMBER(char, frame, unwinder->debug_offsets.interpreter_frame.owner) == FRAME_OWNED_BY_INTERPRETER) {
2259+
if (owner == FRAME_OWNED_BY_INTERPRETER) {
22592260
return 0; // C frame
22602261
}
22612262

2262-
if (GET_MEMBER(char, frame, unwinder->debug_offsets.interpreter_frame.owner) != FRAME_OWNED_BY_GENERATOR
2263-
&& GET_MEMBER(char, frame, unwinder->debug_offsets.interpreter_frame.owner) != FRAME_OWNED_BY_THREAD) {
2264-
PyErr_Format(PyExc_RuntimeError, "Unhandled frame owner %d.\n",
2265-
GET_MEMBER(char, frame, unwinder->debug_offsets.interpreter_frame.owner));
2263+
// Reject generator frames with NULL previous pointer - can't unwind past them
2264+
if (owner == FRAME_OWNED_BY_GENERATOR) {
2265+
uintptr_t previous = GET_MEMBER(uintptr_t, frame, unwinder->debug_offsets.interpreter_frame.previous);
2266+
if (previous == 0) {
2267+
return 0; // Generator frame with no caller - skip it
2268+
}
2269+
}
2270+
2271+
if (owner != FRAME_OWNED_BY_GENERATOR && owner != FRAME_OWNED_BY_THREAD) {
2272+
PyErr_Format(PyExc_RuntimeError, "Unhandled frame owner %d.\n", owner);
22662273
set_exception_cause(unwinder, PyExc_RuntimeError, "Unhandled frame owner type in async frame");
22672274
return -1;
22682275
}
@@ -2475,14 +2482,17 @@ process_frame_chain(
24752482
uintptr_t initial_frame_addr,
24762483
StackChunkList *chunks,
24772484
PyObject *frame_info,
2478-
uintptr_t gc_frame)
2485+
uintptr_t gc_frame,
2486+
uintptr_t entry_frame_addr)
24792487
{
24802488
uintptr_t frame_addr = initial_frame_addr;
24812489
uintptr_t prev_frame_addr = 0;
2490+
uintptr_t last_frame_addr = 0; // Track the last frame we processed
24822491
const size_t MAX_FRAMES = 1024;
24832492
size_t frame_count = 0;
24842493

24852494
while ((void*)frame_addr != NULL) {
2495+
last_frame_addr = frame_addr; // Remember this frame before moving to next
24862496
PyObject *frame = NULL;
24872497
uintptr_t next_frame_addr = 0;
24882498
uintptr_t stackpointer = 0;
@@ -2564,6 +2574,17 @@ process_frame_chain(
25642574
frame_addr = next_frame_addr;
25652575
}
25662576

2577+
// Validate we reached the entry frame if it's set
2578+
if (entry_frame_addr != 0 && last_frame_addr != 0) {
2579+
if (last_frame_addr != entry_frame_addr) {
2580+
// We didn't reach the expected bottom frame - incomplete sample
2581+
PyErr_Format(PyExc_RuntimeError,
2582+
"Incomplete sample: reached frame 0x%lx but expected entry frame 0x%lx",
2583+
last_frame_addr, entry_frame_addr);
2584+
return -1;
2585+
}
2586+
}
2587+
25672588
return 0;
25682589
}
25692590

@@ -2806,7 +2827,13 @@ unwind_stack_for_thread(
28062827
goto error;
28072828
}
28082829

2809-
if (process_frame_chain(unwinder, frame_addr, &chunks, frame_info, gc_frame) < 0) {
2830+
// Read entry_frame for validation
2831+
uintptr_t entry_frame_addr = 0;
2832+
if (unwinder->debug_offsets.thread_state.entry_frame != 0) {
2833+
entry_frame_addr = GET_MEMBER(uintptr_t, ts, unwinder->debug_offsets.thread_state.entry_frame);
2834+
}
2835+
2836+
if (process_frame_chain(unwinder, frame_addr, &chunks, frame_info, gc_frame, entry_frame_addr) < 0) {
28102837
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to process frame chain");
28112838
goto error;
28122839
}

Python/ceval.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,6 +1234,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
12341234
entry.frame.previous = tstate->current_frame;
12351235
frame->previous = &entry.frame;
12361236
tstate->current_frame = frame;
1237+
/* Track entry frame for profiling/sampling */
1238+
if (entry.frame.previous == NULL) {
1239+
tstate->entry_frame = &entry.frame;
1240+
}
12371241
entry.frame.localsplus[0] = PyStackRef_NULL;
12381242
#ifdef _Py_TIER2
12391243
if (tstate->current_executor != NULL) {
@@ -1300,6 +1304,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
13001304
assert(frame->owner == FRAME_OWNED_BY_INTERPRETER);
13011305
/* Restore previous frame and exit */
13021306
tstate->current_frame = frame->previous;
1307+
/* Clear entry frame if we're returning to no frame */
1308+
if (tstate->current_frame == NULL) {
1309+
tstate->entry_frame = NULL;
1310+
}
13031311
return NULL;
13041312
}
13051313
#ifdef _Py_TIER2

Python/pystate.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1483,6 +1483,7 @@ init_threadstate(_PyThreadStateImpl *_tstate,
14831483
tstate->gilstate_counter = 1;
14841484

14851485
tstate->current_frame = NULL;
1486+
tstate->entry_frame = NULL;
14861487
tstate->datastack_chunk = NULL;
14871488
tstate->datastack_top = NULL;
14881489
tstate->datastack_limit = NULL;

0 commit comments

Comments
 (0)