From 5188c1cc19d5ce9841a321d4aa5ccd3b5df3f8ae Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 10 Dec 2024 10:57:04 +0000 Subject: [PATCH 01/11] Make explicit that refcnt is 32 bits even on 64 bit machines --- Include/object.h | 14 ++++++++++++-- Include/refcount.h | 10 ++++++++-- Objects/object.c | 4 +++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Include/object.h b/Include/object.h index 3876d8449afbe2..c0d1f167b6dfba 100644 --- a/Include/object.h +++ b/Include/object.h @@ -120,9 +120,19 @@ struct _object { __pragma(warning(disable: 4201)) #endif union { - Py_ssize_t ob_refcnt; #if SIZEOF_VOID_P > 4 - PY_UINT32_T ob_refcnt_split[2]; + PY_INT64_T ob_refcnt_full; /* This field is needed for efficient initialization with Clang on ARM */ + struct { +# if PY_BIG_ENDIAN + PY_UINT32_T ob_flags; + PY_UINT32_T ob_refcnt; +# else + PY_UINT32_T ob_refcnt; + PY_UINT32_T ob_flags; +# endif + }; +#else + Py_ssize_t ob_refcnt; #endif }; #ifdef _MSC_VER diff --git a/Include/refcount.h b/Include/refcount.h index 141cbd34dd72e6..0da724707a045e 100644 --- a/Include/refcount.h +++ b/Include/refcount.h @@ -252,13 +252,13 @@ static inline Py_ALWAYS_INLINE void Py_INCREF(PyObject *op) _Py_atomic_add_ssize(&op->ob_ref_shared, (1 << _Py_REF_SHARED_SHIFT)); } #elif SIZEOF_VOID_P > 4 - PY_UINT32_T cur_refcnt = op->ob_refcnt_split[PY_BIG_ENDIAN]; + PY_UINT32_T cur_refcnt = op->ob_refcnt; if (((int32_t)cur_refcnt) < 0) { // the object is immortal _Py_INCREF_IMMORTAL_STAT_INC(); return; } - op->ob_refcnt_split[PY_BIG_ENDIAN] = cur_refcnt + 1; + op->ob_refcnt = cur_refcnt + 1; #else if (_Py_IsImmortal(op)) { _Py_INCREF_IMMORTAL_STAT_INC(); @@ -354,7 +354,13 @@ static inline void Py_DECREF(PyObject *op) #elif defined(Py_REF_DEBUG) static inline void Py_DECREF(const char *filename, int lineno, PyObject *op) { +#if SIZEOF_VOID_P > 4 + /* If an object has been freed, it will have a negative full refcnt + * If it has not it been freed, will have a very large refcnt */ + if (op->ob_refcnt_full <= 0 || op->ob_refcnt > (UINT32_MAX - (1<<20))) { +#else if (op->ob_refcnt <= 0) { +#endif _Py_NegativeRefcount(filename, lineno, op); } if (_Py_IsImmortal(op)) { diff --git a/Objects/object.c b/Objects/object.c index 74f47fa4239032..1856eb9421dc37 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2475,7 +2475,9 @@ new_reference(PyObject *op) { // Skip the immortal object check in Py_SET_REFCNT; always set refcnt to 1 #if !defined(Py_GIL_DISABLED) - op->ob_refcnt = 1; + op->ob_refcnt_full = 1; + assert(op->ob_refcnt == 1); + assert(op->ob_flags == 0); #else op->ob_tid = _Py_ThreadId(); op->_padding = 0; From ce2f515279d68191845d938a4be5bba303e05393 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 10 Dec 2024 15:31:32 +0000 Subject: [PATCH 02/11] Mark statically allocated objects --- Include/internal/pycore_object.h | 12 +++++++++++- Include/object.h | 2 +- Include/refcount.h | 17 ++++++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 6b0b464a6fdb96..92f09b80d5a099 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -73,14 +73,24 @@ PyAPI_FUNC(int) _PyObject_IsFreed(PyObject *); #define _PyObject_HEAD_INIT(type) \ { \ .ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL, \ + .ob_flags = _Py_STATICALLY_ALLOCATED_FLAG, \ .ob_type = (type) \ } #else +#if SIZEOF_VOID_P > 4 #define _PyObject_HEAD_INIT(type) \ { \ - .ob_refcnt = _Py_IMMORTAL_INITIAL_REFCNT, \ + .ob_refcnt = _Py_IMMORTAL_INITIAL_REFCNT, \ + .ob_flags = _Py_STATICALLY_ALLOCATED_FLAG, \ .ob_type = (type) \ } +#else +#define _PyObject_HEAD_INIT(type) \ + { \ + .ob_refcnt = _Py_STATIC_IMMORTAL_INITIAL_REFCNT, \ + .ob_type = (type) \ + } +#endif #endif #define _PyVarObject_HEAD_INIT(type, size) \ { \ diff --git a/Include/object.h b/Include/object.h index c0d1f167b6dfba..b2a40f9e6411da 100644 --- a/Include/object.h +++ b/Include/object.h @@ -152,7 +152,7 @@ struct _object { // trashcan mechanism as a linked list pointer and by the GC to store the // computed "gc_refs" refcount. uintptr_t ob_tid; - uint16_t _padding; + uint16_t ob_flags; PyMutex ob_mutex; // per-object lock uint8_t ob_gc_bits; // gc-related state uint32_t ob_ref_local; // local reference count diff --git a/Include/refcount.h b/Include/refcount.h index 0da724707a045e..afa5a9cdb851f4 100644 --- a/Include/refcount.h +++ b/Include/refcount.h @@ -54,10 +54,15 @@ immortality, but the execution would still be correct. Reference count increases and decreases will first go through an immortality check by comparing the reference count field to the minimum immortality refcount. */ -#define _Py_IMMORTAL_INITIAL_REFCNT ((Py_ssize_t)(3L << 29)) +#define _Py_IMMORTAL_INITIAL_REFCNT ((Py_ssize_t)(5L << 28)) #define _Py_IMMORTAL_MINIMUM_REFCNT ((Py_ssize_t)(1L << 30)) +#define _Py_STATIC_IMMORTAL_INITIAL_REFCNT ((Py_ssize_t)(7L << 28)) +#define _Py_STATIC_IMMORTAL_MINIMUM_REFCNT ((Py_ssize_t)(6L << 28)) #endif +/* Leave the low bits for refcount overflow for old stable ABI code */ +#define _Py_STATICALLY_ALLOCATED_FLAG (1 << 7) + // Py_GIL_DISABLED builds indicate immortal objects using `ob_ref_local`, which is // always 32-bits. #ifdef Py_GIL_DISABLED @@ -123,6 +128,16 @@ static inline Py_ALWAYS_INLINE int _Py_IsImmortal(PyObject *op) #define _Py_IsImmortal(op) _Py_IsImmortal(_PyObject_CAST(op)) +static inline Py_ALWAYS_INLINE int _Py_IsStaticImmortal(PyObject *op) +{ +#if defined(Py_GIL_DISABLED) || SIZEOF_VOID_P > 4 + return (op->ob_flags & _Py_STATICALLY_ALLOCATED_FLAG) != 0; +#else + return op->ob_refcnt >= _Py_STATIC_IMMORTAL_MINIMUM_REFCNT; +#endif +} +#define _Py_IsStaticImmortal(op) _Py_IsStaticImmortal(_PyObject_CAST(op)) + // Py_SET_REFCNT() implementation for stable ABI PyAPI_FUNC(void) _Py_SetRefcnt(PyObject *ob, Py_ssize_t refcnt); From 8d48e234e76bdde98b251e22671c7e20950c0b6a Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 10 Dec 2024 16:28:41 +0000 Subject: [PATCH 03/11] Update some initializers --- Objects/object.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Objects/object.c b/Objects/object.c index 1856eb9421dc37..7643a6c3de0989 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2475,12 +2475,16 @@ new_reference(PyObject *op) { // Skip the immortal object check in Py_SET_REFCNT; always set refcnt to 1 #if !defined(Py_GIL_DISABLED) +#if SIZEOF_VOID_P > 4 op->ob_refcnt_full = 1; assert(op->ob_refcnt == 1); assert(op->ob_flags == 0); +#else + op->ob_refcnt = 1; +#endif #else op->ob_tid = _Py_ThreadId(); - op->_padding = 0; + op->ob_flags = 0; op->ob_mutex = (PyMutex){ 0 }; op->ob_gc_bits = 0; op->ob_ref_local = 1; From 22f58e793c56559a5f2fc2dddd87f5ef1a2519c9 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 10 Dec 2024 16:45:44 +0000 Subject: [PATCH 04/11] Fix test on 32 bit machines --- Lib/test/test_builtin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index e51711d9b4f1a4..06df217881a52f 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -2691,7 +2691,7 @@ def __del__(self): class ImmortalTests(unittest.TestCase): if sys.maxsize < (1 << 32): - IMMORTAL_REFCOUNT = 3 << 29 + IMMORTAL_REFCOUNT = 7 << 28 else: IMMORTAL_REFCOUNT = 3 << 30 From 5d7e1c046fd91ecc9a6442c3899bed7e58ad7354 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 10 Dec 2024 16:57:05 +0000 Subject: [PATCH 05/11] Fix some compiler warnings --- Include/internal/pycore_object.h | 4 ++++ Include/refcount.h | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 92f09b80d5a099..22de3c9d4e32ea 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -137,7 +137,11 @@ static inline void _Py_RefcntAdd(PyObject* op, Py_ssize_t n) _Py_AddRefTotal(_PyThreadState_GET(), n); #endif #if !defined(Py_GIL_DISABLED) +#if SIZEOF_VOID_P > 4 + op->ob_refcnt += (PY_UINT32_T)n; +#else op->ob_refcnt += n; +#endif #else if (_Py_IsOwnedByCurrentThread(op)) { uint32_t local = op->ob_ref_local; diff --git a/Include/refcount.h b/Include/refcount.h index afa5a9cdb851f4..fd57d15d1e91ab 100644 --- a/Include/refcount.h +++ b/Include/refcount.h @@ -142,6 +142,7 @@ static inline Py_ALWAYS_INLINE int _Py_IsStaticImmortal(PyObject *op) PyAPI_FUNC(void) _Py_SetRefcnt(PyObject *ob, Py_ssize_t refcnt); static inline void Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt) { + assert(refcnt >= 0); #if defined(Py_LIMITED_API) && Py_LIMITED_API+0 >= 0x030d0000 // Stable ABI implements Py_SET_REFCNT() as a function call // on limited C API version 3.13 and newer. @@ -154,9 +155,12 @@ static inline void Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt) { if (_Py_IsImmortal(ob)) { return; } - #ifndef Py_GIL_DISABLED +#if SIZEOF_VOID_P > 4 + ob->ob_refcnt = (PY_UINT32_T)refcnt; +#else ob->ob_refcnt = refcnt; +#endif #else if (_Py_IsOwnedByCurrentThread(ob)) { if ((size_t)refcnt > (size_t)UINT32_MAX) { From 2fd9d3402830f5f0135d27888bf787ab8776f211 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 10 Dec 2024 18:00:43 +0000 Subject: [PATCH 06/11] Set flags properly for all objects --- Include/object.h | 2 +- Include/refcount.h | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/Include/object.h b/Include/object.h index b2a40f9e6411da..3d0b655d0d73d8 100644 --- a/Include/object.h +++ b/Include/object.h @@ -81,7 +81,7 @@ whose size is determined when the object is allocated. #else #define PyObject_HEAD_INIT(type) \ { \ - { _Py_IMMORTAL_INITIAL_REFCNT }, \ + { _Py_STATIC_IMMORTAL_INITIAL_REFCNT }, \ (type) \ }, #endif diff --git a/Include/refcount.h b/Include/refcount.h index fd57d15d1e91ab..6908c426141378 100644 --- a/Include/refcount.h +++ b/Include/refcount.h @@ -19,6 +19,9 @@ immortal. The latter should be the only instances that require cleanup during runtime finalization. */ +/* Leave the low bits for refcount overflow for old stable ABI code */ +#define _Py_STATICALLY_ALLOCATED_FLAG (1 << 7) + #if SIZEOF_VOID_P > 4 /* In 64+ bit systems, any object whose 32 bit reference count is >= 2**31 @@ -39,7 +42,8 @@ beyond the refcount limit. Immortality checks for reference count decreases will be done by checking the bit sign flag in the lower 32 bits. */ -#define _Py_IMMORTAL_INITIAL_REFCNT ((Py_ssize_t)(3UL << 30)) +#define _Py_IMMORTAL_INITIAL_REFCNT (3UL << 30) +#define _Py_STATIC_IMMORTAL_INITIAL_REFCNT ((Py_ssize_t)(_Py_IMMORTAL_INITIAL_REFCNT | (((Py_ssize_t)_Py_STATICALLY_ALLOCATED_FLAG) << 32))) #else /* @@ -60,9 +64,6 @@ check by comparing the reference count field to the minimum immortality refcount #define _Py_STATIC_IMMORTAL_MINIMUM_REFCNT ((Py_ssize_t)(6L << 28)) #endif -/* Leave the low bits for refcount overflow for old stable ABI code */ -#define _Py_STATICALLY_ALLOCATED_FLAG (1 << 7) - // Py_GIL_DISABLED builds indicate immortal objects using `ob_ref_local`, which is // always 32-bits. #ifdef Py_GIL_DISABLED From 3788c60c8d3f26fb3e2b5e03a8d26657fd0572b0 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 11 Dec 2024 10:06:32 +0000 Subject: [PATCH 07/11] Add test --- Lib/test/test_capi/test_immortal.py | 13 +++++++++++++ Modules/_testinternalcapi.c | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/Lib/test/test_capi/test_immortal.py b/Lib/test/test_capi/test_immortal.py index ef5d32b7f01935..bd5ae87b64d410 100644 --- a/Lib/test/test_capi/test_immortal.py +++ b/Lib/test/test_capi/test_immortal.py @@ -2,6 +2,7 @@ from test.support import import_helper _testcapi = import_helper.import_module('_testcapi') +_testinternalcapi = import_helper.import_module('_testinternalcapi') class TestCAPI(unittest.TestCase): @@ -11,6 +12,18 @@ def test_immortal_builtins(self): def test_immortal_small_ints(self): _testcapi.test_immortal_small_ints() +class TestInternalCAPI(unittest.TestCase): + + def test_immortal_builtins(self): + for obj in range(-5, 256): + self.assertTrue(_testinternalcapi.is_static_immortal(obj)) + for obj in (None, True, False, ..., NotImplemented, ()): + self.assertTrue(_testinternalcapi.is_static_immortal(obj)) + for obj in range(300, 400): + self.assertFalse(_testinternalcapi.is_static_immortal(obj)) + for obj in ([], {}, set()): + self.assertFalse(_testinternalcapi.is_static_immortal(obj)) + if __name__ == "__main__": unittest.main() diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 1bb71a3e80b39d..5b2174ee36c5e4 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -2082,6 +2082,15 @@ get_tracked_heap_size(PyObject *self, PyObject *Py_UNUSED(ignored)) return PyLong_FromInt64(PyInterpreterState_Get()->gc.heap_size); } +static PyObject * +is_static_immortal(PyObject *self, PyObject *op) +{ + if (_Py_IsStaticImmortal(op)) { + Py_RETURN_TRUE; + } + Py_RETURN_FALSE; +} + static PyMethodDef module_functions[] = { {"get_configs", get_configs, METH_NOARGS}, {"get_recursion_depth", get_recursion_depth, METH_NOARGS}, @@ -2180,6 +2189,7 @@ static PyMethodDef module_functions[] = { {"identify_type_slot_wrappers", identify_type_slot_wrappers, METH_NOARGS}, {"has_deferred_refcount", has_deferred_refcount, METH_O}, {"get_tracked_heap_size", get_tracked_heap_size, METH_NOARGS}, + {"is_static_immortal", is_static_immortal, METH_O}, {NULL, NULL} /* sentinel */ }; From dea0fa8773c9403b913cd252efa0e04073991e2b Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 11 Dec 2024 11:04:29 +0000 Subject: [PATCH 08/11] Set immortal flag for FT build --- Include/object.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/object.h b/Include/object.h index 3d0b655d0d73d8..da7b3668c033f4 100644 --- a/Include/object.h +++ b/Include/object.h @@ -71,7 +71,7 @@ whose size is determined when the object is allocated. #define PyObject_HEAD_INIT(type) \ { \ 0, \ - 0, \ + _Py_STATICALLY_ALLOCATED_FLAG, \ { 0 }, \ 0, \ _Py_IMMORTAL_REFCNT_LOCAL, \ From 7cb320b21aaaebcbeb1053703ff35e4558cc9264 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 11 Dec 2024 11:11:11 +0000 Subject: [PATCH 09/11] Unroll loop to see which test is failing --- Lib/test/test_capi/test_immortal.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_capi/test_immortal.py b/Lib/test/test_capi/test_immortal.py index bd5ae87b64d410..c11dcc956658cf 100644 --- a/Lib/test/test_capi/test_immortal.py +++ b/Lib/test/test_capi/test_immortal.py @@ -17,8 +17,11 @@ class TestInternalCAPI(unittest.TestCase): def test_immortal_builtins(self): for obj in range(-5, 256): self.assertTrue(_testinternalcapi.is_static_immortal(obj)) - for obj in (None, True, False, ..., NotImplemented, ()): - self.assertTrue(_testinternalcapi.is_static_immortal(obj)) + self.assertTrue(_testinternalcapi.is_static_immortal(None) + self.assertTrue(_testinternalcapi.is_static_immortal(False) + self.assertTrue(_testinternalcapi.is_static_immortal(True) + self.assertTrue(_testinternalcapi.is_static_immortal(...) + self.assertTrue(_testinternalcapi.is_static_immortal(()) for obj in range(300, 400): self.assertFalse(_testinternalcapi.is_static_immortal(obj)) for obj in ([], {}, set()): From 7aefe5765aae8e28d489d905a9de1cea02ac0e87 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 11 Dec 2024 11:13:28 +0000 Subject: [PATCH 10/11] Fix copy and paste errors --- Lib/test/test_capi/test_immortal.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_capi/test_immortal.py b/Lib/test/test_capi/test_immortal.py index c11dcc956658cf..3e36913ac301c3 100644 --- a/Lib/test/test_capi/test_immortal.py +++ b/Lib/test/test_capi/test_immortal.py @@ -17,11 +17,11 @@ class TestInternalCAPI(unittest.TestCase): def test_immortal_builtins(self): for obj in range(-5, 256): self.assertTrue(_testinternalcapi.is_static_immortal(obj)) - self.assertTrue(_testinternalcapi.is_static_immortal(None) - self.assertTrue(_testinternalcapi.is_static_immortal(False) - self.assertTrue(_testinternalcapi.is_static_immortal(True) - self.assertTrue(_testinternalcapi.is_static_immortal(...) - self.assertTrue(_testinternalcapi.is_static_immortal(()) + self.assertTrue(_testinternalcapi.is_static_immortal(None)) + self.assertTrue(_testinternalcapi.is_static_immortal(False)) + self.assertTrue(_testinternalcapi.is_static_immortal(True)) + self.assertTrue(_testinternalcapi.is_static_immortal(...)) + self.assertTrue(_testinternalcapi.is_static_immortal(())) for obj in range(300, 400): self.assertFalse(_testinternalcapi.is_static_immortal(obj)) for obj in ([], {}, set()): From ddff59692c3af17c3a52d207a9d62fbae87da08d Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 11 Dec 2024 11:38:27 +0000 Subject: [PATCH 11/11] Down downgrade from static immortal to plain immortal --- Objects/object.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Objects/object.c b/Objects/object.c index 7643a6c3de0989..c64675b5e1d6c2 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2521,6 +2521,10 @@ _Py_SetImmortalUntracked(PyObject *op) || PyUnicode_CHECK_INTERNED(op) == SSTATE_INTERNED_IMMORTAL_STATIC); } #endif + // Check if already immortal to avoid degrading from static immortal to plain immortal + if (_Py_IsImmortal(op)) { + return; + } #ifdef Py_GIL_DISABLED op->ob_tid = _Py_UNOWNED_TID; op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;