GH-120024: Use pointer for stack pointer (GH-121923)

Author: markshannon

Include/internal/pycore_frame.h

@@ -68,7 +68,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  */
+    _PyStackRef *stackpointer;
     uint16_t return_offset;  /* Only relevant during a function call */
     char owner;
     /* Locals and stack */
@@ -88,20 +88,20 @@ static inline _PyStackRef *_PyFrame_Stackbase(_PyInterpreterFrame *f) {
 }
 
 static inline _PyStackRef _PyFrame_StackPeek(_PyInterpreterFrame *f) {
-    assert(f->stacktop > _PyFrame_GetCode(f)->co_nlocalsplus);
-    assert(!PyStackRef_IsNull(f->localsplus[f->stacktop-1]));
-    return f->localsplus[f->stacktop-1];
+    assert(f->stackpointer >  f->localsplus + _PyFrame_GetCode(f)->co_nlocalsplus);
+    assert(!PyStackRef_IsNull(f->stackpointer[-1]));
+    return f->stackpointer[-1];
 }
 
 static inline _PyStackRef _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, _PyStackRef value) {
-    f->localsplus[f->stacktop] = value;
-    f->stacktop++;
+    *f->stackpointer = value;
+    f->stackpointer++;
 }
 
 #define FRAME_SPECIALS_SIZE ((int)((sizeof(_PyInterpreterFrame)-1)/sizeof(PyObject *)))
@@ -117,9 +117,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
@@ -141,7 +144,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;
@@ -161,22 +164,23 @@ _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 _PyStackRef*
 _PyFrame_GetStackPointer(_PyInterpreterFrame *frame)
 {
-    _PyStackRef *sp = frame->localsplus + frame->stacktop;
-    frame->stacktop = -1;
+    assert(frame->stackpointer != NULL);
+    _PyStackRef *sp = frame->stackpointer;
+    frame->stackpointer = NULL;
     return sp;
 }
 
 static inline void
 _PyFrame_SetStackPointer(_PyInterpreterFrame *frame, _PyStackRef *stack_pointer)
 {
-    frame->stacktop = (int)(stack_pointer - frame->localsplus);
+    assert(frame->stackpointer == NULL);
+    frame->stackpointer = stack_pointer;
 }
 
 /* Determine whether a frame is incomplete.
@@ -304,7 +308,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;

Lib/test/test_sys.py

@@ -1603,7 +1603,8 @@ class C(object): pass
         def func():
             return sys._getframe()
         x = func()
-        check(x, size('3Pi2c2P7P2ic??2P'))
+        INTERPRETER_FRAME = '9PhcP'
+        check(x, size('3PiccPP' + INTERPRETER_FRAME + 'P'))
         # function
         def func(): pass
         check(func, size('16Pi'))
@@ -1620,7 +1621,7 @@ def bar(cls):
             check(bar, size('PP'))
         # generator
         def get_gen(): yield 1
-        check(get_gen(), size('PP4P4c7P2ic??2P'))
+        check(get_gen(), size('6P4c' + INTERPRETER_FRAME + 'P'))
         # iterator
         check(iter('abc'), size('lP'))
         # callable-iterator

Objects/frameobject.c

@@ -1620,8 +1620,10 @@ frame_dealloc(PyFrameObject *f)
         Py_CLEAR(frame->f_funcobj);
         Py_CLEAR(frame->f_locals);
         _PyStackRef *locals = _PyFrame_GetLocalsArray(frame);
-        for (int i = 0; i < frame->stacktop; i++) {
-            PyStackRef_CLEAR(locals[i]);
+        _PyStackRef *sp = frame->stackpointer;
+        while (sp > locals) {
+            sp--;
+            PyStackRef_CLEAR(*sp);
         }
     }
     Py_CLEAR(f->f_back);
@@ -1656,11 +1658,13 @@ frame_tp_clear(PyFrameObject *f)
 
     /* locals and stack */
     _PyStackRef *locals = _PyFrame_GetLocalsArray(f->f_frame);
-    assert(f->f_frame->stacktop >= 0);
-    for (int i = 0; i < f->f_frame->stacktop; i++) {
-        PyStackRef_CLEAR(locals[i]);
+    _PyStackRef *sp = f->f_frame->stackpointer;
+    assert(sp >= locals);
+    while (sp > locals) {
+        sp--;
+        PyStackRef_CLEAR(*sp);
     }
-    f->f_frame->stacktop = 0;
+    f->f_frame->stackpointer = locals;
     Py_CLEAR(f->f_frame->f_locals);
     return 0;
 }
@@ -1878,8 +1882,9 @@ frame_get_var(_PyInterpreterFrame *frame, PyCodeObject *co, int i,
         return 0;
     }
 
-    PyObject *value = PyStackRef_AsPyObjectBorrow(frame->localsplus[i]);
-    if (frame->stacktop) {
+    PyObject *value = NULL;
+    if (frame->stackpointer == NULL || frame->stackpointer > frame->localsplus + i) {
+        value = PyStackRef_AsPyObjectBorrow(frame->localsplus[i]);
         if (kind & CO_FAST_FREE) {
             // The cell was set by COPY_FREE_VARS.
             assert(value != NULL && PyCell_Check(value));
@@ -1897,9 +1902,6 @@ frame_get_var(_PyInterpreterFrame *frame, PyCodeObject *co, int i,
             }
         }
     }
-    else {
-        assert(value == NULL);
-    }
     *pvalue = value;
     return 1;
 }

Python/ceval.c

@@ -764,7 +764,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
 #endif
     entry_frame.f_executable = Py_None;
     entry_frame.instr_ptr = (_Py_CODEUNIT *)_Py_INTERPRETER_TRAMPOLINE_INSTRUCTIONS + 1;
-    entry_frame.stacktop = 0;
+    entry_frame.stackpointer = entry_frame.localsplus;
     entry_frame.owner = FRAME_OWNED_BY_CSTACK;
     entry_frame.return_offset = 0;
     /* Push frame */

Python/frame.c

@@ -17,10 +17,11 @@ _PyFrame_Traverse(_PyInterpreterFrame *frame, visitproc visit, void *arg)
     Py_VISIT(_PyFrame_GetCode(frame));
    /* locals */
     _PyStackRef *locals = _PyFrame_GetLocalsArray(frame);
-    int i = 0;
+    _PyStackRef *sp = frame->stackpointer;
     /* locals and stack */
-    for (; i <frame->stacktop; i++) {
-        Py_VISIT(PyStackRef_AsPyObjectBorrow(locals[i]));
+    while (sp > locals) {
+        sp--;
+        Py_VISIT(PyStackRef_AsPyObjectBorrow(*sp));
     }
     return 0;
 }
@@ -59,10 +60,11 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame)
     assert(frame->owner != FRAME_OWNED_BY_CSTACK);
     assert(frame->owner != FRAME_OWNED_BY_FRAME_OBJECT);
     assert(frame->owner != FRAME_CLEARED);
-    Py_ssize_t size = ((char*)&frame->localsplus[frame->stacktop]) - (char *)frame;
+    Py_ssize_t size = ((char*)frame->stackpointer) - (char *)frame;
     Py_INCREF(_PyFrame_GetCode(frame));
     memcpy((_PyInterpreterFrame *)f->_f_frame_data, frame, size);
     frame = (_PyInterpreterFrame *)f->_f_frame_data;
+    frame->stackpointer = (_PyStackRef *)(((char *)frame) + size);
     f->f_frame = frame;
     frame->owner = FRAME_OWNED_BY_FRAME_OBJECT;
     if (_PyFrame_IsIncomplete(frame)) {
@@ -97,11 +99,13 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame)
 void
 _PyFrame_ClearLocals(_PyInterpreterFrame *frame)
 {
-    assert(frame->stacktop >= 0);
-    int stacktop = frame->stacktop;
-    frame->stacktop = 0;
-    for (int i = 0; i < stacktop; i++) {
-        PyStackRef_XCLOSE(frame->localsplus[i]);
+    assert(frame->stackpointer != NULL);
+    _PyStackRef *sp = frame->stackpointer;
+    _PyStackRef *locals = frame->localsplus;
+    frame->stackpointer = locals;
+    while (sp > locals) {
+        sp--;
+        PyStackRef_XCLOSE(*sp);
     }
     Py_CLEAR(frame->f_locals);
 }