Skip to content

Commit 0dae3ba

Browse files
committed
gh-142534: Avoid TSan warnings in dictobject.c
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.
1 parent 4629567 commit 0dae3ba

4 files changed

Lines changed: 19 additions & 7 deletions

File tree

Include/cpython/pyatomic.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,17 @@ static inline void _Py_atomic_fence_release(void);
591591

592592
// --- aliases ---------------------------------------------------------------
593593

594+
// Compilers don't really support "consume" semantics, so we fake it. Use
595+
// "acquire" with TSan to support false positives. Use "relaxed" otherwise,
596+
// because CPUs on all platforms we support respect address dependencies without
597+
// extra barriers.
598+
// See 2.6.7 in https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2055r0.pdf
599+
#if defined(_Py_THREAD_SANITIZER)
600+
# define _Py_atomic_load_ptr_consume _Py_atomic_load_ptr_acquire
601+
#else
602+
# define _Py_atomic_load_ptr_consume _Py_atomic_load_ptr_relaxed
603+
#endif
604+
594605
#if SIZEOF_LONG == 8
595606
# define _Py_atomic_load_ulong(p) \
596607
_Py_atomic_load_uint64((uint64_t *)p)

Include/cpython/pyatomic_gcc.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,6 @@ static inline void
549549
_Py_atomic_store_llong_relaxed(long long *obj, long long value)
550550
{ __atomic_store_n(obj, value, __ATOMIC_RELAXED); }
551551

552-
553552
// --- _Py_atomic_load_ptr_acquire / _Py_atomic_store_ptr_release ------------
554553

555554
static inline void *

Include/internal/pycore_pyatomic_ft_wrappers.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ extern "C" {
3131
_Py_atomic_store_ptr(&value, new_value)
3232
#define FT_ATOMIC_LOAD_PTR_ACQUIRE(value) \
3333
_Py_atomic_load_ptr_acquire(&value)
34+
#define FT_ATOMIC_LOAD_PTR_CONSUME(value) \
35+
_Py_atomic_load_ptr_consume(&value)
3436
#define FT_ATOMIC_LOAD_UINTPTR_ACQUIRE(value) \
3537
_Py_atomic_load_uintptr_acquire(&value)
3638
#define FT_ATOMIC_LOAD_PTR_RELAXED(value) \

Objects/dictobject.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,7 +1078,7 @@ compare_unicode_unicode(PyDictObject *mp, PyDictKeysObject *dk,
10781078
void *ep0, Py_ssize_t ix, PyObject *key, Py_hash_t hash)
10791079
{
10801080
PyDictUnicodeEntry *ep = &((PyDictUnicodeEntry *)ep0)[ix];
1081-
PyObject *ep_key = FT_ATOMIC_LOAD_PTR_RELAXED(ep->me_key);
1081+
PyObject *ep_key = FT_ATOMIC_LOAD_PTR_CONSUME(ep->me_key);
10821082
assert(ep_key != NULL);
10831083
assert(PyUnicode_CheckExact(ep_key));
10841084
if (ep_key == key ||
@@ -1371,7 +1371,7 @@ compare_unicode_generic_threadsafe(PyDictObject *mp, PyDictKeysObject *dk,
13711371
void *ep0, Py_ssize_t ix, PyObject *key, Py_hash_t hash)
13721372
{
13731373
PyDictUnicodeEntry *ep = &((PyDictUnicodeEntry *)ep0)[ix];
1374-
PyObject *startkey = _Py_atomic_load_ptr_relaxed(&ep->me_key);
1374+
PyObject *startkey = _Py_atomic_load_ptr_consume(&ep->me_key);
13751375
assert(startkey == NULL || PyUnicode_CheckExact(ep->me_key));
13761376
assert(!PyUnicode_CheckExact(key));
13771377

@@ -1414,7 +1414,7 @@ compare_unicode_unicode_threadsafe(PyDictObject *mp, PyDictKeysObject *dk,
14141414
void *ep0, Py_ssize_t ix, PyObject *key, Py_hash_t hash)
14151415
{
14161416
PyDictUnicodeEntry *ep = &((PyDictUnicodeEntry *)ep0)[ix];
1417-
PyObject *startkey = _Py_atomic_load_ptr_relaxed(&ep->me_key);
1417+
PyObject *startkey = _Py_atomic_load_ptr_consume(&ep->me_key);
14181418
if (startkey == key) {
14191419
assert(PyUnicode_CheckExact(startkey));
14201420
return 1;
@@ -1450,7 +1450,7 @@ compare_generic_threadsafe(PyDictObject *mp, PyDictKeysObject *dk,
14501450
void *ep0, Py_ssize_t ix, PyObject *key, Py_hash_t hash)
14511451
{
14521452
PyDictKeyEntry *ep = &((PyDictKeyEntry *)ep0)[ix];
1453-
PyObject *startkey = _Py_atomic_load_ptr_relaxed(&ep->me_key);
1453+
PyObject *startkey = _Py_atomic_load_ptr_consume(&ep->me_key);
14541454
if (startkey == key) {
14551455
return 1;
14561456
}
@@ -5526,7 +5526,7 @@ dictiter_iternext_threadsafe(PyDictObject *d, PyObject *self,
55265526
k = _Py_atomic_load_ptr_acquire(&d->ma_keys);
55275527
assert(i >= 0);
55285528
if (_PyDict_HasSplitTable(d)) {
5529-
PyDictValues *values = _Py_atomic_load_ptr_relaxed(&d->ma_values);
5529+
PyDictValues *values = _Py_atomic_load_ptr_consume(&d->ma_values);
55305530
if (values == NULL) {
55315531
goto concurrent_modification;
55325532
}
@@ -7114,7 +7114,7 @@ _PyObject_TryGetInstanceAttribute(PyObject *obj, PyObject *name, PyObject **attr
71147114
Py_BEGIN_CRITICAL_SECTION(dict);
71157115

71167116
if (dict->ma_values == values && FT_ATOMIC_LOAD_UINT8(values->valid)) {
7117-
value = _Py_atomic_load_ptr_relaxed(&values->values[ix]);
7117+
value = _Py_atomic_load_ptr_consume(&values->values[ix]);
71187118
*attr = _Py_XNewRefWithLock(value);
71197119
success = true;
71207120
} else {

0 commit comments

Comments
 (0)