From ba30e9aaf8a5d2197e684711ad34e6a59055002d Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 11 Apr 2025 11:22:32 +0100 Subject: [PATCH 01/14] Use tagged integers on the evaluation stack for lasti --- Include/internal/pycore_stackref.h | 35 +++++++++++++++++++++++++++++- Python/bytecodes.c | 20 ++++------------- Python/ceval.c | 3 +++ Python/executor_cases.c.h | 4 +++- Python/gc.c | 10 +++++++-- Python/gc_free_threading.c | 1 + Python/generated_cases.c.h | 31 ++++++++++---------------- 7 files changed, 64 insertions(+), 40 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 5683b98470d3ea..5e66b91e7f10bc 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -210,8 +210,32 @@ _PyStackRef_FromPyObjectNewMortal(PyObject *obj, const char *filename, int linen extern int PyStackRef_Is(_PyStackRef a, _PyStackRef b); +extern _PyStackRef PyStackRef_TagInt(intptr_t i); + #else +#define Py_INT_TAG 3 + +static inline bool +PyStackRef_IsTaggedInt(_PyStackRef i) +{ + return (i.bits & Py_INT_TAG) == Py_INT_TAG; +} + +static inline _PyStackRef +PyStackRef_TagInt(intptr_t i) +{ + assert(Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, (i << 2), 2) == i); + return (_PyStackRef){ .bits = ((((uintptr_t)i) << 2) | Py_INT_TAG) }; +} + +static inline intptr_t +PyStackRef_UntagInt(_PyStackRef i) +{ + assert((i.bits & Py_INT_TAG) == Py_INT_TAG); + return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, i.bits, 2); +} + #ifdef Py_GIL_DISABLED @@ -232,6 +256,8 @@ static const _PyStackRef PyStackRef_NULL = { .bits = Py_TAG_DEFERRED}; #define PyStackRef_IsTrue(ref) (PyStackRef_AsPyObjectBorrow(ref) == Py_True) #define PyStackRef_IsFalse(ref) (PyStackRef_AsPyObjectBorrow(ref) == Py_False) +#define PyStackRef_IsNullOrInt(stackref) (PyStackRef_IsNull(stackref) || PyStackRef_IsTaggedInt(stackref)) + static inline PyObject * PyStackRef_AsPyObjectBorrow(_PyStackRef stackref) { @@ -451,6 +477,7 @@ PyStackRef_RefcountOnObject(_PyStackRef ref) static inline PyObject * PyStackRef_AsPyObjectBorrow(_PyStackRef ref) { + assert(!PyStackRef_IsTaggedInt(ref)); return BITS_TO_PTR_MASKED(ref); } @@ -587,6 +614,12 @@ PyStackRef_CLOSE(_PyStackRef ref) } #endif +static inline bool +PyStackRef_IsNullOrInt(_PyStackRef ref) +{ + return PyStackRef_IsNull(ref) || PyStackRef_IsTaggedInt(ref); +} + static inline void PyStackRef_CLOSE_SPECIALIZED(_PyStackRef ref, destructor destruct) { @@ -726,7 +759,7 @@ _Py_TryXGetStackRef(PyObject **src, _PyStackRef *out) // Like Py_VISIT but for _PyStackRef fields #define _Py_VISIT_STACKREF(ref) \ do { \ - if (!PyStackRef_IsNull(ref)) { \ + if (!PyStackRef_IsNullOrInt(ref)) { \ int vret = _PyGC_VisitStackRef(&(ref), visit, arg); \ if (vret) \ return vret; \ diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 95786c91371e98..dae6be035c295e 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1385,16 +1385,7 @@ dummy_func( assert(oparg >= 0 && oparg <= 2); if (oparg) { - PyObject *lasti = PyStackRef_AsPyObjectBorrow(values[0]); - if (PyLong_Check(lasti)) { - frame->instr_ptr = _PyFrame_GetBytecode(frame) + PyLong_AsLong(lasti); - assert(!_PyErr_Occurred(tstate)); - } - else { - _PyErr_SetString(tstate, PyExc_SystemError, "lasti is not an int"); - Py_DECREF(exc); - ERROR_NO_POP(); - } + frame->instr_ptr = _PyFrame_GetBytecode(frame) + PyStackRef_UntagInt(values[0]); } assert(exc && PyExceptionInstance_Check(exc)); _PyErr_SetRaisedException(tstate, exc); @@ -3458,7 +3449,7 @@ dummy_func( if (tb == NULL) { tb = Py_None; } - assert(PyStackRef_LongCheck(lasti)); + assert(PyStackRef_IsTaggedInt(lasti)); (void)lasti; // Shut up compiler warning if asserts are off PyObject *stack[5] = {NULL, PyStackRef_AsPyObjectBorrow(exit_self), exc, val_o, tb}; int has_self = !PyStackRef_IsNull(exit_self); @@ -5344,11 +5335,8 @@ dummy_func( } if (lasti) { int frame_lasti = _PyInterpreterFrame_LASTI(frame); - PyObject *lasti = PyLong_FromLong(frame_lasti); - if (lasti == NULL) { - goto exception_unwind; - } - _PyFrame_StackPush(frame, PyStackRef_FromPyObjectSteal(lasti)); + _PyStackRef lasti = PyStackRef_TagInt(frame_lasti); + _PyFrame_StackPush(frame, lasti); } /* Make the raw exception data diff --git a/Python/ceval.c b/Python/ceval.c index 278d9e36045da2..d8fff92fcb4338 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -146,6 +146,9 @@ dump_item(_PyStackRef item) printf(""); return; } + if (PyStackRef_IsTaggedInt(item)) { + printf("%ld", PyStackRef_UntagInt(item)); + } PyObject *obj = PyStackRef_AsPyObjectBorrow(item); if (obj == NULL) { printf(""); diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 9bfb13e2d9773f..8beab2069ee029 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -4462,7 +4462,9 @@ if (tb == NULL) { tb = Py_None; } - assert(PyStackRef_LongCheck(lasti)); + _PyFrame_SetStackPointer(frame, stack_pointer); + assert(PyStackRef_IsTaggedInt(lasti)); + stack_pointer = _PyFrame_GetStackPointer(frame); (void)lasti; PyObject *stack[5] = {NULL, PyStackRef_AsPyObjectBorrow(exit_self), exc, val_o, tb}; int has_self = !PyStackRef_IsNull(exit_self); diff --git a/Python/gc.c b/Python/gc.c index dad088e09f872f..58224acff2cdd9 100644 --- a/Python/gc.c +++ b/Python/gc.c @@ -547,6 +547,7 @@ _PyGC_VisitStackRef(_PyStackRef *ref, visitproc visit, void *arg) // This is a bit tricky! We want to ignore stackrefs with embedded // refcounts when computing the incoming references, but otherwise treat // them like normal. + assert(!PyStackRef_IsTaggedInt(*ref)); if (!PyStackRef_RefcountOnObject(*ref) && (visit == visit_decref)) { return 0; } @@ -560,7 +561,9 @@ _PyGC_VisitFrameStack(_PyInterpreterFrame *frame, visitproc visit, void *arg) _PyStackRef *ref = _PyFrame_GetLocalsArray(frame); /* locals and stack */ for (; ref < frame->stackpointer; ref++) { - _Py_VISIT_STACKREF(*ref); + if (!PyStackRef_IsTaggedInt(*ref)) { + _Py_VISIT_STACKREF(*ref); + } } return 0; } @@ -1495,8 +1498,11 @@ mark_stacks(PyInterpreterState *interp, PyGC_Head *visited, int visited_space, b objects_marked += move_to_reachable(func, &reachable, visited_space); while (sp > locals) { sp--; + if (PyStackRef_IsNullOrInt(*sp)) { + continue; + } PyObject *op = PyStackRef_AsPyObjectBorrow(*sp); - if (op == NULL || _Py_IsImmortal(op)) { + if (_Py_IsImmortal(op)) { continue; } if (_PyObject_IS_GC(op)) { diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index fa4cb56f01e800..3ba402e665ea43 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -1706,6 +1706,7 @@ _PyGC_VisitStackRef(_PyStackRef *ref, visitproc visit, void *arg) // This is a bit tricky! We want to ignore deferred references when // computing the incoming references, but otherwise treat them like // regular references. + assert(!PyStackRef_IsTaggedInt(*ref)); if (!PyStackRef_IsDeferred(*ref) || (visit != visit_decref && visit != visit_decref_unreachable)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 6fe647d6197a07..4a0658ed14080b 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -10096,20 +10096,12 @@ PyObject *exc = PyStackRef_AsPyObjectSteal(exc_st); assert(oparg >= 0 && oparg <= 2); if (oparg) { - PyObject *lasti = PyStackRef_AsPyObjectBorrow(values[0]); - if (PyLong_Check(lasti)) { - frame->instr_ptr = _PyFrame_GetBytecode(frame) + PyLong_AsLong(lasti); - assert(!_PyErr_Occurred(tstate)); - } - else { - stack_pointer += -1; - assert(WITHIN_STACK_BOUNDS()); - _PyFrame_SetStackPointer(frame, stack_pointer); - _PyErr_SetString(tstate, PyExc_SystemError, "lasti is not an int"); - Py_DECREF(exc); - stack_pointer = _PyFrame_GetStackPointer(frame); - JUMP_TO_LABEL(error); - } + stack_pointer += -1; + assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + frame->instr_ptr = _PyFrame_GetBytecode(frame) + PyStackRef_UntagInt(values[0]); + stack_pointer = _PyFrame_GetStackPointer(frame); + stack_pointer += 1; } assert(exc && PyExceptionInstance_Check(exc)); stack_pointer += -1; @@ -11964,7 +11956,9 @@ if (tb == NULL) { tb = Py_None; } - assert(PyStackRef_LongCheck(lasti)); + _PyFrame_SetStackPointer(frame, stack_pointer); + assert(PyStackRef_IsTaggedInt(lasti)); + stack_pointer = _PyFrame_GetStackPointer(frame); (void)lasti; PyObject *stack[5] = {NULL, PyStackRef_AsPyObjectBorrow(exit_self), exc, val_o, tb}; int has_self = !PyStackRef_IsNull(exit_self); @@ -12136,11 +12130,8 @@ JUMP_TO_LABEL(error); } if (lasti) { int frame_lasti = _PyInterpreterFrame_LASTI(frame); - PyObject *lasti = PyLong_FromLong(frame_lasti); - if (lasti == NULL) { - JUMP_TO_LABEL(exception_unwind); - } - _PyFrame_StackPush(frame, PyStackRef_FromPyObjectSteal(lasti)); + _PyStackRef lasti = PyStackRef_TagInt(frame_lasti); + _PyFrame_StackPush(frame, lasti); } PyObject *exc = _PyErr_GetRaisedException(tstate); _PyFrame_StackPush(frame, PyStackRef_FromPyObjectSteal(exc)); From 1d223c34e02e4d261385716370e9977e7464f791 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 14 Apr 2025 14:54:06 +0100 Subject: [PATCH 02/14] Add support for Py_STACKREF_DEBUG --- Include/internal/pycore_stackref.h | 40 +++++++++++++++++++++++------- Python/gc_free_threading.c | 6 ++--- Python/pystate.c | 2 +- Python/stackrefs.c | 28 +++++++++++++++++++-- 4 files changed, 61 insertions(+), 15 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 5e66b91e7f10bc..97c871120b26a5 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -63,11 +63,11 @@ extern void _Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _P static const _PyStackRef PyStackRef_NULL = { .index = 0 }; -#define PyStackRef_None ((_PyStackRef){ .index = 1 } ) -#define PyStackRef_False ((_PyStackRef){ .index = 2 }) -#define PyStackRef_True ((_PyStackRef){ .index = 3 }) +#define PyStackRef_None ((_PyStackRef){ .index = 2 } ) +#define PyStackRef_False ((_PyStackRef){ .index = 4 }) +#define PyStackRef_True ((_PyStackRef){ .index = 6 }) -#define LAST_PREDEFINED_STACKREF_INDEX 3 +#define LAST_PREDEFINED_STACKREF_INDEX 6 static inline int PyStackRef_IsNull(_PyStackRef ref) @@ -96,6 +96,7 @@ PyStackRef_IsNone(_PyStackRef ref) static inline PyObject * _PyStackRef_AsPyObjectBorrow(_PyStackRef ref, const char *filename, int linenumber) { + assert((ref.index & 1) == 0); _Py_stackref_record_borrow(ref, filename, linenumber); return _Py_stackref_get_object(ref); } @@ -132,31 +133,45 @@ _PyStackRef_FromPyObjectImmortal(PyObject *obj, const char *filename, int linenu } #define PyStackRef_FromPyObjectImmortal(obj) _PyStackRef_FromPyObjectImmortal(_PyObject_CAST(obj), __FILE__, __LINE__) +static inline bool +is_tagged_int(_PyStackRef ref) +{ + return (ref.index & 1) == 1; +} + static inline void _PyStackRef_CLOSE(_PyStackRef ref, const char *filename, int linenumber) { + if (is_tagged_int(ref)) { + return; + } PyObject *obj = _Py_stackref_close(ref, filename, linenumber); Py_DECREF(obj); } #define PyStackRef_CLOSE(REF) _PyStackRef_CLOSE((REF), __FILE__, __LINE__) + static inline void _PyStackRef_XCLOSE(_PyStackRef ref, const char *filename, int linenumber) { if (PyStackRef_IsNull(ref)) { return; } - PyObject *obj = _Py_stackref_close(ref, filename, linenumber); - Py_DECREF(obj); + _PyStackRef_CLOSE(ref, filename, linenumber); } #define PyStackRef_XCLOSE(REF) _PyStackRef_XCLOSE((REF), __FILE__, __LINE__) static inline _PyStackRef _PyStackRef_DUP(_PyStackRef ref, const char *filename, int linenumber) { - PyObject *obj = _Py_stackref_get_object(ref); - Py_INCREF(obj); - return _Py_stackref_create(obj, filename, linenumber); + if (ref.index & 1) { + return ref; + } + else { + PyObject *obj = _Py_stackref_get_object(ref); + Py_INCREF(obj); + return _Py_stackref_create(obj, filename, linenumber); + } } #define PyStackRef_DUP(REF) _PyStackRef_DUP(REF, __FILE__, __LINE__) @@ -210,8 +225,15 @@ _PyStackRef_FromPyObjectNewMortal(PyObject *obj, const char *filename, int linen extern int PyStackRef_Is(_PyStackRef a, _PyStackRef b); +extern bool PyStackRef_IsTaggedInt(_PyStackRef ref); + +extern intptr_t PyStackRef_UntagInt(_PyStackRef ref); + extern _PyStackRef PyStackRef_TagInt(intptr_t i); +extern bool +PyStackRef_IsNullOrInt(_PyStackRef ref); + #else #define Py_INT_TAG 3 diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 3ba402e665ea43..d22307ae4ff74e 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -265,7 +265,7 @@ frame_disable_deferred_refcounting(_PyInterpreterFrame *frame) frame->f_funcobj = PyStackRef_AsStrongReference(frame->f_funcobj); for (_PyStackRef *ref = frame->localsplus; ref < frame->stackpointer; ref++) { - if (!PyStackRef_IsNull(*ref) && PyStackRef_IsDeferred(*ref)) { + if (!PyStackRef_IsNullOrInt(*ref) && PyStackRef_IsDeferred(*ref)) { *ref = PyStackRef_AsStrongReference(*ref); } } @@ -420,7 +420,7 @@ gc_visit_heaps(PyInterpreterState *interp, mi_block_visit_fun *visitor, static inline void gc_visit_stackref(_PyStackRef stackref) { - if (PyStackRef_IsDeferred(stackref) && !PyStackRef_IsNull(stackref)) { + if (PyStackRef_IsDeferred(stackref) && !PyStackRef_IsNullOrInt(stackref)) { PyObject *obj = PyStackRef_AsPyObjectBorrow(stackref); if (_PyObject_GC_IS_TRACKED(obj) && !gc_is_frozen(obj)) { gc_add_refs(obj, 1); @@ -817,7 +817,7 @@ gc_abort_mark_alive(PyInterpreterState *interp, static int gc_visit_stackref_mark_alive(gc_mark_args_t *args, _PyStackRef stackref) { - if (!PyStackRef_IsNull(stackref)) { + if (!PyStackRef_IsNullOrInt(stackref)) { PyObject *op = PyStackRef_AsPyObjectBorrow(stackref); if (gc_mark_enqueue(op, args) < 0) { return -1; diff --git a/Python/pystate.c b/Python/pystate.c index ee35f0fa945f8b..bee6c7eae30e68 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -673,7 +673,7 @@ init_interpreter(PyInterpreterState *interp, interp->dtoa = (struct _dtoa_state)_dtoa_state_INIT(interp); } #if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG) - interp->next_stackref = 1; + interp->next_stackref = 2; _Py_hashtable_allocator_t alloc = { .malloc = malloc, .free = free, diff --git a/Python/stackrefs.c b/Python/stackrefs.c index 1a2f66feb04c4d..09a2146e166d51 100644 --- a/Python/stackrefs.c +++ b/Python/stackrefs.c @@ -113,7 +113,8 @@ _Py_stackref_create(PyObject *obj, const char *filename, int linenumber) Py_FatalError("Cannot create a stackref for NULL"); } PyInterpreterState *interp = PyInterpreterState_Get(); - uint64_t new_id = interp->next_stackref++; + uint64_t new_id = interp->next_stackref; + interp->next_stackref = new_id + 2; TableEntry *entry = make_table_entry(obj, filename, linenumber); if (entry == NULL) { Py_FatalError("No memory left for stackref debug table"); @@ -152,7 +153,7 @@ void _Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _PyStackRef ref) { assert(interp->next_stackref >= ref.index); - interp->next_stackref = ref.index+1; + interp->next_stackref = ref.index+2; TableEntry *entry = make_table_entry(obj, "builtin-object", 0); if (entry == NULL) { Py_FatalError("No memory left for stackref debug table"); @@ -197,4 +198,27 @@ _PyStackRef_CLOSE_SPECIALIZED(_PyStackRef ref, destructor destruct, const char * _Py_DECREF_SPECIALIZED(obj, destruct); } +_PyStackRef PyStackRef_TagInt(intptr_t i) +{ + return (_PyStackRef){ .index = (i << 1) + 1 }; +} + +intptr_t +PyStackRef_UntagInt(_PyStackRef i) +{ + assert(is_tagged_int(i)); + return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, i.index, 1); +} + +bool PyStackRef_IsTaggedInt(_PyStackRef ref) +{ + return is_tagged_int(ref); +} + +bool +PyStackRef_IsNullOrInt(_PyStackRef ref) +{ + return PyStackRef_IsNull(ref) || PyStackRef_IsTaggedInt(ref); +} + #endif From 246199bd4fe929dd0653cb4dbd549dd37b5fed8d Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 15 Apr 2025 10:10:04 +0100 Subject: [PATCH 03/14] Add news --- .../2025-04-15-10-09-49.gh-issue-132508.zVe3iI.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-04-15-10-09-49.gh-issue-132508.zVe3iI.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-04-15-10-09-49.gh-issue-132508.zVe3iI.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-04-15-10-09-49.gh-issue-132508.zVe3iI.rst new file mode 100644 index 00000000000000..1f4fe1d5f4e2da --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-04-15-10-09-49.gh-issue-132508.zVe3iI.rst @@ -0,0 +1,3 @@ +Uses tagged integers on the evaluation stack to represent the instruction +offsets when reraising an exception. This avoids the need to box the integer +which could fail in low memory conditions. From b1b1252ec73e12c91b2df8e746e286ca577b89d0 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 15 Apr 2025 10:21:07 +0100 Subject: [PATCH 04/14] Fix dump_item --- Python/ceval.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/ceval.c b/Python/ceval.c index d8fff92fcb4338..b94c2c756d180a 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -147,7 +147,8 @@ dump_item(_PyStackRef item) return; } if (PyStackRef_IsTaggedInt(item)) { - printf("%ld", PyStackRef_UntagInt(item)); + printf("%" PRId64, (int64_t)PyStackRef_UntagInt(item)); + return; } PyObject *obj = PyStackRef_AsPyObjectBorrow(item); if (obj == NULL) { From 8d3c3ea80b3c997777ef87ec1b973632f811a735 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 15 Apr 2025 15:44:39 +0100 Subject: [PATCH 05/14] Clarify division between pre-defined stackref indexes and others --- Include/internal/pycore_stackref.h | 4 +++- Python/pystate.c | 2 +- Python/stackrefs.c | 7 +++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 97c871120b26a5..55870070ee5b94 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -63,11 +63,13 @@ extern void _Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _P static const _PyStackRef PyStackRef_NULL = { .index = 0 }; +// Use the first 3 even numbers for None, True and False. +// Odd numbers are reserved for (tagged) integers #define PyStackRef_None ((_PyStackRef){ .index = 2 } ) #define PyStackRef_False ((_PyStackRef){ .index = 4 }) #define PyStackRef_True ((_PyStackRef){ .index = 6 }) -#define LAST_PREDEFINED_STACKREF_INDEX 6 +#define INITIAL_STACKREF_INDEX 8 static inline int PyStackRef_IsNull(_PyStackRef ref) diff --git a/Python/pystate.c b/Python/pystate.c index bee6c7eae30e68..9a05ece3692a77 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -673,7 +673,7 @@ init_interpreter(PyInterpreterState *interp, interp->dtoa = (struct _dtoa_state)_dtoa_state_INIT(interp); } #if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG) - interp->next_stackref = 2; + interp->next_stackref = INITIAL_STACKREF_INDEX; _Py_hashtable_allocator_t alloc = { .malloc = malloc, .free = free, diff --git a/Python/stackrefs.c b/Python/stackrefs.c index 09a2146e166d51..dd7b7c19731cfc 100644 --- a/Python/stackrefs.c +++ b/Python/stackrefs.c @@ -70,7 +70,7 @@ _Py_stackref_close(_PyStackRef ref, const char *filename, int linenumber) } PyObject *obj; - if (ref.index <= LAST_PREDEFINED_STACKREF_INDEX) { + if (ref.index < INITIAL_STACKREF_INDEX) { if (ref.index == 0) { _Py_FatalErrorFormat(__func__, "Passing NULL to PyStackRef_CLOSE at %s:%d\n", filename, linenumber); } @@ -128,7 +128,7 @@ _Py_stackref_create(PyObject *obj, const char *filename, int linenumber) void _Py_stackref_record_borrow(_PyStackRef ref, const char *filename, int linenumber) { - if (ref.index <= LAST_PREDEFINED_STACKREF_INDEX) { + if (ref.index < INITIAL_STACKREF_INDEX) { return; } PyInterpreterState *interp = PyInterpreterState_Get(); @@ -152,8 +152,7 @@ _Py_stackref_record_borrow(_PyStackRef ref, const char *filename, int linenumber void _Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _PyStackRef ref) { - assert(interp->next_stackref >= ref.index); - interp->next_stackref = ref.index+2; + assert(ref.index < INITIAL_STACKREF_INDEX); TableEntry *entry = make_table_entry(obj, "builtin-object", 0); if (entry == NULL) { Py_FatalError("No memory left for stackref debug table"); From aa4ddf08574e51ec1980230e56a5137ea5380aba Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 15 Apr 2025 16:54:34 +0100 Subject: [PATCH 06/14] Cast before shifting --- Include/internal/pycore_stackref.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 55870070ee5b94..448a32d1cd3585 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -257,7 +257,8 @@ static inline intptr_t PyStackRef_UntagInt(_PyStackRef i) { assert((i.bits & Py_INT_TAG) == Py_INT_TAG); - return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, i.bits, 2); + intptr_t val = (intptr_t)i.bits; + return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, val, 2); } From e223e367128aacea97638b1748542afd30d6b4a3 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 15 Apr 2025 17:47:11 +0100 Subject: [PATCH 07/14] Integer tagging doesn't escape --- Python/executor_cases.c.h | 2 -- Python/generated_cases.c.h | 7 ------- Tools/cases_generator/analyzer.py | 3 +++ 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 8beab2069ee029..bdac45e2c1e9b0 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -4462,9 +4462,7 @@ if (tb == NULL) { tb = Py_None; } - _PyFrame_SetStackPointer(frame, stack_pointer); assert(PyStackRef_IsTaggedInt(lasti)); - stack_pointer = _PyFrame_GetStackPointer(frame); (void)lasti; PyObject *stack[5] = {NULL, PyStackRef_AsPyObjectBorrow(exit_self), exc, val_o, tb}; int has_self = !PyStackRef_IsNull(exit_self); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 4a0658ed14080b..3182f9b0cc768d 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -10096,12 +10096,7 @@ PyObject *exc = PyStackRef_AsPyObjectSteal(exc_st); assert(oparg >= 0 && oparg <= 2); if (oparg) { - stack_pointer += -1; - assert(WITHIN_STACK_BOUNDS()); - _PyFrame_SetStackPointer(frame, stack_pointer); frame->instr_ptr = _PyFrame_GetBytecode(frame) + PyStackRef_UntagInt(values[0]); - stack_pointer = _PyFrame_GetStackPointer(frame); - stack_pointer += 1; } assert(exc && PyExceptionInstance_Check(exc)); stack_pointer += -1; @@ -11956,9 +11951,7 @@ if (tb == NULL) { tb = Py_None; } - _PyFrame_SetStackPointer(frame, stack_pointer); assert(PyStackRef_IsTaggedInt(lasti)); - stack_pointer = _PyFrame_GetStackPointer(frame); (void)lasti; PyObject *stack[5] = {NULL, PyStackRef_AsPyObjectBorrow(exit_self), exc, val_o, tb}; int has_self = !PyStackRef_IsNull(exit_self); diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index a217d7136a5401..dddbf2cf872e3d 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -678,6 +678,9 @@ def has_error_without_pop(op: parser.CodeDef) -> bool: "JUMP_TO_LABEL", "restart_backoff_counter", "_Py_ReachedRecursionLimit", + "PyStackRef_IsTaggedInt", + "PyStackRef_TagInt", + "PyStackRef_UntagInt", ) def check_escaping_calls(instr: parser.CodeDef, escapes: dict[SimpleStmt, EscapingCall]) -> None: From 9e11cc732ce16f072111203272062d9993e6e311 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 15 Apr 2025 18:19:22 +0100 Subject: [PATCH 08/14] Push assert up to caller --- Include/internal/pycore_stackref.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 448a32d1cd3585..c8359118ccf415 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -506,6 +506,11 @@ PyStackRef_AsPyObjectBorrow(_PyStackRef ref) return BITS_TO_PTR_MASKED(ref); } +#ifdef Py_DEBUG +#define PyStackRef_AsPyObjectBorrow(REF) \ + (assert(!PyStackRef_IsTaggedInt(REF)), PyStackRef_AsPyObjectBorrow(REF)) +#endif + static inline _PyStackRef PyStackRef_Borrow(_PyStackRef ref) { From 79ed46982ea950b15ebac18526a671f17c7d622b Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 16 Apr 2025 08:33:34 +0100 Subject: [PATCH 09/14] Avoid side effects in macro --- Objects/frameobject.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Objects/frameobject.c b/Objects/frameobject.c index db4e4b7e1939de..7a62219c139ee5 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -1813,15 +1813,16 @@ frame_lineno_set_impl(PyFrameObject *self, PyObject *value) start_stack = pop_value(start_stack); } while (start_stack > best_stack) { + _PyStackRef popped = _PyFrame_StackPop(self->f_frame); if (top_of_stack(start_stack) == Except) { /* Pop exception stack as well as the evaluation stack */ - PyObject *exc = PyStackRef_AsPyObjectBorrow(_PyFrame_StackPop(self->f_frame)); + PyObject *exc = PyStackRef_AsPyObjectBorrow(popped); assert(PyExceptionInstance_Check(exc) || exc == Py_None); PyThreadState *tstate = _PyThreadState_GET(); Py_XSETREF(tstate->exc_info->exc_value, exc == Py_None ? NULL : exc); } else { - PyStackRef_XCLOSE(_PyFrame_StackPop(self->f_frame)); + PyStackRef_XCLOSE(popped); } start_stack = pop_value(start_stack); } From b4915443c7dcd1d208bd32f082a4429ec5009e03 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 16 Apr 2025 09:17:15 +0100 Subject: [PATCH 10/14] Remove debug code --- Include/internal/pycore_interpframe.h | 1 + Include/internal/pycore_stackref.h | 5 ----- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/Include/internal/pycore_interpframe.h b/Include/internal/pycore_interpframe.h index 1d373d55ab2de8..d3fd218b27eed7 100644 --- a/Include/internal/pycore_interpframe.h +++ b/Include/internal/pycore_interpframe.h @@ -18,6 +18,7 @@ extern "C" { ((int)((IF)->instr_ptr - _PyFrame_GetBytecode((IF)))) static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) { + assert(!PyStackRef_IsNull(f->f_executable)); PyObject *executable = PyStackRef_AsPyObjectBorrow(f->f_executable); assert(PyCode_Check(executable)); return (PyCodeObject *)executable; diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index c8359118ccf415..448a32d1cd3585 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -506,11 +506,6 @@ PyStackRef_AsPyObjectBorrow(_PyStackRef ref) return BITS_TO_PTR_MASKED(ref); } -#ifdef Py_DEBUG -#define PyStackRef_AsPyObjectBorrow(REF) \ - (assert(!PyStackRef_IsTaggedInt(REF)), PyStackRef_AsPyObjectBorrow(REF)) -#endif - static inline _PyStackRef PyStackRef_Borrow(_PyStackRef ref) { From f6126b1622c56f3698882084f3e79ad65880c81e Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 16 Apr 2025 10:06:09 +0100 Subject: [PATCH 11/14] Handle negative ints properly in Py_STACKREF_DEBUG build --- Python/stackrefs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/stackrefs.c b/Python/stackrefs.c index dd7b7c19731cfc..1bd0288fce690e 100644 --- a/Python/stackrefs.c +++ b/Python/stackrefs.c @@ -206,7 +206,8 @@ intptr_t PyStackRef_UntagInt(_PyStackRef i) { assert(is_tagged_int(i)); - return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, i.index, 1); + intptr_t val = (intptr_t)i.index; + return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, val, 1); } bool PyStackRef_IsTaggedInt(_PyStackRef ref) From 1b4aba64e1be121f3af13847ba2c412bbd78b146 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Mon, 28 Apr 2025 18:09:19 +0100 Subject: [PATCH 12/14] Brandt's fix to the stencils --- Tools/jit/_stencils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Tools/jit/_stencils.py b/Tools/jit/_stencils.py index 8faa9e8cac2d85..df9f21f5b782c8 100644 --- a/Tools/jit/_stencils.py +++ b/Tools/jit/_stencils.py @@ -320,6 +320,7 @@ def process_relocations( elif ( hole.kind in {"IMAGE_REL_AMD64_REL32"} and hole.value is HoleValue.ZERO + and hole.symbol not in self.symbols ): raise ValueError( f"Add PyAPI_FUNC(...) or PyAPI_DATA(...) to declaration of {hole.symbol}!" From d84cd2d61c1f423a210beb68c8e640f3aa23e5c6 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 29 Apr 2025 08:01:04 +0100 Subject: [PATCH 13/14] Put Brandt's patch in the right place :) --- Tools/jit/_stencils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tools/jit/_stencils.py b/Tools/jit/_stencils.py index df9f21f5b782c8..639e4bcc79344e 100644 --- a/Tools/jit/_stencils.py +++ b/Tools/jit/_stencils.py @@ -291,6 +291,7 @@ def process_relocations( hole.kind in {"R_AARCH64_CALL26", "R_AARCH64_JUMP26", "ARM64_RELOC_BRANCH26"} and hole.value is HoleValue.ZERO + and hole.symbol not in self.symbols ): hole.func = "patch_aarch64_trampoline" hole.need_state = True @@ -320,7 +321,6 @@ def process_relocations( elif ( hole.kind in {"IMAGE_REL_AMD64_REL32"} and hole.value is HoleValue.ZERO - and hole.symbol not in self.symbols ): raise ValueError( f"Add PyAPI_FUNC(...) or PyAPI_DATA(...) to declaration of {hole.symbol}!" From fb89cd9d2d810a1c142599944b419b831c1c3ed8 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 29 Apr 2025 14:23:13 +0100 Subject: [PATCH 14/14] Replace is_tagged_int with PyStackRef_IsTaggedInt --- Include/internal/pycore_stackref.h | 6 +++--- Python/stackrefs.c | 7 +------ 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 448a32d1cd3585..a2acf311ff4c65 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -136,7 +136,7 @@ _PyStackRef_FromPyObjectImmortal(PyObject *obj, const char *filename, int linenu #define PyStackRef_FromPyObjectImmortal(obj) _PyStackRef_FromPyObjectImmortal(_PyObject_CAST(obj), __FILE__, __LINE__) static inline bool -is_tagged_int(_PyStackRef ref) +PyStackRef_IsTaggedInt(_PyStackRef ref) { return (ref.index & 1) == 1; } @@ -144,7 +144,7 @@ is_tagged_int(_PyStackRef ref) static inline void _PyStackRef_CLOSE(_PyStackRef ref, const char *filename, int linenumber) { - if (is_tagged_int(ref)) { + if (PyStackRef_IsTaggedInt(ref)) { return; } PyObject *obj = _Py_stackref_close(ref, filename, linenumber); @@ -166,7 +166,7 @@ _PyStackRef_XCLOSE(_PyStackRef ref, const char *filename, int linenumber) static inline _PyStackRef _PyStackRef_DUP(_PyStackRef ref, const char *filename, int linenumber) { - if (ref.index & 1) { + if (PyStackRef_IsTaggedInt(ref)) { return ref; } else { diff --git a/Python/stackrefs.c b/Python/stackrefs.c index 1bd0288fce690e..979a6b1c62820a 100644 --- a/Python/stackrefs.c +++ b/Python/stackrefs.c @@ -205,16 +205,11 @@ _PyStackRef PyStackRef_TagInt(intptr_t i) intptr_t PyStackRef_UntagInt(_PyStackRef i) { - assert(is_tagged_int(i)); + assert(PyStackRef_IsTaggedInt(i)); intptr_t val = (intptr_t)i.index; return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, val, 1); } -bool PyStackRef_IsTaggedInt(_PyStackRef ref) -{ - return is_tagged_int(ref); -} - bool PyStackRef_IsNullOrInt(_PyStackRef ref) {