Skip to content

Commit 0c8dcfc

Browse files
tom-pyteltom-pytel
committed
gh-129069: make list ass_slice and memory_repeat safe
1 parent 96ef4c5 commit 0c8dcfc

File tree

4 files changed

+62
-23
lines changed

4 files changed

+62
-23
lines changed

‎Include/internal/pycore_list.h‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,17 @@ _PyList_AppendTakeRef(PyListObject *self, PyObject *newitem)
4848
return _PyList_AppendTakeRefListResize(self, newitem);
4949
}
5050

51-
// Repeat the bytes of a buffer in place
51+
// Repeat the bytes of a buffer of pointers in place
5252
static inline void
53-
_Py_memory_repeat(char* dest, Py_ssize_t len_dest, Py_ssize_t len_src)
53+
_Py_memory_ptrs_repeat(char* dest, Py_ssize_t len_dest, Py_ssize_t len_src)
5454
{
5555
assert(len_src > 0);
56+
assert(len_src % sizeof(void *) == 0);
57+
assert(((uintptr_t)dest & (sizeof (void *) - 1)) == 0);
5658
Py_ssize_t copied = len_src;
5759
while (copied < len_dest) {
5860
Py_ssize_t bytes_to_copy = Py_MIN(copied, len_dest - copied);
59-
memcpy(dest + copied, dest, (size_t)bytes_to_copy);
61+
FT_ATOMIC_MEMCPY_PTR_RELAXED(dest + copied, dest, (size_t)bytes_to_copy);
6062
copied += bytes_to_copy;
6163
}
6264
}

‎Include/internal/pycore_pyatomic_ft_wrappers.h‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,48 @@ extern "C" {
110110
#define FT_ATOMIC_LOAD_ULLONG_RELAXED(value) \
111111
_Py_atomic_load_ullong_relaxed(&value)
112112

113+
static inline void *
114+
FT_ATOMIC_MEMCPY_PTR_RELAXED(void *dest, void *src, ssize_t n)
115+
{
116+
assert(((uintptr_t)dest & (sizeof (void *) - 1)) == 0);
117+
assert(((uintptr_t)src & (sizeof (void *) - 1)) == 0);
118+
assert(n % sizeof(void *) == 0);
119+
120+
if (dest != src) {
121+
for (void **d = (void **)dest, **s = (void **)src, **e = d + n / sizeof(void *); d != e; d++, s++) {
122+
void *v = _Py_atomic_load_ptr_relaxed(s);
123+
_Py_atomic_store_ptr_relaxed(d, v);
124+
}
125+
}
126+
127+
return dest;
128+
}
129+
130+
static inline void *
131+
FT_ATOMIC_MEMMOVE_PTR_RELAXED(void *dest, void *src, ssize_t n)
132+
{
133+
assert(((uintptr_t)dest & (sizeof (void *) - 1)) == 0);
134+
assert(((uintptr_t)src & (sizeof (void *) - 1)) == 0);
135+
assert(n % sizeof(void *) == 0);
136+
137+
if (dest < src || dest >= (void *)((char *)src + n)) { // prefer incrementing copy
138+
for (void **d = (void **)dest, **s = (void **)src, **e = d + n / sizeof(void *); d != e; d++, s++) {
139+
void *v = _Py_atomic_load_ptr_relaxed(s);
140+
_Py_atomic_store_ptr_relaxed(d, v);
141+
}
142+
}
143+
else if (dest > src) {
144+
n = n / sizeof(void *) - 1;
145+
for (void **d = (void **)dest + n, **s = (void **)src + n, **e = (void **)dest - 1; d != e; d--, s--) {
146+
void *v = _Py_atomic_load_ptr_relaxed(s);
147+
_Py_atomic_store_ptr_relaxed(d, v);
148+
}
149+
}
150+
151+
return dest;
152+
}
153+
154+
113155
#else
114156
#define FT_ATOMIC_LOAD_PTR(value) value
115157
#define FT_ATOMIC_STORE_PTR(value, new_value) value = new_value
@@ -157,6 +199,9 @@ extern "C" {
157199
#define FT_ATOMIC_LOAD_ULLONG_RELAXED(value) value
158200
#define FT_ATOMIC_STORE_ULLONG_RELAXED(value, new_value) value = new_value
159201

202+
#define FT_ATOMIC_MEMCPY_PTR_RELAXED(dest, src, n) memcpy(dest, src, n)
203+
#define FT_ATOMIC_MEMMOVE_PTR_RELAXED(dest, src, n) memmove(dest, src, n)
204+
160205
#endif
161206

162207
#ifdef __cplusplus

‎Objects/listobject.c‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -813,10 +813,8 @@ list_repeat_lock_held(PyListObject *a, Py_ssize_t n)
813813
_Py_RefcntAdd(*src, n);
814814
*dest++ = *src++;
815815
}
816-
// TODO: _Py_memory_repeat calls are not safe for shared lists in
817-
// GIL_DISABLED builds. (See issue #129069)
818-
_Py_memory_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size,
819-
sizeof(PyObject *)*input_size);
816+
_Py_memory_ptrs_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size,
817+
sizeof(PyObject *)*input_size);
820818
}
821819

822820
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
949947
if (d < 0) { /* Delete -d items */
950948
Py_ssize_t tail;
951949
tail = (Py_SIZE(a) - ihigh) * sizeof(PyObject *);
952-
// TODO: these memmove/memcpy calls are not safe for shared lists in
953-
// GIL_DISABLED builds. (See issue #129069)
954-
memmove(&item[ihigh+d], &item[ihigh], tail);
950+
FT_ATOMIC_MEMMOVE_PTR_RELAXED(&item[ihigh+d], &item[ihigh], tail);
955951
if (list_resize(a, Py_SIZE(a) + d) < 0) {
956-
memmove(&item[ihigh], &item[ihigh+d], tail);
957-
memcpy(&item[ilow], recycle, s);
952+
FT_ATOMIC_MEMMOVE_PTR_RELAXED(&item[ihigh], &item[ihigh+d], tail);
953+
FT_ATOMIC_MEMCPY_PTR_RELAXED(&item[ilow], recycle, s);
958954
goto Error;
959955
}
960956
item = a->ob_item;
@@ -964,10 +960,8 @@ list_ass_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyO
964960
if (list_resize(a, k+d) < 0)
965961
goto Error;
966962
item = a->ob_item;
967-
// TODO: these memmove/memcpy calls are not safe for shared lists in
968-
// GIL_DISABLED builds. (See issue #129069)
969-
memmove(&item[ihigh+d], &item[ihigh],
970-
(k - ihigh)*sizeof(PyObject *));
963+
FT_ATOMIC_MEMMOVE_PTR_RELAXED(&item[ihigh+d], &item[ihigh],
964+
(k - ihigh)*sizeof(PyObject *));
971965
}
972966
for (k = 0; k < n; k++, ilow++) {
973967
PyObject *w = vitem[k];
@@ -1051,10 +1045,8 @@ list_inplace_repeat_lock_held(PyListObject *self, Py_ssize_t n)
10511045
for (Py_ssize_t j = 0; j < input_size; j++) {
10521046
_Py_RefcntAdd(items[j], n-1);
10531047
}
1054-
// TODO: _Py_memory_repeat calls are not safe for shared lists in
1055-
// GIL_DISABLED builds. (See issue #129069)
1056-
_Py_memory_repeat((char *)items, sizeof(PyObject *)*output_size,
1057-
sizeof(PyObject *)*input_size);
1048+
_Py_memory_ptrs_repeat((char *)items, sizeof(PyObject *)*output_size,
1049+
sizeof(PyObject *)*input_size);
10581050
return 0;
10591051
}
10601052

‎Objects/tupleobject.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
#include "pycore_ceval.h" // _PyEval_GetBuiltin()
66
#include "pycore_freelist.h" // _Py_FREELIST_PUSH()
77
#include "pycore_gc.h" // _PyObject_GC_IS_TRACKED()
8-
#include "pycore_list.h" // _Py_memory_repeat()
8+
#include "pycore_list.h" // _Py_memory_ptrs_repeat()
99
#include "pycore_modsupport.h" // _PyArg_NoKwnames()
1010
#include "pycore_object.h" // _PyObject_GC_TRACK()
1111
#include "pycore_stackref.h" // PyStackRef_AsPyObjectSteal()
@@ -547,8 +547,8 @@ tuple_repeat(PyObject *self, Py_ssize_t n)
547547
*dest++ = *src++;
548548
}
549549

550-
_Py_memory_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size,
551-
sizeof(PyObject *)*input_size);
550+
_Py_memory_ptrs_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size,
551+
sizeof(PyObject *)*input_size);
552552
}
553553
_PyObject_GC_TRACK(np);
554554
return (PyObject *) np;

0 commit comments

Comments
 (0)