-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
GH-119866: Spill stack pointer when making "escaping" calls. #119875
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
bd94999
e12729e
f748355
0a50860
c2775e0
2bd1339
792c553
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,7 +63,7 @@ typedef struct _PyInterpreterFrame { | |
PyObject *f_locals; /* Strong reference, may be NULL. Only valid if not on C stack */ | ||
PyFrameObject *frame_obj; /* Strong reference, may be NULL. Only valid if not on C stack */ | ||
_Py_CODEUNIT *instr_ptr; /* Instruction currently executing (or about to begin) */ | ||
int stacktop; /* Offset of TOS from localsplus */ | ||
PyObject **stackpointer; | ||
uint16_t return_offset; /* Only relevant during a function call */ | ||
char owner; | ||
/* Locals and stack */ | ||
|
@@ -83,20 +83,20 @@ static inline PyObject **_PyFrame_Stackbase(_PyInterpreterFrame *f) { | |
} | ||
|
||
static inline PyObject *_PyFrame_StackPeek(_PyInterpreterFrame *f) { | ||
assert(f->stacktop > _PyFrame_GetCode(f)->co_nlocalsplus); | ||
assert(f->localsplus[f->stacktop-1] != NULL); | ||
return f->localsplus[f->stacktop-1]; | ||
assert(f->stackpointer > f->localsplus + _PyFrame_GetCode(f)->co_nlocalsplus); | ||
assert(f->stackpointer[-1] != NULL); | ||
return f->stackpointer[-1]; | ||
} | ||
|
||
static inline PyObject *_PyFrame_StackPop(_PyInterpreterFrame *f) { | ||
assert(f->stacktop > _PyFrame_GetCode(f)->co_nlocalsplus); | ||
f->stacktop--; | ||
return f->localsplus[f->stacktop]; | ||
assert(f->stackpointer > f->localsplus + _PyFrame_GetCode(f)->co_nlocalsplus); | ||
f->stackpointer--; | ||
return *f->stackpointer; | ||
} | ||
|
||
static inline void _PyFrame_StackPush(_PyInterpreterFrame *f, PyObject *value) { | ||
f->localsplus[f->stacktop] = value; | ||
f->stacktop++; | ||
*f->stackpointer = value; | ||
f->stackpointer++; | ||
} | ||
|
||
#define FRAME_SPECIALS_SIZE ((int)((sizeof(_PyInterpreterFrame)-1)/sizeof(PyObject *))) | ||
|
@@ -112,9 +112,12 @@ _PyFrame_NumSlotsForCodeObject(PyCodeObject *code) | |
|
||
static inline void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *dest) | ||
{ | ||
assert(src->stacktop >= _PyFrame_GetCode(src)->co_nlocalsplus); | ||
*dest = *src; | ||
for (int i = 1; i < src->stacktop; i++) { | ||
assert(src->stackpointer != NULL); | ||
int stacktop = (int)(src->stackpointer - src->localsplus); | ||
assert(stacktop >= _PyFrame_GetCode(src)->co_nlocalsplus); | ||
dest->stackpointer = dest->localsplus + stacktop; | ||
for (int i = 1; i < stacktop; i++) { | ||
dest->localsplus[i] = src->localsplus[i]; | ||
} | ||
// Don't leave a dangling pointer to the old frame when creating generators | ||
|
@@ -136,7 +139,7 @@ _PyFrame_Initialize( | |
frame->f_builtins = func->func_builtins; | ||
frame->f_globals = func->func_globals; | ||
frame->f_locals = locals; | ||
frame->stacktop = code->co_nlocalsplus; | ||
frame->stackpointer = frame->localsplus + code->co_nlocalsplus; | ||
frame->frame_obj = NULL; | ||
frame->instr_ptr = _PyCode_CODE(code); | ||
frame->return_offset = 0; | ||
|
@@ -156,22 +159,29 @@ _PyFrame_GetLocalsArray(_PyInterpreterFrame *frame) | |
return frame->localsplus; | ||
} | ||
|
||
/* Fetches the stack pointer, and sets stacktop to -1. | ||
Having stacktop <= 0 ensures that invalid | ||
values are not visible to the cycle GC. | ||
We choose -1 rather than 0 to assist debugging. */ | ||
/* Fetches the stack pointer, and sets stackpointer to NULL. | ||
Having stackpointer == NULL ensures that invalid | ||
values are not visible to the cycle GC. */ | ||
static inline PyObject** | ||
_PyFrame_GetStackPointer(_PyInterpreterFrame *frame) | ||
{ | ||
PyObject **sp = frame->localsplus + frame->stacktop; | ||
frame->stacktop = -1; | ||
#ifndef Py_DEBUG | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this (and the corresponding line in the next function) be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. |
||
PyThreadState_GET()->sp_cached++; | ||
#endif | ||
assert(frame->stackpointer != NULL); | ||
PyObject **sp = frame->stackpointer; | ||
frame->stackpointer = NULL; | ||
return sp; | ||
} | ||
|
||
static inline void | ||
_PyFrame_SetStackPointer(_PyInterpreterFrame *frame, PyObject **stack_pointer) | ||
{ | ||
frame->stacktop = (int)(stack_pointer - frame->localsplus); | ||
#ifndef Py_DEBUG | ||
PyThreadState_GET()->sp_cached--; | ||
#endif | ||
assert(frame->stackpointer == NULL); | ||
frame->stackpointer = stack_pointer; | ||
} | ||
|
||
/* Determine whether a frame is incomplete. | ||
|
@@ -299,7 +309,8 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int | |
frame->f_globals = NULL; | ||
#endif | ||
frame->f_locals = NULL; | ||
frame->stacktop = code->co_nlocalsplus + stackdepth; | ||
assert(stackdepth <= code->co_stacksize); | ||
frame->stackpointer = frame->localsplus + code->co_nlocalsplus + stackdepth; | ||
frame->frame_obj = NULL; | ||
frame->instr_ptr = _PyCode_CODE(code); | ||
frame->owner = FRAME_OWNED_BY_THREAD; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If only used in debug builds, should this be inside
#ifdef Py_DEBUG
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think debug builds have the same ABI compatibility as release builds these days. I know we like to pretend that
PyThreadState
is opaque, but this seems like it might case some nasty bugs if we do this.