From be3017fba7e78d4eb904fbcb411a4c9257d12934 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 16 Nov 2023 19:12:04 +0100 Subject: [PATCH 1/6] gh-110481: Fix Py_SET_REFCNT() integer overflow If Py_NOGIL is defined and Py_SET_REFCNT() is called with a reference count larger than UINT32_MAX, make the object immortal. Set _Py_IMMORTAL_REFCNT constant type to Py_ssize_t to fix the following compiler warning: Include/internal/pycore_global_objects_fini_generated.h:14:24: warning: comparison of integers of different signs: 'Py_ssize_t' (aka 'long') and 'unsigned int' [-Wsign-compare] if (Py_REFCNT(obj) < _Py_IMMORTAL_REFCNT) { ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~ --- Include/object.h | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/Include/object.h b/Include/object.h index 6b70a494844476..4145b5e63017f3 100644 --- a/Include/object.h +++ b/Include/object.h @@ -88,7 +88,7 @@ having all the lower 32 bits set, which will avoid the reference count to go 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_REFCNT UINT_MAX +#define _Py_IMMORTAL_REFCNT (Py_ssize_t)UINT_MAX #else /* @@ -103,7 +103,7 @@ 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 immortality reference count. */ -#define _Py_IMMORTAL_REFCNT (UINT_MAX >> 2) +#define _Py_IMMORTAL_REFCNT (Py_ssize_t)(UINT_MAX >> 2) #endif // Py_GIL_DISABLED builds indicate immortal objects using `ob_ref_local`, which is @@ -317,11 +317,11 @@ static inline Py_ssize_t Py_SIZE(PyObject *ob) { static inline Py_ALWAYS_INLINE int _Py_IsImmortal(PyObject *op) { #if defined(Py_GIL_DISABLED) - return op->ob_ref_local == _Py_IMMORTAL_REFCNT_LOCAL; + return (op->ob_ref_local == _Py_IMMORTAL_REFCNT_LOCAL); #elif SIZEOF_VOID_P > 4 - return _Py_CAST(PY_INT32_T, op->ob_refcnt) < 0; + return (_Py_CAST(PY_INT32_T, op->ob_refcnt) < 0); #else - return op->ob_refcnt == _Py_IMMORTAL_REFCNT; + return (op->ob_refcnt == _Py_IMMORTAL_REFCNT); #endif } #define _Py_IsImmortal(op) _Py_IsImmortal(_PyObject_CAST(op)) @@ -350,15 +350,23 @@ static inline void Py_SET_REFCNT(PyObject *ob, Py_ssize_t refcnt) { if (_Py_IsImmortal(ob)) { return; } + #ifndef Py_GIL_DISABLED ob->ob_refcnt = refcnt; #else if (_Py_IsOwnedByCurrentThread(ob)) { - // Set local refcount to desired refcount and shared refcount to zero, - // but preserve the shared refcount flags. - assert(refcnt < UINT32_MAX); - ob->ob_ref_local = _Py_STATIC_CAST(uint32_t, refcnt); - ob->ob_ref_shared &= _Py_REF_SHARED_FLAG_MASK; + if ((size_t)refcnt > (size_t)UINT32_MAX) { + // On overflow, make the object immortal + op->ob_tid = _Py_UNOWNED_TID; + op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL; + op->ob_ref_shared = 0; + } + else { + // Set local refcount to desired refcount and shared refcount + // to zero, but preserve the shared refcount flags. + ob->ob_ref_local = _Py_STATIC_CAST(uint32_t, refcnt); + ob->ob_ref_shared &= _Py_REF_SHARED_FLAG_MASK; + } } else { // Set local refcount to zero and shared refcount to desired refcount. From 3a53d9b6056b2ab5b4792d379f749ec4264b1e42 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 16 Nov 2023 21:35:49 +0100 Subject: [PATCH 2/6] Use _Py_CAST() --- Include/object.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/object.h b/Include/object.h index 4145b5e63017f3..cd600a2f2a24d8 100644 --- a/Include/object.h +++ b/Include/object.h @@ -88,7 +88,7 @@ having all the lower 32 bits set, which will avoid the reference count to go 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_REFCNT (Py_ssize_t)UINT_MAX +#define _Py_IMMORTAL_REFCNT _Py_CAST(Py_ssize_t, UINT_MAX) #else /* @@ -103,7 +103,7 @@ 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 immortality reference count. */ -#define _Py_IMMORTAL_REFCNT (Py_ssize_t)(UINT_MAX >> 2) +#define _Py_IMMORTAL_REFCNT _Py_CAST(Py_ssize_t, UINT_MAX >> 2) #endif // Py_GIL_DISABLED builds indicate immortal objects using `ob_ref_local`, which is From 53df2c16602d36631e5483f3c9ba33e567f11382 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 17 Nov 2023 15:12:44 +0100 Subject: [PATCH 3/6] Document the behavior --- Doc/c-api/refcounting.rst | 3 +++ Doc/using/configure.rst | 2 ++ 2 files changed, 5 insertions(+) diff --git a/Doc/c-api/refcounting.rst b/Doc/c-api/refcounting.rst index 68119a27b18ec2..a6a5154c703d04 100644 --- a/Doc/c-api/refcounting.rst +++ b/Doc/c-api/refcounting.rst @@ -34,6 +34,9 @@ of Python objects. Set the object *o* reference counter to *refcnt*. + On ref:`Python build with Free Threading `, if *refcnt* + is larger than ``UINT32_MAX``, the object is made :term:`immortal`. + This function has no effect on :term:`immortal` objects. .. versionadded:: 3.9 diff --git a/Doc/using/configure.rst b/Doc/using/configure.rst index b51546e072a353..8a5e16ece60bb5 100644 --- a/Doc/using/configure.rst +++ b/Doc/using/configure.rst @@ -287,6 +287,8 @@ General Options .. versionadded:: 3.11 +.. _free-threading-buid: + .. option:: --disable-gil Enables **experimental** support for running Python without the From 3bb41e5e95c902193c22ab8ff7509254f59a2834 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 17 Nov 2023 15:16:15 +0100 Subject: [PATCH 4/6] Add comment about _Py_IMMORTAL_REFCNT in Py_INCREF() --- Include/object.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Include/object.h b/Include/object.h index cd600a2f2a24d8..86fcba21caa9c8 100644 --- a/Include/object.h +++ b/Include/object.h @@ -758,6 +758,7 @@ static inline Py_ALWAYS_INLINE void Py_INCREF(PyObject *op) uint32_t local = _Py_atomic_load_uint32_relaxed(&op->ob_ref_local); uint32_t new_local = local + 1; if (new_local == 0) { + // local is equal to _Py_IMMORTAL_REFCNT: do nothing return; } if (_Py_IsOwnedByCurrentThread(op)) { @@ -771,6 +772,8 @@ static inline Py_ALWAYS_INLINE void Py_INCREF(PyObject *op) PY_UINT32_T cur_refcnt = op->ob_refcnt_split[PY_BIG_ENDIAN]; PY_UINT32_T new_refcnt = cur_refcnt + 1; if (new_refcnt == 0) { + // cur_refcnt is equal to _Py_IMMORTAL_REFCNT: the object is immortal, + // do nothing return; } op->ob_refcnt_split[PY_BIG_ENDIAN] = new_refcnt; From 11f91f3765886525a9cd8c268eb2e7dcdb8acd2e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 17 Nov 2023 15:18:54 +0100 Subject: [PATCH 5/6] fix sphinx syntax in the doc --- Doc/c-api/refcounting.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/c-api/refcounting.rst b/Doc/c-api/refcounting.rst index a6a5154c703d04..90317777dc66e7 100644 --- a/Doc/c-api/refcounting.rst +++ b/Doc/c-api/refcounting.rst @@ -34,8 +34,8 @@ of Python objects. Set the object *o* reference counter to *refcnt*. - On ref:`Python build with Free Threading `, if *refcnt* - is larger than ``UINT32_MAX``, the object is made :term:`immortal`. + On :ref:`Python build with Free Threading `, if + *refcnt* is larger than ``UINT32_MAX``, the object is made :term:`immortal`. This function has no effect on :term:`immortal` objects. From d873b981a9e7dd82c852bbae28b204891e560042 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 27 Nov 2023 12:18:36 +0100 Subject: [PATCH 6/6] fix typo in the doc link --- Doc/c-api/refcounting.rst | 2 +- Doc/using/configure.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/c-api/refcounting.rst b/Doc/c-api/refcounting.rst index 90317777dc66e7..75e1d46474f1e7 100644 --- a/Doc/c-api/refcounting.rst +++ b/Doc/c-api/refcounting.rst @@ -34,7 +34,7 @@ of Python objects. Set the object *o* reference counter to *refcnt*. - On :ref:`Python build with Free Threading `, if + On :ref:`Python build with Free Threading `, if *refcnt* is larger than ``UINT32_MAX``, the object is made :term:`immortal`. This function has no effect on :term:`immortal` objects. diff --git a/Doc/using/configure.rst b/Doc/using/configure.rst index 8a5e16ece60bb5..56d2d6dc4ab5f1 100644 --- a/Doc/using/configure.rst +++ b/Doc/using/configure.rst @@ -287,7 +287,7 @@ General Options .. versionadded:: 3.11 -.. _free-threading-buid: +.. _free-threading-build: .. option:: --disable-gil