From e0ea1b1be7495417fc0872d1665c2d4689ea42d1 Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Wed, 1 Oct 2025 16:38:58 +0300 Subject: [PATCH 1/4] Fixes for PyStackRefs in debug builds. --- Include/internal/pycore_stackref.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 062834368bcd29..5334d2be7c164b 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -72,6 +72,19 @@ static const _PyStackRef PyStackRef_ERROR = { .index = 2 }; #define INITIAL_STACKREF_INDEX 10 +static inline _PyStackRef +PyStackRef_Wrap(void *ptr) +{ + assert(ptr != NULL); + return (_PyStackRef){ .index = (uint64_t)ptr }; +} + +static inline void * +PyStackRef_Unwrap(_PyStackRef ref) +{ + return (void *)(ref.index); +} + static inline int PyStackRef_IsNull(_PyStackRef ref) { @@ -151,6 +164,7 @@ _PyStackRef_FromPyObjectSteal(PyObject *obj, const char *filename, int linenumbe static inline _PyStackRef _PyStackRef_FromPyObjectBorrow(PyObject *obj, const char *filename, int linenumber) { + Py_INCREF(obj); return _Py_stackref_create(obj, filename, linenumber); } #define PyStackRef_FromPyObjectBorrow(obj) _PyStackRef_FromPyObjectBorrow(_PyObject_CAST(obj), __FILE__, __LINE__) From 56a2076962a27074ef78141214fa1eff42c83c5a Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Fri, 3 Oct 2025 17:52:10 +0300 Subject: [PATCH 2/4] The same pattern of refcounting for stackrefs as in production build --- Include/internal/pycore_stackref.h | 141 +++++++++++++----- ...-10-03-17-51-43.gh-issue-139475._684ED.rst | 2 + Python/stackrefs.c | 22 ++- 3 files changed, 115 insertions(+), 50 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-10-03-17-51-43.gh-issue-139475._684ED.rst diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 5334d2be7c164b..996fa82c74bdc3 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -53,36 +53,53 @@ extern "C" { #if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG) -#define Py_TAG_BITS 0 +#define Py_INT_TAG 3 +#define Py_TAG_INVALID 2 +#define Py_TAG_REFCNT 1 +#define Py_TAG_BITS 3 PyAPI_FUNC(PyObject *) _Py_stackref_get_object(_PyStackRef ref); PyAPI_FUNC(PyObject *) _Py_stackref_close(_PyStackRef ref, const char *filename, int linenumber); -PyAPI_FUNC(_PyStackRef) _Py_stackref_create(PyObject *obj, const char *filename, int linenumber); +PyAPI_FUNC(_PyStackRef) _Py_stackref_create(PyObject *obj, uint16_t flags, const char *filename, int linenumber); PyAPI_FUNC(void) _Py_stackref_record_borrow(_PyStackRef ref, const char *filename, int linenumber); extern void _Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _PyStackRef ref); static const _PyStackRef PyStackRef_NULL = { .index = 0 }; -static const _PyStackRef PyStackRef_ERROR = { .index = 2 }; +static const _PyStackRef PyStackRef_ERROR = { .index = 4 }; -// Use the first 3 even numbers for None, True and False. -// Odd numbers are reserved for (tagged) integers -#define PyStackRef_None ((_PyStackRef){ .index = 4 } ) -#define PyStackRef_False ((_PyStackRef){ .index = 6 }) -#define PyStackRef_True ((_PyStackRef){ .index = 8 }) +#define PyStackRef_None ((_PyStackRef){ .index = 8 } ) +#define PyStackRef_False ((_PyStackRef){ .index = 12 }) +#define PyStackRef_True ((_PyStackRef){ .index = 16 }) -#define INITIAL_STACKREF_INDEX 10 +#define INITIAL_STACKREF_INDEX 20 static inline _PyStackRef PyStackRef_Wrap(void *ptr) { assert(ptr != NULL); +#ifdef Py_DEBUG + assert(((uint64_t)ptr & Py_TAG_BITS) == 0); + return (_PyStackRef){ .index = ((uint64_t)ptr) | Py_TAG_INVALID }; +#else return (_PyStackRef){ .index = (uint64_t)ptr }; +#endif } static inline void * PyStackRef_Unwrap(_PyStackRef ref) { +#ifdef Py_DEBUG + assert ((ref.index & Py_TAG_BITS) == Py_TAG_INVALID); + return (void *)(ref.index & ~Py_TAG_BITS); +#else return (void *)(ref.index); +#endif +} + +static inline int +PyStackRef_RefcountOnObject(_PyStackRef ref) +{ + return (ref.index & Py_TAG_REFCNT) == 0; } static inline int @@ -94,7 +111,7 @@ PyStackRef_IsNull(_PyStackRef ref) static inline bool PyStackRef_IsError(_PyStackRef ref) { - return ref.index == 2; + return ref.index == 4; } static inline bool @@ -125,7 +142,7 @@ PyStackRef_IsNone(_PyStackRef ref) static inline bool PyStackRef_IsTaggedInt(_PyStackRef ref) { - return (ref.index & 1) == 1; + return (ref.index & Py_TAG_BITS) == Py_INT_TAG; } static inline PyObject * @@ -136,51 +153,68 @@ _PyStackRef_AsPyObjectBorrow(_PyStackRef ref, const char *filename, int linenumb _Py_stackref_record_borrow(ref, filename, linenumber); return _Py_stackref_get_object(ref); } - #define PyStackRef_AsPyObjectBorrow(REF) _PyStackRef_AsPyObjectBorrow((REF), __FILE__, __LINE__) static inline PyObject * _PyStackRef_AsPyObjectSteal(_PyStackRef ref, const char *filename, int linenumber) { - return _Py_stackref_close(ref, filename, linenumber); + PyObject *obj = _Py_stackref_close(ref, filename, linenumber); + if (PyStackRef_RefcountOnObject(ref)) { + return obj; + } + return Py_NewRef(obj); } #define PyStackRef_AsPyObjectSteal(REF) _PyStackRef_AsPyObjectSteal((REF), __FILE__, __LINE__) static inline _PyStackRef _PyStackRef_FromPyObjectNew(PyObject *obj, const char *filename, int linenumber) { - Py_INCREF(obj); - return _Py_stackref_create(obj, filename, linenumber); + assert(obj != NULL); + uint16_t flags = 0; + if (!_Py_IsImmortal(obj)) { + _Py_INCREF_MORTAL(obj); + } else { + flags = Py_TAG_REFCNT; + } + return _Py_stackref_create(obj, flags, filename, linenumber); } #define PyStackRef_FromPyObjectNew(obj) _PyStackRef_FromPyObjectNew(_PyObject_CAST(obj), __FILE__, __LINE__) static inline _PyStackRef _PyStackRef_FromPyObjectSteal(PyObject *obj, const char *filename, int linenumber) { - return _Py_stackref_create(obj, filename, linenumber); + assert(obj != NULL); + uint16_t flags = 0; + if (_Py_IsImmortal(obj)) { + flags = Py_TAG_REFCNT; + } + return _Py_stackref_create(obj, flags, filename, linenumber); } #define PyStackRef_FromPyObjectSteal(obj) _PyStackRef_FromPyObjectSteal(_PyObject_CAST(obj), __FILE__, __LINE__) static inline _PyStackRef _PyStackRef_FromPyObjectBorrow(PyObject *obj, const char *filename, int linenumber) { - Py_INCREF(obj); - return _Py_stackref_create(obj, filename, linenumber); + return _Py_stackref_create(obj, Py_TAG_REFCNT, filename, linenumber); } #define PyStackRef_FromPyObjectBorrow(obj) _PyStackRef_FromPyObjectBorrow(_PyObject_CAST(obj), __FILE__, __LINE__) static inline void _PyStackRef_CLOSE(_PyStackRef ref, const char *filename, int linenumber) { + assert(!PyStackRef_IsError(ref)); + assert(!PyStackRef_IsNull(ref)); if (PyStackRef_IsTaggedInt(ref)) { return; } PyObject *obj = _Py_stackref_close(ref, filename, linenumber); - Py_DECREF(obj); + assert(Py_REFCNT(obj) > 0); + if (PyStackRef_RefcountOnObject(ref)) { + Py_DECREF(obj); + } } #define PyStackRef_CLOSE(REF) _PyStackRef_CLOSE((REF), __FILE__, __LINE__) - static inline void _PyStackRef_XCLOSE(_PyStackRef ref, const char *filename, int linenumber) { @@ -196,31 +230,46 @@ static inline _PyStackRef _PyStackRef_DUP(_PyStackRef ref, const char *filename, int linenumber) { assert(!PyStackRef_IsError(ref)); + assert(!PyStackRef_IsNull(ref)); if (PyStackRef_IsTaggedInt(ref)) { return ref; } - else { - PyObject *obj = _Py_stackref_get_object(ref); + PyObject *obj = _Py_stackref_get_object(ref); + uint16_t flags = 0; + if (PyStackRef_RefcountOnObject(ref)) { Py_INCREF(obj); - return _Py_stackref_create(obj, filename, linenumber); + } else { + flags = Py_TAG_REFCNT; } + return _Py_stackref_create(obj, flags, filename, linenumber); } #define PyStackRef_DUP(REF) _PyStackRef_DUP(REF, __FILE__, __LINE__) -extern void _PyStackRef_CLOSE_SPECIALIZED(_PyStackRef ref, destructor destruct, const char *filename, int linenumber); -#define PyStackRef_CLOSE_SPECIALIZED(REF, DESTRUCT) _PyStackRef_CLOSE_SPECIALIZED(REF, DESTRUCT, __FILE__, __LINE__) - -static inline _PyStackRef -PyStackRef_MakeHeapSafe(_PyStackRef ref) +static inline void +_PyStackRef_CLOSE_SPECIALIZED(_PyStackRef ref, destructor destruct, const char *filename, int linenumber) { - return ref; + assert(!PyStackRef_IsError(ref)); + assert(!PyStackRef_IsNull(ref)); + assert(!PyStackRef_IsTaggedInt(ref)); + PyObject *obj = _Py_stackref_close(ref, filename, linenumber); + if (PyStackRef_RefcountOnObject(ref)) { + _Py_DECREF_SPECIALIZED(obj, destruct); + } } +#define PyStackRef_CLOSE_SPECIALIZED(REF, DESTRUCT) _PyStackRef_CLOSE_SPECIALIZED(REF, DESTRUCT, __FILE__, __LINE__) static inline _PyStackRef -PyStackRef_Borrow(_PyStackRef ref) +_PyStackRef_Borrow(_PyStackRef ref, const char *filename, int linenumber) { - return PyStackRef_DUP(ref); + assert(!PyStackRef_IsError(ref)); + assert(!PyStackRef_IsNull(ref)); + if (PyStackRef_IsTaggedInt(ref)) { + return ref; + } + PyObject *obj = _Py_stackref_get_object(ref); + return _Py_stackref_create(obj, Py_TAG_REFCNT, filename, linenumber); } +#define PyStackRef_Borrow(REF) _PyStackRef_Borrow((REF), __FILE__, __LINE__) #define PyStackRef_CLEAR(REF) \ do { \ @@ -234,27 +283,44 @@ static inline _PyStackRef _PyStackRef_FromPyObjectStealMortal(PyObject *obj, const char *filename, int linenumber) { assert(!_Py_IsImmortal(obj)); - return _Py_stackref_create(obj, filename, linenumber); + return _Py_stackref_create(obj, 0, filename, linenumber); } #define PyStackRef_FromPyObjectStealMortal(obj) _PyStackRef_FromPyObjectStealMortal(_PyObject_CAST(obj), __FILE__, __LINE__) static inline bool PyStackRef_IsHeapSafe(_PyStackRef ref) { - return true; + if ((ref.index & Py_TAG_BITS) != Py_TAG_REFCNT || PyStackRef_IsNull(ref)) { + // Tagged ints and ERROR are included. + return true; + } + + PyObject *obj = _Py_stackref_get_object(ref); + return _Py_IsImmortal(obj); } +static inline _PyStackRef +_PyStackRef_MakeHeapSafe(_PyStackRef ref, const char *filename, int linenumber) +{ + if (PyStackRef_IsHeapSafe(ref)) { + return ref; + } + + PyObject *obj = _Py_stackref_close(ref, filename, linenumber); + Py_INCREF(obj); + return _Py_stackref_create(obj, 0, filename, linenumber); +} +#define PyStackRef_MakeHeapSafe(REF) _PyStackRef_MakeHeapSafe(REF, __FILE__, __LINE__) + static inline _PyStackRef _PyStackRef_FromPyObjectNewMortal(PyObject *obj, const char *filename, int linenumber) { assert(!_Py_IsStaticImmortal(obj)); - Py_INCREF(obj); - return _Py_stackref_create(obj, filename, linenumber); + Py_XINCREF(obj); + return _Py_stackref_create(obj, 0, filename, linenumber); } #define PyStackRef_FromPyObjectNewMortal(obj) _PyStackRef_FromPyObjectNewMortal(_PyObject_CAST(obj), __FILE__, __LINE__) -#define PyStackRef_RefcountOnObject(REF) 1 - extern int PyStackRef_Is(_PyStackRef a, _PyStackRef b); extern bool PyStackRef_IsTaggedInt(_PyStackRef ref); @@ -287,6 +353,7 @@ PyStackRef_Wrap(void *ptr) { assert(ptr != NULL); #ifdef Py_DEBUG + assert(((uintptr_t)ptr & Py_TAG_BITS) == 0); return (_PyStackRef){ .bits = ((uintptr_t)ptr) | Py_TAG_INVALID }; #else return (_PyStackRef){ .bits = (uintptr_t)ptr }; diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-10-03-17-51-43.gh-issue-139475._684ED.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-10-03-17-51-43.gh-issue-139475._684ED.rst new file mode 100644 index 00000000000000..f4d50b7d0207a0 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-10-03-17-51-43.gh-issue-139475._684ED.rst @@ -0,0 +1,2 @@ +Changes in stackref debugging mode when ``Py_STACKREF_DEBUG`` is set. We use +the same pattern of refcounting for stackrefs as in production build. diff --git a/Python/stackrefs.c b/Python/stackrefs.c index ecc0012ef17b39..9a6e497d88e695 100644 --- a/Python/stackrefs.c +++ b/Python/stackrefs.c @@ -109,18 +109,19 @@ _Py_stackref_close(_PyStackRef ref, const char *filename, int linenumber) } _PyStackRef -_Py_stackref_create(PyObject *obj, const char *filename, int linenumber) +_Py_stackref_create(PyObject *obj, uint16_t flags, const char *filename, int linenumber) { if (obj == NULL) { Py_FatalError("Cannot create a stackref for NULL"); } PyInterpreterState *interp = PyInterpreterState_Get(); uint64_t new_id = interp->next_stackref; - interp->next_stackref = new_id + 2; + interp->next_stackref = new_id + 4; TableEntry *entry = make_table_entry(obj, filename, linenumber); if (entry == NULL) { Py_FatalError("No memory left for stackref debug table"); } + new_id |= flags; if (_Py_hashtable_set(interp->open_stackrefs_table, (void *)new_id, entry) < 0) { Py_FatalError("No memory left for stackref debug table"); } @@ -194,16 +195,10 @@ _Py_stackref_report_leaks(PyInterpreterState *interp) } } -void -_PyStackRef_CLOSE_SPECIALIZED(_PyStackRef ref, destructor destruct, const char *filename, int linenumber) -{ - PyObject *obj = _Py_stackref_close(ref, filename, linenumber); - _Py_DECREF_SPECIALIZED(obj, destruct); -} - _PyStackRef PyStackRef_TagInt(intptr_t i) { - return (_PyStackRef){ .index = (i << 1) + 1 }; + assert(Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, (i << 2), 2) == i); + return (_PyStackRef){ .index = (i << 2) | Py_INT_TAG }; } intptr_t @@ -211,7 +206,7 @@ PyStackRef_UntagInt(_PyStackRef i) { assert(PyStackRef_IsTaggedInt(i)); intptr_t val = (intptr_t)i.index; - return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, val, 1); + return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, val, 2); } bool @@ -223,8 +218,9 @@ PyStackRef_IsNullOrInt(_PyStackRef ref) _PyStackRef PyStackRef_IncrementTaggedIntNoOverflow(_PyStackRef ref) { - assert(ref.index <= INT_MAX - 2); // No overflow - return (_PyStackRef){ .index = ref.index + 2 }; + assert(PyStackRef_IsTaggedInt(ref)); + assert((ref.index & (~Py_TAG_BITS)) != (INT_MAX & (~Py_TAG_BITS))); // Isn't about to overflow + return (_PyStackRef){ .index = ref.index + 4 }; } From a2754b7cfe34ee13b1c12342e56b5e102989ab90 Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Wed, 22 Oct 2025 19:48:07 +0300 Subject: [PATCH 3/4] Adoption after merge main, review addressed --- Include/internal/pycore_stackref.h | 41 ++++++++++++++++-------------- Python/stackrefs.c | 12 ++++----- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/Include/internal/pycore_stackref.h b/Include/internal/pycore_stackref.h index 5a56773717e4b0..5adfa2d18a55c5 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -50,14 +50,15 @@ extern "C" { CPython refcounting operations on it! */ - -#if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG) - #define Py_INT_TAG 3 #define Py_TAG_INVALID 2 #define Py_TAG_REFCNT 1 #define Py_TAG_BITS 3 +#define Py_TAGGED_SHIFT 2 + +#if !defined(Py_GIL_DISABLED) && defined(Py_STACKREF_DEBUG) + PyAPI_FUNC(PyObject *) _Py_stackref_get_object(_PyStackRef ref); PyAPI_FUNC(PyObject *) _Py_stackref_close(_PyStackRef ref, const char *filename, int linenumber); PyAPI_FUNC(_PyStackRef) _Py_stackref_create(PyObject *obj, uint16_t flags, const char *filename, int linenumber); @@ -65,13 +66,13 @@ PyAPI_FUNC(void) _Py_stackref_record_borrow(_PyStackRef ref, const char *filenam extern void _Py_stackref_associate(PyInterpreterState *interp, PyObject *obj, _PyStackRef ref); static const _PyStackRef PyStackRef_NULL = { .index = 0 }; -static const _PyStackRef PyStackRef_ERROR = { .index = 4 }; +static const _PyStackRef PyStackRef_ERROR = { .index = (1 << Py_TAGGED_SHIFT) }; -#define PyStackRef_None ((_PyStackRef){ .index = 8 } ) -#define PyStackRef_False ((_PyStackRef){ .index = 12 }) -#define PyStackRef_True ((_PyStackRef){ .index = 16 }) +#define PyStackRef_None ((_PyStackRef){ .index = (2 << Py_TAGGED_SHIFT) } ) +#define PyStackRef_False ((_PyStackRef){ .index = (3 << Py_TAGGED_SHIFT) }) +#define PyStackRef_True ((_PyStackRef){ .index = (4 << Py_TAGGED_SHIFT) }) -#define INITIAL_STACKREF_INDEX 20 +#define INITIAL_STACKREF_INDEX (5 << Py_TAGGED_SHIFT) static inline _PyStackRef PyStackRef_Wrap(void *ptr) @@ -111,7 +112,13 @@ PyStackRef_IsNull(_PyStackRef ref) static inline bool PyStackRef_IsError(_PyStackRef ref) { - return ref.index == 4; + return ref.index == (1 << Py_TAGGED_SHIFT); +} + +static inline bool +PyStackRef_IsMalformed(_PyStackRef ref) +{ + return (ref.index & Py_TAG_BITS) == Py_TAG_INVALID; } static inline bool @@ -315,6 +322,7 @@ _PyStackRef_MakeHeapSafe(_PyStackRef ref, const char *filename, int linenumber) static inline _PyStackRef _PyStackRef_FromPyObjectNewMortal(PyObject *obj, const char *filename, int linenumber) { + assert(obj != NULL); assert(!_Py_IsStaticImmortal(obj)); Py_XINCREF(obj); return _Py_stackref_create(obj, 0, filename, linenumber); @@ -337,11 +345,6 @@ PyStackRef_IsNullOrInt(_PyStackRef ref); #else -#define Py_INT_TAG 3 -#define Py_TAG_INVALID 2 -#define Py_TAG_REFCNT 1 -#define Py_TAG_BITS 3 - static const _PyStackRef PyStackRef_ERROR = { .bits = Py_TAG_INVALID }; /* Wrap a pointer in a stack ref. @@ -399,8 +402,8 @@ PyStackRef_IsTaggedInt(_PyStackRef i) 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) }; + assert(Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, (i << Py_TAGGED_SHIFT), Py_TAGGED_SHIFT) == i); + return (_PyStackRef){ .bits = ((((uintptr_t)i) << Py_TAGGED_SHIFT) | Py_INT_TAG) }; } static inline intptr_t @@ -408,7 +411,7 @@ PyStackRef_UntagInt(_PyStackRef i) { assert(PyStackRef_IsTaggedInt(i)); intptr_t val = (intptr_t)i.bits; - return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, val, 2); + return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, val, Py_TAGGED_SHIFT); } @@ -416,8 +419,8 @@ static inline _PyStackRef PyStackRef_IncrementTaggedIntNoOverflow(_PyStackRef ref) { assert((ref.bits & Py_TAG_BITS) == Py_INT_TAG); // Is tagged int - assert((ref.bits & (~Py_TAG_BITS)) != (INT_MAX & (~Py_TAG_BITS))); // Isn't about to overflow - return (_PyStackRef){ .bits = ref.bits + 4 }; + assert((ref.bits & (~Py_TAG_BITS)) != (INTPTR_MAX & (~Py_TAG_BITS))); // Isn't about to overflow + return (_PyStackRef){ .bits = ref.bits + (1 << Py_TAGGED_SHIFT) }; } #define PyStackRef_IsDeferredOrTaggedInt(ref) (((ref).bits & Py_TAG_REFCNT) != 0) diff --git a/Python/stackrefs.c b/Python/stackrefs.c index 9a6e497d88e695..720916e0854f5c 100644 --- a/Python/stackrefs.c +++ b/Python/stackrefs.c @@ -116,7 +116,7 @@ _Py_stackref_create(PyObject *obj, uint16_t flags, const char *filename, int lin } PyInterpreterState *interp = PyInterpreterState_Get(); uint64_t new_id = interp->next_stackref; - interp->next_stackref = new_id + 4; + interp->next_stackref = new_id + (1 << Py_TAGGED_SHIFT); TableEntry *entry = make_table_entry(obj, filename, linenumber); if (entry == NULL) { Py_FatalError("No memory left for stackref debug table"); @@ -197,8 +197,8 @@ _Py_stackref_report_leaks(PyInterpreterState *interp) _PyStackRef PyStackRef_TagInt(intptr_t i) { - assert(Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, (i << 2), 2) == i); - return (_PyStackRef){ .index = (i << 2) | Py_INT_TAG }; + assert(Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, (i << Py_TAGGED_SHIFT), Py_TAGGED_SHIFT) == i); + return (_PyStackRef){ .index = (i << Py_TAGGED_SHIFT) | Py_INT_TAG }; } intptr_t @@ -206,7 +206,7 @@ PyStackRef_UntagInt(_PyStackRef i) { assert(PyStackRef_IsTaggedInt(i)); intptr_t val = (intptr_t)i.index; - return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, val, 2); + return Py_ARITHMETIC_RIGHT_SHIFT(intptr_t, val, Py_TAGGED_SHIFT); } bool @@ -219,8 +219,8 @@ _PyStackRef PyStackRef_IncrementTaggedIntNoOverflow(_PyStackRef ref) { assert(PyStackRef_IsTaggedInt(ref)); - assert((ref.index & (~Py_TAG_BITS)) != (INT_MAX & (~Py_TAG_BITS))); // Isn't about to overflow - return (_PyStackRef){ .index = ref.index + 4 }; + assert((ref.index & (~Py_TAG_BITS)) != (INTPTR_MAX & (~Py_TAG_BITS))); // Isn't about to overflow + return (_PyStackRef){ .index = ref.index + (1 << Py_TAGGED_SHIFT) }; } From 14b89e5bc8e6b8a8c1ff8b919c366b7d0a54bc28 Mon Sep 17 00:00:00 2001 From: Mikhail Efimov Date: Thu, 23 Oct 2025 10:14:30 +0300 Subject: [PATCH 4/4] Address review --- 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 5adfa2d18a55c5..94fcb1d8aee52b 100644 --- a/Include/internal/pycore_stackref.h +++ b/Include/internal/pycore_stackref.h @@ -289,6 +289,7 @@ _PyStackRef_Borrow(_PyStackRef ref, const char *filename, int linenumber) static inline _PyStackRef _PyStackRef_FromPyObjectStealMortal(PyObject *obj, const char *filename, int linenumber) { + assert(obj != NULL); assert(!_Py_IsImmortal(obj)); return _Py_stackref_create(obj, 0, filename, linenumber); } @@ -324,7 +325,7 @@ _PyStackRef_FromPyObjectNewMortal(PyObject *obj, const char *filename, int linen { assert(obj != NULL); assert(!_Py_IsStaticImmortal(obj)); - Py_XINCREF(obj); + Py_INCREF(obj); return _Py_stackref_create(obj, 0, filename, linenumber); } #define PyStackRef_FromPyObjectNewMortal(obj) _PyStackRef_FromPyObjectNewMortal(_PyObject_CAST(obj), __FILE__, __LINE__)