gh-117657: TSAN Fix races in `PyMember_Get` and `PyMember_Set` for C extensions (GH-123211)

1 files changed, 51 insertions, 46 deletions

diff --git a/Python/structmember.c b/Python/structmember.c
index d5e7ab8..d36e049 100644
--- a/Python/structmember.c
+++ b/Python/structmember.c
@@ -34,40 +34,40 @@ PyMember_GetOne(const char *obj_addr, PyMemberDef *l)
     const char* addr = obj_addr + l->offset;
     switch (l->type) {
     case Py_T_BOOL:
-        v = PyBool_FromLong(*(char*)addr);
+        v = PyBool_FromLong(FT_ATOMIC_LOAD_CHAR_RELAXED(*(char*)addr));
         break;
     case Py_T_BYTE:
-        v = PyLong_FromLong(*(char*)addr);
+        v = PyLong_FromLong(FT_ATOMIC_LOAD_CHAR_RELAXED(*(char*)addr));
         break;
     case Py_T_UBYTE:
-        v = PyLong_FromUnsignedLong(*(unsigned char*)addr);
+        v = PyLong_FromUnsignedLong(FT_ATOMIC_LOAD_UCHAR_RELAXED(*(unsigned char*)addr));
         break;
     case Py_T_SHORT:
-        v = PyLong_FromLong(*(short*)addr);
+        v = PyLong_FromLong(FT_ATOMIC_LOAD_SHORT_RELAXED(*(short*)addr));
         break;
     case Py_T_USHORT:
-        v = PyLong_FromUnsignedLong(*(unsigned short*)addr);
+        v = PyLong_FromUnsignedLong(FT_ATOMIC_LOAD_USHORT_RELAXED(*(unsigned short*)addr));
         break;
     case Py_T_INT:
-        v = PyLong_FromLong(*(int*)addr);
+        v = PyLong_FromLong(FT_ATOMIC_LOAD_INT_RELAXED(*(int*)addr));
         break;
     case Py_T_UINT:
-        v = PyLong_FromUnsignedLong(*(unsigned int*)addr);
+        v = PyLong_FromUnsignedLong(FT_ATOMIC_LOAD_UINT_RELAXED(*(unsigned int*)addr));
         break;
     case Py_T_LONG:
-        v = PyLong_FromLong(*(long*)addr);
+        v = PyLong_FromLong(FT_ATOMIC_LOAD_LONG_RELAXED(*(long*)addr));
         break;
     case Py_T_ULONG:
-        v = PyLong_FromUnsignedLong(*(unsigned long*)addr);
+        v = PyLong_FromUnsignedLong(FT_ATOMIC_LOAD_ULONG_RELAXED(*(unsigned long*)addr));
         break;
     case Py_T_PYSSIZET:
-        v = PyLong_FromSsize_t(*(Py_ssize_t*)addr);
+        v = PyLong_FromSsize_t(FT_ATOMIC_LOAD_SSIZE_RELAXED(*(Py_ssize_t*)addr));
         break;
     case Py_T_FLOAT:
-        v = PyFloat_FromDouble((double)*(float*)addr);
+        v = PyFloat_FromDouble((double)FT_ATOMIC_LOAD_FLOAT_RELAXED(*(float*)addr));
         break;
     case Py_T_DOUBLE:
-        v = PyFloat_FromDouble(*(double*)addr);
+        v = PyFloat_FromDouble(FT_ATOMIC_LOAD_DOUBLE_RELAXED(*(double*)addr));
         break;
     case Py_T_STRING:
         if (*(char**)addr == NULL) {
@@ -79,9 +79,11 @@ PyMember_GetOne(const char *obj_addr, PyMemberDef *l)
     case Py_T_STRING_INPLACE:
         v = PyUnicode_FromString((char*)addr);
         break;
-    case Py_T_CHAR:
-        v = PyUnicode_FromStringAndSize((char*)addr, 1);
+    case Py_T_CHAR: {
+        char char_val = FT_ATOMIC_LOAD_CHAR_RELAXED(*addr);
+        v = PyUnicode_FromStringAndSize(&char_val, 1);
         break;
+    }
     case _Py_T_OBJECT:
         v = *(PyObject **)addr;
         if (v == NULL)
@@ -104,10 +106,10 @@ PyMember_GetOne(const char *obj_addr, PyMemberDef *l)
 #endif
         break;
     case Py_T_LONGLONG:
-        v = PyLong_FromLongLong(*(long long *)addr);
+        v = PyLong_FromLongLong(FT_ATOMIC_LOAD_LLONG_RELAXED(*(long long *)addr));
         break;
     case Py_T_ULONGLONG:
-        v = PyLong_FromUnsignedLongLong(*(unsigned long long *)addr);
+        v = PyLong_FromUnsignedLongLong(FT_ATOMIC_LOAD_ULLONG_RELAXED(*(unsigned long long *)addr));
         break;
     case _Py_T_NONE:
         // doesn't require free-threading code path
@@ -169,16 +171,16 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
             return -1;
         }
         if (v == Py_True)
-            *(char*)addr = (char) 1;
+            FT_ATOMIC_STORE_CHAR_RELAXED(*(char*)addr, 1);
         else
-            *(char*)addr = (char) 0;
+            FT_ATOMIC_STORE_CHAR_RELAXED(*(char*)addr, 0);
         break;
         }
     case Py_T_BYTE:{
         long long_val = PyLong_AsLong(v);
         if ((long_val == -1) && PyErr_Occurred())
             return -1;
-        *(char*)addr = (char)long_val;
+        FT_ATOMIC_STORE_CHAR_RELAXED(*(char*)addr, (char)long_val);
         /* XXX: For compatibility, only warn about truncations
            for now. */
         if ((long_val > CHAR_MAX) || (long_val < CHAR_MIN))
@@ -189,7 +191,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
         long long_val = PyLong_AsLong(v);
         if ((long_val == -1) && PyErr_Occurred())
             return -1;
-        *(unsigned char*)addr = (unsigned char)long_val;
+        FT_ATOMIC_STORE_UCHAR_RELAXED(*(unsigned char*)addr, (unsigned char)long_val);
         if ((long_val > UCHAR_MAX) || (long_val < 0))
             WARN("Truncation of value to unsigned char");
         break;
@@ -198,7 +200,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
         long long_val = PyLong_AsLong(v);
         if ((long_val == -1) && PyErr_Occurred())
             return -1;
-        *(short*)addr = (short)long_val;
+        FT_ATOMIC_STORE_SHORT_RELAXED(*(short*)addr, (short)long_val);
         if ((long_val > SHRT_MAX) || (long_val < SHRT_MIN))
             WARN("Truncation of value to short");
         break;
@@ -207,7 +209,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
         long long_val = PyLong_AsLong(v);
         if ((long_val == -1) && PyErr_Occurred())
             return -1;
-        *(unsigned short*)addr = (unsigned short)long_val;
+        FT_ATOMIC_STORE_USHORT_RELAXED(*(unsigned short*)addr, (unsigned short)long_val);
         if ((long_val > USHRT_MAX) || (long_val < 0))
             WARN("Truncation of value to unsigned short");
         break;
@@ -216,7 +218,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
         long long_val = PyLong_AsLong(v);
         if ((long_val == -1) && PyErr_Occurred())
             return -1;
-        *(int *)addr = (int)long_val;
+        FT_ATOMIC_STORE_INT_RELAXED(*(int *)addr, (int)long_val);
         if ((long_val > INT_MAX) || (long_val < INT_MIN))
             WARN("Truncation of value to int");
         break;
@@ -234,7 +236,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
             if (long_val == -1 && PyErr_Occurred()) {
                 return -1;
             }
-            *(unsigned int *)addr = (unsigned int)(unsigned long)long_val;
+            FT_ATOMIC_STORE_UINT_RELAXED(*(unsigned int *)addr, (unsigned int)(unsigned long)long_val);
             WARN("Writing negative value into unsigned field");
         }
         else {
@@ -243,19 +245,20 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
             if (ulong_val == (unsigned long)-1 && PyErr_Occurred()) {
                 return -1;
             }
-            *(unsigned int*)addr = (unsigned int)ulong_val;
+            FT_ATOMIC_STORE_UINT_RELAXED(*(unsigned int *)addr, (unsigned int)ulong_val);
             if (ulong_val > UINT_MAX) {
                 WARN("Truncation of value to unsigned int");
             }
         }
         break;
     }
-    case Py_T_LONG:{
-        *(long*)addr = PyLong_AsLong(v);
-        if ((*(long*)addr == -1) && PyErr_Occurred())
+    case Py_T_LONG: {
+        const long long_val = PyLong_AsLong(v);
+        if ((long_val == -1) && PyErr_Occurred())
             return -1;
+        FT_ATOMIC_STORE_LONG_RELAXED(*(long*)addr, long_val);
         break;
-        }
+    }
     case Py_T_ULONG: {
         /* XXX: For compatibility, accept negative int values
            as well. */
@@ -269,7 +272,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
             if (long_val == -1 && PyErr_Occurred()) {
                 return -1;
             }
-            *(unsigned long *)addr = (unsigned long)long_val;
+            FT_ATOMIC_STORE_ULONG_RELAXED(*(unsigned long *)addr, (unsigned long)long_val);
             WARN("Writing negative value into unsigned field");
         }
         else {
@@ -278,29 +281,31 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
             if (ulong_val == (unsigned long)-1 && PyErr_Occurred()) {
                 return -1;
             }
-            *(unsigned long*)addr = ulong_val;
+            FT_ATOMIC_STORE_ULONG_RELAXED(*(unsigned long *)addr, ulong_val);
         }
         break;
     }
-    case Py_T_PYSSIZET:{
-        *(Py_ssize_t*)addr = PyLong_AsSsize_t(v);
-        if ((*(Py_ssize_t*)addr == (Py_ssize_t)-1)
-            && PyErr_Occurred())
-                        return -1;
+    case Py_T_PYSSIZET: {
+        const Py_ssize_t ssize_val = PyLong_AsSsize_t(v);
+        if ((ssize_val == (Py_ssize_t)-1) && PyErr_Occurred())
+            return -1;
+        FT_ATOMIC_STORE_SSIZE_RELAXED(*(Py_ssize_t*)addr, ssize_val);
         break;
-        }
+    }
     case Py_T_FLOAT:{
         double double_val = PyFloat_AsDouble(v);
         if ((double_val == -1) && PyErr_Occurred())
             return -1;
-        *(float*)addr = (float)double_val;
+        FT_ATOMIC_STORE_FLOAT_RELAXED(*(float*)addr, (float)double_val);
         break;
         }
-    case Py_T_DOUBLE:
-        *(double*)addr = PyFloat_AsDouble(v);
-        if ((*(double*)addr == -1) && PyErr_Occurred())
+    case Py_T_DOUBLE: {
+        const double double_val = PyFloat_AsDouble(v);
+        if ((double_val == -1) && PyErr_Occurred())
             return -1;
+        FT_ATOMIC_STORE_DOUBLE_RELAXED(*(double *) addr, double_val);
         break;
+    }
     case _Py_T_OBJECT:
     case Py_T_OBJECT_EX:
         Py_BEGIN_CRITICAL_SECTION(obj);
@@ -318,7 +323,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
             PyErr_BadArgument();
             return -1;
         }
-        *(char*)addr = string[0];
+        FT_ATOMIC_STORE_CHAR_RELAXED(*(char*)addr, string[0]);
         break;
         }
     case Py_T_STRING:
@@ -326,10 +331,10 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
         PyErr_SetString(PyExc_TypeError, "readonly attribute");
         return -1;
     case Py_T_LONGLONG:{
-        long long value;
-        *(long long*)addr = value = PyLong_AsLongLong(v);
+        long long value = PyLong_AsLongLong(v);
         if ((value == -1) && PyErr_Occurred())
             return -1;
+        FT_ATOMIC_STORE_LLONG_RELAXED(*(long long*)addr, value);
         break;
         }
     case Py_T_ULONGLONG: {
@@ -343,7 +348,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
             if (long_val == -1 && PyErr_Occurred()) {
                 return -1;
             }
-            *(unsigned long long *)addr = (unsigned long long)(long long)long_val;
+            FT_ATOMIC_STORE_ULLONG_RELAXED(*(unsigned long long *)addr, (unsigned long long)(long long)long_val);
             WARN("Writing negative value into unsigned field");
         }
         else {
@@ -352,7 +357,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v)
             if (ulonglong_val == (unsigned long long)-1 && PyErr_Occurred()) {
                 return -1;
             }
-            *(unsigned long long*)addr = ulonglong_val;
+            FT_ATOMIC_STORE_ULLONG_RELAXED(*(unsigned long long *)addr, ulonglong_val);
         }
         break;
     }