Skip to content
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Include/cpython/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ struct _ts {
PyObject *previous_executor;

uint64_t dict_global_version;
int sp_cached; /* Only used in debug builds */
Copy link
Contributor

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?

Copy link
Member

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.


};

#ifdef Py_DEBUG
Expand Down
51 changes: 31 additions & 20 deletions Include/internal/pycore_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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 *)))
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this (and the corresponding line in the next function) be #ifdef Py_DEBUG? I don't think the assertion added to _Py_Dealloc will ever see an updated value of sp_cached otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

The 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.
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_sys.py
Original file line number Diff line number Diff line change
Expand Up @@ -1561,7 +1561,7 @@ class C(object): pass
def func():
return sys._getframe()
x = func()
check(x, size('3Pi2cP7P2ic??2P'))
check(x, size('3Pi2cP9Phc2P'))
# function
def func(): pass
check(func, size('16Pi'))
Expand All @@ -1578,7 +1578,7 @@ def bar(cls):
check(bar, size('PP'))
# generator
def get_gen(): yield 1
check(get_gen(), size('PP4P4c7P2ic??2P'))
check(get_gen(), size('PP4P4cP9PhcP'))
# iterator
check(iter('abc'), size('lP'))
# callable-iterator
Expand Down
18 changes: 11 additions & 7 deletions Objects/frameobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1619,8 +1619,10 @@ frame_dealloc(PyFrameObject *f)
Py_CLEAR(frame->f_funcobj);
Py_CLEAR(frame->f_locals);
PyObject **locals = _PyFrame_GetLocalsArray(frame);
for (int i = 0; i < frame->stacktop; i++) {
Py_CLEAR(locals[i]);
PyObject **sp = frame->stackpointer;
while (sp > locals) {
sp--;
Py_CLEAR(*sp);
}
}
Py_CLEAR(f->f_back);
Expand Down Expand Up @@ -1652,11 +1654,13 @@ frame_tp_clear(PyFrameObject *f)

/* locals and stack */
PyObject **locals = _PyFrame_GetLocalsArray(f->f_frame);
assert(f->f_frame->stacktop >= 0);
for (int i = 0; i < f->f_frame->stacktop; i++) {
Py_CLEAR(locals[i]);
PyObject **sp = f->f_frame->stackpointer;
assert(sp >= locals);
while (sp > locals) {
sp--;
Py_CLEAR(*sp);
}
f->f_frame->stacktop = 0;
f->f_frame->stackpointer = locals;
Py_CLEAR(f->f_frame->f_locals);
return 0;
}
Expand Down Expand Up @@ -1874,7 +1878,7 @@ frame_get_var(_PyInterpreterFrame *frame, PyCodeObject *co, int i,
}

PyObject *value = frame->localsplus[i];
if (frame->stacktop) {
if (frame->stackpointer > frame->localsplus) {
if (kind & CO_FAST_FREE) {
// The cell was set by COPY_FREE_VARS.
assert(value != NULL && PyCell_Check(value));
Expand Down
1 change: 1 addition & 0 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -2831,6 +2831,7 @@ _Py_Dealloc(PyObject *op)
destructor dealloc = type->tp_dealloc;
#ifdef Py_DEBUG
PyThreadState *tstate = _PyThreadState_GET();
assert(tstate->sp_cached <= 1);
PyObject *old_exc = tstate != NULL ? tstate->current_exception : NULL;
// Keep the old exception type alive to prevent undefined behavior
// on (tstate->curexc_type != old_exc_type) below
Expand Down
Loading