From 2fbc9861c3b1325095019b3df6f3b5b0a1235f1f Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 12 Feb 2025 10:43:25 -0500 Subject: [PATCH] [3.13] gh-128759: Fix accesses to `tp_version_tag`. (GH-129750) (GH-130042) We should use a relaxed atomic load in the free threading build in `PyType_Modified()` because that's called without the type lock held. It's not necessary to use atomics in `type_modified_unlocked()`. We should also use `FT_ATOMIC_STORE_UINT_RELAXED()` instead of the `UINT32` variant because `tp_version_tag` is declared as `unsigned int`. (cherry picked from commit 57f45ee2d8ee23c2a1d1daba4095a5a044169419) --- .../internal/pycore_pyatomic_ft_wrappers.h | 73 ++++++++++++++++++- Objects/typeobject.c | 11 +-- 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index a1bb383bcd22..d755d03a5fa1 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -61,6 +61,54 @@ extern "C" { _Py_atomic_store_uint16_relaxed(&value, new_value) #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) \ _Py_atomic_store_uint32_relaxed(&value, new_value) +#define FT_ATOMIC_STORE_CHAR_RELAXED(value, new_value) \ + _Py_atomic_store_char_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_CHAR_RELAXED(value) \ + _Py_atomic_load_char_relaxed(&value) +#define FT_ATOMIC_STORE_UCHAR_RELAXED(value, new_value) \ + _Py_atomic_store_uchar_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_UCHAR_RELAXED(value) \ + _Py_atomic_load_uchar_relaxed(&value) +#define FT_ATOMIC_STORE_SHORT_RELAXED(value, new_value) \ + _Py_atomic_store_short_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_SHORT_RELAXED(value) \ + _Py_atomic_load_short_relaxed(&value) +#define FT_ATOMIC_STORE_USHORT_RELAXED(value, new_value) \ + _Py_atomic_store_ushort_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_USHORT_RELAXED(value) \ + _Py_atomic_load_ushort_relaxed(&value) +#define FT_ATOMIC_STORE_INT_RELAXED(value, new_value) \ + _Py_atomic_store_int_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_INT_RELAXED(value) \ + _Py_atomic_load_int_relaxed(&value) +#define FT_ATOMIC_STORE_UINT_RELAXED(value, new_value) \ + _Py_atomic_store_uint_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_UINT_RELAXED(value) \ + _Py_atomic_load_uint_relaxed(&value) +#define FT_ATOMIC_STORE_LONG_RELAXED(value, new_value) \ + _Py_atomic_store_long_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_LONG_RELAXED(value) \ + _Py_atomic_load_long_relaxed(&value) +#define FT_ATOMIC_STORE_ULONG_RELAXED(value, new_value) \ + _Py_atomic_store_ulong_relaxed(&value, new_value) +#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \ + _Py_atomic_store_ssize_relaxed(&value, new_value) +#define FT_ATOMIC_STORE_FLOAT_RELAXED(value, new_value) \ + _Py_atomic_store_float_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_FLOAT_RELAXED(value) \ + _Py_atomic_load_float_relaxed(&value) +#define FT_ATOMIC_STORE_DOUBLE_RELAXED(value, new_value) \ + _Py_atomic_store_double_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_DOUBLE_RELAXED(value) \ + _Py_atomic_load_double_relaxed(&value) +#define FT_ATOMIC_STORE_LLONG_RELAXED(value, new_value) \ + _Py_atomic_store_llong_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_LLONG_RELAXED(value) \ + _Py_atomic_load_llong_relaxed(&value) +#define FT_ATOMIC_STORE_ULLONG_RELAXED(value, new_value) \ + _Py_atomic_store_ullong_relaxed(&value, new_value) +#define FT_ATOMIC_LOAD_ULLONG_RELAXED(value) \ + _Py_atomic_load_ullong_relaxed(&value) #else #define FT_ATOMIC_LOAD_PTR(value) value @@ -68,7 +116,6 @@ extern "C" { #define FT_ATOMIC_LOAD_SSIZE(value) value #define FT_ATOMIC_LOAD_SSIZE_ACQUIRE(value) value #define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value -#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value #define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) value #define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) value #define FT_ATOMIC_LOAD_PTR_RELAXED(value) value @@ -85,6 +132,30 @@ extern "C" { #define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value #define FT_ATOMIC_STORE_UINT32_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_CHAR_RELAXED(value) value +#define FT_ATOMIC_STORE_CHAR_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_UCHAR_RELAXED(value) value +#define FT_ATOMIC_STORE_UCHAR_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_SHORT_RELAXED(value) value +#define FT_ATOMIC_STORE_SHORT_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_USHORT_RELAXED(value) value +#define FT_ATOMIC_STORE_USHORT_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_INT_RELAXED(value) value +#define FT_ATOMIC_STORE_INT_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_UINT_RELAXED(value) value +#define FT_ATOMIC_STORE_UINT_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_LONG_RELAXED(value) value +#define FT_ATOMIC_STORE_LONG_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_STORE_ULONG_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_FLOAT_RELAXED(value) value +#define FT_ATOMIC_STORE_FLOAT_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_DOUBLE_RELAXED(value) value +#define FT_ATOMIC_STORE_DOUBLE_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_LLONG_RELAXED(value) value +#define FT_ATOMIC_STORE_LLONG_RELAXED(value, new_value) value = new_value +#define FT_ATOMIC_LOAD_ULLONG_RELAXED(value) value +#define FT_ATOMIC_STORE_ULLONG_RELAXED(value, new_value) value = new_value #endif diff --git a/Objects/typeobject.c b/Objects/typeobject.c index bd79676b8e92..57e03c669d91 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -975,7 +975,7 @@ set_version_unlocked(PyTypeObject *tp, unsigned int version) _Py_atomic_add_uint16(&tp->tp_versions_used, 1); } #endif - FT_ATOMIC_STORE_UINT32_RELAXED(tp->tp_version_tag, version); + FT_ATOMIC_STORE_UINT_RELAXED(tp->tp_version_tag, version); } static void @@ -996,15 +996,10 @@ type_modified_unlocked(PyTypeObject *type) We don't assign new version tags eagerly, but only as needed. */ -#ifdef Py_GIL_DISABLED - if (_Py_atomic_load_uint_relaxed(&type->tp_version_tag) == 0) { - return; - } -#else + ASSERT_TYPE_LOCK_HELD(); if (type->tp_version_tag == 0) { return; } -#endif // Cannot modify static builtin types. assert((type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN) == 0); @@ -1056,7 +1051,7 @@ void PyType_Modified(PyTypeObject *type) { // Quick check without the lock held - if (type->tp_version_tag == 0) { + if (FT_ATOMIC_LOAD_UINT_RELAXED(type->tp_version_tag) == 0) { return; } -- 2.47.3