gh-129069: make list ass_slice and memory_repeat safe

Author: tom-pytel

Include/internal/pycore_list.h

@@ -48,15 +48,17 @@ _PyList_AppendTakeRef(PyListObject *self, PyObject *newitem)
     return _PyList_AppendTakeRefListResize(self, newitem);
 }
 
-// Repeat the bytes of a buffer in place
+// Repeat the bytes of a buffer of pointers in place
 static inline void
-_Py_memory_repeat(char* dest, Py_ssize_t len_dest, Py_ssize_t len_src)
+_Py_memory_ptrs_repeat(char* dest, Py_ssize_t len_dest, Py_ssize_t len_src)
 {
     assert(len_src > 0);
+    assert(len_src % sizeof(void *) == 0);
+    assert(((uintptr_t)dest & (sizeof (void *) - 1)) == 0);
     Py_ssize_t copied = len_src;
     while (copied < len_dest) {
         Py_ssize_t bytes_to_copy = Py_MIN(copied, len_dest - copied);
-        memcpy(dest + copied, dest, (size_t)bytes_to_copy);
+        FT_ATOMIC_MEMCPY_PTR_RELAXED(dest + copied, dest, (size_t)bytes_to_copy);
         copied += bytes_to_copy;
     }
 }

Include/internal/pycore_pyatomic_ft_wrappers.h

@@ -110,6 +110,48 @@ extern "C" {
 #define FT_ATOMIC_LOAD_ULLONG_RELAXED(value) \
     _Py_atomic_load_ullong_relaxed(&value)
 
+static inline void *
+FT_ATOMIC_MEMCPY_PTR_RELAXED(void *dest, void *src, ssize_t n)
+{
+    assert(((uintptr_t)dest & (sizeof (void *) - 1)) == 0);
+    assert(((uintptr_t)src & (sizeof (void *) - 1)) == 0);
+    assert(n % sizeof(void *) == 0);
+
+    if (dest != src) {
+        for (void **d = (void **)dest, **s = (void **)src, **e = d + n / sizeof(void *); d != e; d++, s++) {
+            void *v = _Py_atomic_load_ptr_relaxed(s);
+            _Py_atomic_store_ptr_relaxed(d, v);
+        }
+    }
+
+    return dest;
+}
+
+static inline void *
+FT_ATOMIC_MEMMOVE_PTR_RELAXED(void *dest, void *src, ssize_t n)
+{
+    assert(((uintptr_t)dest & (sizeof (void *) - 1)) == 0);
+    assert(((uintptr_t)src & (sizeof (void *) - 1)) == 0);
+    assert(n % sizeof(void *) == 0);
+
+    if (dest < src || dest >= (void *)((char *)src + n)) {  // prefer incrementing copy
+        for (void **d = (void **)dest, **s = (void **)src, **e = d + n / sizeof(void *); d != e; d++, s++) {
+            void *v = _Py_atomic_load_ptr_relaxed(s);
+            _Py_atomic_store_ptr_relaxed(d, v);
+        }
+    }
+    else if (dest > src) {
+        n = n / sizeof(void *) - 1;
+        for (void **d = (void **)dest + n, **s = (void **)src + n, **e = (void **)dest - 1; d != e; d--, s--) {
+            void *v = _Py_atomic_load_ptr_relaxed(s);
+            _Py_atomic_store_ptr_relaxed(d, v);
+        }
+    }
+
+    return dest;
+}
+
+
 #else
 #define FT_ATOMIC_LOAD_PTR(value) value
 #define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
@@ -157,6 +199,9 @@ extern "C" {
 #define FT_ATOMIC_LOAD_ULLONG_RELAXED(value) value
 #define FT_ATOMIC_STORE_ULLONG_RELAXED(value, new_value) value = new_value
 
+#define FT_ATOMIC_MEMCPY_PTR_RELAXED(dest, src, n) memcpy(dest, src, n)
+#define FT_ATOMIC_MEMMOVE_PTR_RELAXED(dest, src, n) memmove(dest, src, n)
+
 #endif
 
 #ifdef __cplusplus

Objects/listobject.c

@@ -813,10 +813,8 @@ list_repeat_lock_held(PyListObject *a, Py_ssize_t n)
             _Py_RefcntAdd(*src, n);
             *dest++ = *src++;
         }
-        // TODO: _Py_memory_repeat calls are not safe for shared lists in
-        // GIL_DISABLED builds. (See issue #129069)
-        _Py_memory_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size,
-                                        sizeof(PyObject *)*input_size);
+        _Py_memory_ptrs_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size,
+                               sizeof(PyObject *)*input_size);
     }
 
     Py_SET_SIZE(np, output_size);
@@ -949,12 +947,10 @@ list_ass_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyO
     if (d < 0) { /* Delete -d items */
         Py_ssize_t tail;
         tail = (Py_SIZE(a) - ihigh) * sizeof(PyObject *);
-        // TODO: these memmove/memcpy calls are not safe for shared lists in
-        // GIL_DISABLED builds. (See issue #129069)
-        memmove(&item[ihigh+d], &item[ihigh], tail);
+        FT_ATOMIC_MEMMOVE_PTR_RELAXED(&item[ihigh+d], &item[ihigh], tail);
         if (list_resize(a, Py_SIZE(a) + d) < 0) {
-            memmove(&item[ihigh], &item[ihigh+d], tail);
-            memcpy(&item[ilow], recycle, s);
+            FT_ATOMIC_MEMMOVE_PTR_RELAXED(&item[ihigh], &item[ihigh+d], tail);
+            FT_ATOMIC_MEMCPY_PTR_RELAXED(&item[ilow], recycle, s);
             goto Error;
         }
         item = a->ob_item;
@@ -964,10 +960,8 @@ list_ass_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyO
         if (list_resize(a, k+d) < 0)
             goto Error;
         item = a->ob_item;
-        // TODO: these memmove/memcpy calls are not safe for shared lists in
-        // GIL_DISABLED builds. (See issue #129069)
-        memmove(&item[ihigh+d], &item[ihigh],
-            (k - ihigh)*sizeof(PyObject *));
+        FT_ATOMIC_MEMMOVE_PTR_RELAXED(&item[ihigh+d], &item[ihigh],
+                                      (k - ihigh)*sizeof(PyObject *));
     }
     for (k = 0; k < n; k++, ilow++) {
         PyObject *w = vitem[k];
@@ -1051,10 +1045,8 @@ list_inplace_repeat_lock_held(PyListObject *self, Py_ssize_t n)
     for (Py_ssize_t j = 0; j < input_size; j++) {
         _Py_RefcntAdd(items[j], n-1);
     }
-    // TODO: _Py_memory_repeat calls are not safe for shared lists in
-    // GIL_DISABLED builds. (See issue #129069)
-    _Py_memory_repeat((char *)items, sizeof(PyObject *)*output_size,
-                      sizeof(PyObject *)*input_size);
+    _Py_memory_ptrs_repeat((char *)items, sizeof(PyObject *)*output_size,
+                           sizeof(PyObject *)*input_size);
     return 0;
 }
 

Objects/tupleobject.c

@@ -5,7 +5,7 @@
 #include "pycore_ceval.h"         // _PyEval_GetBuiltin()
 #include "pycore_freelist.h"      // _Py_FREELIST_PUSH()
 #include "pycore_gc.h"            // _PyObject_GC_IS_TRACKED()
-#include "pycore_list.h"          // _Py_memory_repeat()
+#include "pycore_list.h"          // _Py_memory_ptrs_repeat()
 #include "pycore_modsupport.h"    // _PyArg_NoKwnames()
 #include "pycore_object.h"        // _PyObject_GC_TRACK()
 #include "pycore_stackref.h"      // PyStackRef_AsPyObjectSteal()
@@ -547,8 +547,8 @@ tuple_repeat(PyObject *self, Py_ssize_t n)
             *dest++ = *src++;
         }
 
-        _Py_memory_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size,
-                          sizeof(PyObject *)*input_size);
+        _Py_memory_ptrs_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size,
+                               sizeof(PyObject *)*input_size);
     }
     _PyObject_GC_TRACK(np);
     return (PyObject *) np;