https://github.com/python/cpython/commit/b868f1414c5fec33664e49e60738498e1443f36c commit: b868f1414c5fec33664e49e60738498e1443f36c branch: 3.14 author: Miss Islington (bot) <[email protected]> committer: colesbury <[email protected]> date: 2025-12-11T21:51:02Z summary:
[3.14] gh-142534: Avoid TSan warnings in dictobject.c (gh-142544) (gh-142603) There are places we use "relaxed" loads where C11 requires "consume" or stronger. Unfortunately, compilers don't really implement "consume" so fake it for our use in a way that avoids upsetting TSan. (cherry picked from commit 0a62f8277e9a0dd9f34b0b070adb83994e81b2a8) Co-authored-by: Sam Gross <[email protected]> files: M Include/cpython/pyatomic.h M Include/internal/pycore_pyatomic_ft_wrappers.h M Objects/dictobject.c diff --git a/Include/cpython/pyatomic.h b/Include/cpython/pyatomic.h index 2a0c11e7b3ad66..790640309f1e03 100644 --- a/Include/cpython/pyatomic.h +++ b/Include/cpython/pyatomic.h @@ -591,6 +591,17 @@ static inline void _Py_atomic_fence_release(void); // --- aliases --------------------------------------------------------------- +// Compilers don't really support "consume" semantics, so we fake it. Use +// "acquire" with TSan to support false positives. Use "relaxed" otherwise, +// because CPUs on all platforms we support respect address dependencies without +// extra barriers. +// See 2.6.7 in https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2055r0.pdf +#if defined(_Py_THREAD_SANITIZER) +# define _Py_atomic_load_ptr_consume _Py_atomic_load_ptr_acquire +#else +# define _Py_atomic_load_ptr_consume _Py_atomic_load_ptr_relaxed +#endif + #if SIZEOF_LONG == 8 # define _Py_atomic_load_ulong(p) \ _Py_atomic_load_uint64((uint64_t *)p) diff --git a/Include/internal/pycore_pyatomic_ft_wrappers.h b/Include/internal/pycore_pyatomic_ft_wrappers.h index 223ee951e4bcc2..4187f46995e2c3 100644 --- a/Include/internal/pycore_pyatomic_ft_wrappers.h +++ b/Include/internal/pycore_pyatomic_ft_wrappers.h @@ -31,6 +31,8 @@ extern "C" { _Py_atomic_store_ptr(&value, new_value) #define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) \ _Py_atomic_load_ptr_acquire(&value) +#define FT_ATOMIC_LOAD_PTR_CONSUME(value) \ + _Py_atomic_load_ptr_consume(&value) #define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) \ _Py_atomic_load_uintptr_acquire(&value) #define FT_ATOMIC_LOAD_PTR_RELAXED(value) \ @@ -123,6 +125,7 @@ extern "C" { #define FT_ATOMIC_LOAD_SSIZE_ACQUIRE(value) value #define FT_ATOMIC_LOAD_SSIZE_RELAXED(value) value #define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) value +#define FT_ATOMIC_LOAD_PTR_CONSUME(value) value #define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) value #define FT_ATOMIC_LOAD_PTR_RELAXED(value) value #define FT_ATOMIC_LOAD_UINT8(value) value diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 0eb00410cf41eb..62be2c53d670ef 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1082,7 +1082,7 @@ compare_unicode_unicode(PyDictObject *mp, PyDictKeysObject *dk, void *ep0, Py_ssize_t ix, PyObject *key, Py_hash_t hash) { PyDictUnicodeEntry *ep = &((PyDictUnicodeEntry *)ep0)[ix]; - PyObject *ep_key = FT_ATOMIC_LOAD_PTR_RELAXED(ep->me_key); + PyObject *ep_key = FT_ATOMIC_LOAD_PTR_CONSUME(ep->me_key); assert(ep_key != NULL); assert(PyUnicode_CheckExact(ep_key)); if (ep_key == key || @@ -1375,7 +1375,7 @@ compare_unicode_generic_threadsafe(PyDictObject *mp, PyDictKeysObject *dk, void *ep0, Py_ssize_t ix, PyObject *key, Py_hash_t hash) { PyDictUnicodeEntry *ep = &((PyDictUnicodeEntry *)ep0)[ix]; - PyObject *startkey = _Py_atomic_load_ptr_relaxed(&ep->me_key); + PyObject *startkey = _Py_atomic_load_ptr_consume(&ep->me_key); assert(startkey == NULL || PyUnicode_CheckExact(ep->me_key)); assert(!PyUnicode_CheckExact(key)); @@ -1418,7 +1418,7 @@ compare_unicode_unicode_threadsafe(PyDictObject *mp, PyDictKeysObject *dk, void *ep0, Py_ssize_t ix, PyObject *key, Py_hash_t hash) { PyDictUnicodeEntry *ep = &((PyDictUnicodeEntry *)ep0)[ix]; - PyObject *startkey = _Py_atomic_load_ptr_relaxed(&ep->me_key); + PyObject *startkey = _Py_atomic_load_ptr_consume(&ep->me_key); if (startkey == key) { assert(PyUnicode_CheckExact(startkey)); return 1; @@ -1454,7 +1454,7 @@ compare_generic_threadsafe(PyDictObject *mp, PyDictKeysObject *dk, void *ep0, Py_ssize_t ix, PyObject *key, Py_hash_t hash) { PyDictKeyEntry *ep = &((PyDictKeyEntry *)ep0)[ix]; - PyObject *startkey = _Py_atomic_load_ptr_relaxed(&ep->me_key); + PyObject *startkey = _Py_atomic_load_ptr_consume(&ep->me_key); if (startkey == key) { return 1; } @@ -5519,7 +5519,7 @@ dictiter_iternext_threadsafe(PyDictObject *d, PyObject *self, k = _Py_atomic_load_ptr_acquire(&d->ma_keys); assert(i >= 0); if (_PyDict_HasSplitTable(d)) { - PyDictValues *values = _Py_atomic_load_ptr_relaxed(&d->ma_values); + PyDictValues *values = _Py_atomic_load_ptr_consume(&d->ma_values); if (values == NULL) { goto concurrent_modification; } @@ -7111,7 +7111,7 @@ _PyObject_TryGetInstanceAttribute(PyObject *obj, PyObject *name, PyObject **attr Py_BEGIN_CRITICAL_SECTION(dict); if (dict->ma_values == values && FT_ATOMIC_LOAD_UINT8(values->valid)) { - value = _Py_atomic_load_ptr_relaxed(&values->values[ix]); + value = _Py_atomic_load_ptr_consume(&values->values[ix]); *attr = _Py_XNewRefWithLock(value); success = true; } else { _______________________________________________ Python-checkins mailing list -- [email protected] To unsubscribe send an email to [email protected] https://mail.python.org/mailman3//lists/python-checkins.python.org Member address: [email protected]
