Skip to content

Commit

Permalink
gh-110481: Fix Py_SET_REFCNT() integer overflow (#112174)
Browse files Browse the repository at this point in the history
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) {
        ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
  • Loading branch information
vstinner committed Dec 1, 2023
1 parent c298238 commit 5f6ac2d
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 10 deletions.
3 changes: 3 additions & 0 deletions Doc/c-api/refcounting.rst
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ of Python objects.
Set the object *o* reference counter to *refcnt*.
On :ref:`Python build with Free Threading <free-threading-build>`, 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
Expand Down
2 changes: 2 additions & 0 deletions Doc/using/configure.rst
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,8 @@ General Options

.. versionadded:: 3.11

.. _free-threading-build:

.. option:: --disable-gil

Enables **experimental** support for running Python without the
Expand Down
31 changes: 21 additions & 10 deletions Include/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_CAST(Py_ssize_t, UINT_MAX)

#else
/*
Expand All @@ -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_CAST(Py_ssize_t, UINT_MAX >> 2)
#endif

// Py_GIL_DISABLED builds indicate immortal objects using `ob_ref_local`, which is
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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;

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Ubuntu (free-threaded) / build and test

‘op’ undeclared (first use in this function); did you mean ‘ob’?

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Ubuntu (free-threaded) / build and test

‘op’ undeclared (first use in this function); did you mean ‘ob’?

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Ubuntu (free-threaded) / build and test

‘op’ undeclared (first use in this function); did you mean ‘ob’?

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Ubuntu (free-threaded) / build and test

‘op’ undeclared (first use in this function); did you mean ‘ob’?

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build and test (x64)

'op': undeclared identifier (compiling source file ..\Modules\faulthandler.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build and test (x64)

'op': undeclared identifier (compiling source file ..\Modules\atexitmodule.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build and test (x64)

'op': undeclared identifier (compiling source file ..\PC\config_minimal.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build and test (x64)

'op': undeclared identifier (compiling source file ..\Programs\_freeze_module.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build and test (x64)

left of '->ob_tid' must point to struct/union (compiling source file ..\Modules\faulthandler.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build and test (x64)

left of '->ob_tid' must point to struct/union (compiling source file ..\Modules\atexitmodule.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build and test (x64)

left of '->ob_tid' must point to struct/union (compiling source file ..\PC\config_minimal.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build and test (x64)

left of '->ob_tid' must point to struct/union (compiling source file ..\Programs\_freeze_module.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build (arm64)

'op': undeclared identifier (compiling source file ..\Modules\faulthandler.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build (arm64)

'op': undeclared identifier (compiling source file ..\Modules\atexitmodule.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build (arm64)

'op': undeclared identifier (compiling source file ..\PC\config_minimal.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build (arm64)

'op': undeclared identifier (compiling source file ..\Programs\_freeze_module.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build (arm64)

left of '->ob_tid' must point to struct/union (compiling source file ..\Modules\faulthandler.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build (arm64)

left of '->ob_tid' must point to struct/union (compiling source file ..\Modules\atexitmodule.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build (arm64)

left of '->ob_tid' must point to struct/union (compiling source file ..\PC\config_minimal.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 360 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build (arm64)

left of '->ob_tid' must point to struct/union (compiling source file ..\Programs\_freeze_module.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]
op->ob_ref_local = _Py_IMMORTAL_REFCNT_LOCAL;

Check failure on line 361 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build and test (x64)

'op': undeclared identifier (compiling source file ..\Modules\faulthandler.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 361 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build and test (x64)

'op': undeclared identifier (compiling source file ..\Modules\atexitmodule.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 361 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build (arm64)

'op': undeclared identifier (compiling source file ..\Modules\atexitmodule.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]

Check failure on line 361 in Include/object.h

View workflow job for this annotation

GitHub Actions / Windows (free-threaded) / build (arm64)

'op': undeclared identifier (compiling source file ..\Modules\faulthandler.c) [D:\a\cpython\cpython\PCbuild\_freeze_module.vcxproj]
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.
Expand Down Expand Up @@ -750,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)) {
Expand All @@ -763,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;
Expand Down

0 comments on commit 5f6ac2d

Please sign in to comment.