https://github.com/python/cpython/commit/0a62f8277e9a0dd9f34b0b070adb83994e81b2a8 commit: 0a62f8277e9a0dd9f34b0b070adb83994e81b2a8 branch: main author: Sam Gross <[email protected]> committer: colesbury <[email protected]> date: 2025-12-11T16:23:19-05:00 summary:
gh-142534: Avoid TSan warnings in dictobject.c (gh-142544) 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. 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 2ae0185226f847..817c0763bf899b 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) \ @@ -125,6 +127,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 e0eef7b46df4b2..ac4a46dab107e8 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1078,7 +1078,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 || @@ -1371,7 +1371,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)); @@ -1414,7 +1414,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; @@ -1450,7 +1450,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; } @@ -5526,7 +5526,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; } @@ -7114,7 +7114,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]
