Skip to content

Commit cd747e5

Browse files
JukkaLJukkaL
authored
[mypyc] Make list get/set item more memory safe on free-threaded builds (#21683)
Use the CPython C API for list get item and set item, as direct item access is not memory safe in the presence of race conditions on a free-threaded Python. Note that these operations are not necessarily atomic -- this only fixes memory safety. Explicit synchronization is still generally expected for correctness. This still doesn't cover for loops over lists. I will fix them in a follow-up PR. Preserve existing optimized primitives on non-free-threaded builds. Work on mypyc/mypyc#1202.
1 parent 0f95654 commit cd747e5

4 files changed

Lines changed: 213 additions & 62 deletions

File tree

‎mypyc/lib-rt/CPy.h‎

Lines changed: 56 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -664,16 +664,68 @@ PyObject *CPyObject_GetSlice(PyObject *obj, CPyTagged start, CPyTagged end);
664664
// List operations
665665

666666

667-
PyObject *CPyList_Build(Py_ssize_t len, ...);
667+
#ifndef Py_GIL_DISABLED
668+
668669
PyObject *CPyList_GetItem(PyObject *list, CPyTagged index);
669670
PyObject *CPyList_GetItemShort(PyObject *list, CPyTagged index);
671+
PyObject *CPyList_GetItemInt64(PyObject *list, int64_t index);
672+
bool CPyList_SetItem(PyObject *list, CPyTagged index, PyObject *value);
673+
bool CPyList_SetItemInt64(PyObject *list, int64_t index, PyObject *value);
674+
675+
#else
676+
677+
PyObject *CPyList_GetItem_(PyObject *list, CPyTagged index);
678+
bool CPyList_SetItem_(PyObject *list, CPyTagged index, PyObject *value);
679+
680+
static inline PyObject *CPyList_GetItem(PyObject *list, CPyTagged index) {
681+
if (likely(CPyTagged_CheckShort(index) && !CPyTagged_IsNegative(index))) {
682+
// Inlined fast path
683+
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
684+
return PyList_GetItemRef(list, n);
685+
} else {
686+
return CPyList_GetItem_(list, index);
687+
}
688+
}
689+
690+
static inline PyObject *CPyList_GetItemShort(PyObject *list, CPyTagged index) {
691+
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
692+
if (n < 0) {
693+
n += PyList_GET_SIZE(list);
694+
}
695+
return PyList_GetItemRef(list, n);
696+
}
697+
698+
static inline PyObject *CPyList_GetItemInt64(PyObject *list, int64_t index) {
699+
if (index < 0) {
700+
index += PyList_GET_SIZE(list);
701+
}
702+
return PyList_GetItemRef(list, index);
703+
}
704+
705+
static inline bool CPyList_SetItem(PyObject *list, CPyTagged index, PyObject *value) {
706+
if (likely(CPyTagged_CheckShort(index) && !CPyTagged_IsNegative(index))) {
707+
// Inlined fast path
708+
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
709+
return PyList_SetItem(list, n, value) >= 0;
710+
} else {
711+
return CPyList_SetItem_(list, index, value);
712+
}
713+
}
714+
715+
static inline bool CPyList_SetItemInt64(PyObject *list, int64_t index, PyObject *value) {
716+
if (index < 0) {
717+
index += PyList_GET_SIZE(list);
718+
}
719+
return PyList_SetItem(list, index, value) >= 0;
720+
}
721+
722+
#endif
723+
670724
PyObject *CPyList_GetItemBorrow(PyObject *list, CPyTagged index);
671725
PyObject *CPyList_GetItemShortBorrow(PyObject *list, CPyTagged index);
672-
PyObject *CPyList_GetItemInt64(PyObject *list, int64_t index);
673726
PyObject *CPyList_GetItemInt64Borrow(PyObject *list, int64_t index);
674-
bool CPyList_SetItem(PyObject *list, CPyTagged index, PyObject *value);
675727
void CPyList_SetItemUnsafe(PyObject *list, Py_ssize_t index, PyObject *value);
676-
bool CPyList_SetItemInt64(PyObject *list, int64_t index, PyObject *value);
728+
PyObject *CPyList_Build(Py_ssize_t len, ...);
677729
PyObject *CPyList_PopLast(PyObject *obj);
678730
PyObject *CPyList_Pop(PyObject *obj, CPyTagged index);
679731
CPyTagged CPyList_Count(PyObject *obj, PyObject *value);

‎mypyc/lib-rt/list_ops.c‎

Lines changed: 91 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ PyObject *CPyList_Copy(PyObject *list) {
4949
return PyObject_CallMethodNoArgs(list, mypyc_interned_str.copy);
5050
}
5151

52+
#ifndef Py_GIL_DISABLED
53+
5254
PyObject *CPyList_GetItemShort(PyObject *list, CPyTagged index) {
5355
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
5456
Py_ssize_t size = PyList_GET_SIZE(list);
@@ -69,24 +71,6 @@ PyObject *CPyList_GetItemShort(PyObject *list, CPyTagged index) {
6971
return result;
7072
}
7173

72-
PyObject *CPyList_GetItemShortBorrow(PyObject *list, CPyTagged index) {
73-
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
74-
Py_ssize_t size = PyList_GET_SIZE(list);
75-
if (n >= 0) {
76-
if (n >= size) {
77-
PyErr_SetString(PyExc_IndexError, "list index out of range");
78-
return NULL;
79-
}
80-
} else {
81-
n += size;
82-
if (n < 0) {
83-
PyErr_SetString(PyExc_IndexError, "list index out of range");
84-
return NULL;
85-
}
86-
}
87-
return PyList_GET_ITEM(list, n);
88-
}
89-
9074
PyObject *CPyList_GetItem(PyObject *list, CPyTagged index) {
9175
if (CPyTagged_CheckShort(index)) {
9276
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
@@ -112,29 +96,6 @@ PyObject *CPyList_GetItem(PyObject *list, CPyTagged index) {
11296
}
11397
}
11498

115-
PyObject *CPyList_GetItemBorrow(PyObject *list, CPyTagged index) {
116-
if (CPyTagged_CheckShort(index)) {
117-
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
118-
Py_ssize_t size = PyList_GET_SIZE(list);
119-
if (n >= 0) {
120-
if (n >= size) {
121-
PyErr_SetString(PyExc_IndexError, "list index out of range");
122-
return NULL;
123-
}
124-
} else {
125-
n += size;
126-
if (n < 0) {
127-
PyErr_SetString(PyExc_IndexError, "list index out of range");
128-
return NULL;
129-
}
130-
}
131-
return PyList_GET_ITEM(list, n);
132-
} else {
133-
PyErr_SetString(PyExc_OverflowError, CPYTHON_LARGE_INT_ERRMSG);
134-
return NULL;
135-
}
136-
}
137-
13899
PyObject *CPyList_GetItemInt64(PyObject *list, int64_t index) {
139100
size_t size = PyList_GET_SIZE(list);
140101
if (likely((uint64_t)index < size)) {
@@ -156,23 +117,6 @@ PyObject *CPyList_GetItemInt64(PyObject *list, int64_t index) {
156117
return result;
157118
}
158119

159-
PyObject *CPyList_GetItemInt64Borrow(PyObject *list, int64_t index) {
160-
size_t size = PyList_GET_SIZE(list);
161-
if (likely((uint64_t)index < size)) {
162-
return PyList_GET_ITEM(list, index);
163-
}
164-
if (index >= 0) {
165-
PyErr_SetString(PyExc_IndexError, "list index out of range");
166-
return NULL;
167-
}
168-
index += size;
169-
if (index < 0) {
170-
PyErr_SetString(PyExc_IndexError, "list index out of range");
171-
return NULL;
172-
}
173-
return PyList_GET_ITEM(list, index);
174-
}
175-
176120
bool CPyList_SetItem(PyObject *list, CPyTagged index, PyObject *value) {
177121
if (CPyTagged_CheckShort(index)) {
178122
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
@@ -220,6 +164,95 @@ bool CPyList_SetItemInt64(PyObject *list, int64_t index, PyObject *value) {
220164
return true;
221165
}
222166

167+
#else /* Py_GIL_DISABLED */
168+
169+
PyObject *CPyList_GetItem_(PyObject *list, CPyTagged index) {
170+
if (CPyTagged_CheckShort(index)) {
171+
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
172+
if (n < 0) {
173+
n += PyList_GET_SIZE(list);
174+
}
175+
return PyList_GetItemRef(list, n);
176+
} else {
177+
PyErr_SetString(PyExc_OverflowError, CPYTHON_LARGE_INT_ERRMSG);
178+
return NULL;
179+
}
180+
}
181+
182+
bool CPyList_SetItem_(PyObject *list, CPyTagged index, PyObject *value) {
183+
if (CPyTagged_CheckShort(index)) {
184+
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
185+
Py_ssize_t size = PyList_GET_SIZE(list);
186+
if (n < 0) {
187+
n += size;
188+
}
189+
return PyList_SetItem(list, n, value) >= 0;
190+
} else {
191+
PyErr_SetString(PyExc_OverflowError, CPYTHON_LARGE_INT_ERRMSG);
192+
return false;
193+
}
194+
}
195+
196+
#endif
197+
198+
PyObject *CPyList_GetItemShortBorrow(PyObject *list, CPyTagged index) {
199+
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
200+
Py_ssize_t size = PyList_GET_SIZE(list);
201+
if (n >= 0) {
202+
if (n >= size) {
203+
PyErr_SetString(PyExc_IndexError, "list index out of range");
204+
return NULL;
205+
}
206+
} else {
207+
n += size;
208+
if (n < 0) {
209+
PyErr_SetString(PyExc_IndexError, "list index out of range");
210+
return NULL;
211+
}
212+
}
213+
return PyList_GET_ITEM(list, n);
214+
}
215+
216+
PyObject *CPyList_GetItemBorrow(PyObject *list, CPyTagged index) {
217+
if (CPyTagged_CheckShort(index)) {
218+
Py_ssize_t n = CPyTagged_ShortAsSsize_t(index);
219+
Py_ssize_t size = PyList_GET_SIZE(list);
220+
if (n >= 0) {
221+
if (n >= size) {
222+
PyErr_SetString(PyExc_IndexError, "list index out of range");
223+
return NULL;
224+
}
225+
} else {
226+
n += size;
227+
if (n < 0) {
228+
PyErr_SetString(PyExc_IndexError, "list index out of range");
229+
return NULL;
230+
}
231+
}
232+
return PyList_GET_ITEM(list, n);
233+
} else {
234+
PyErr_SetString(PyExc_OverflowError, CPYTHON_LARGE_INT_ERRMSG);
235+
return NULL;
236+
}
237+
}
238+
239+
PyObject *CPyList_GetItemInt64Borrow(PyObject *list, int64_t index) {
240+
size_t size = PyList_GET_SIZE(list);
241+
if (likely((uint64_t)index < size)) {
242+
return PyList_GET_ITEM(list, index);
243+
}
244+
if (index >= 0) {
245+
PyErr_SetString(PyExc_IndexError, "list index out of range");
246+
return NULL;
247+
}
248+
index += size;
249+
if (index < 0) {
250+
PyErr_SetString(PyExc_IndexError, "list index out of range");
251+
return NULL;
252+
}
253+
return PyList_GET_ITEM(list, index);
254+
}
255+
223256
// This function should only be used to fill in brand new lists.
224257
void CPyList_SetItemUnsafe(PyObject *list, Py_ssize_t index, PyObject *value) {
225258
PyList_SET_ITEM(list, index, value);

‎mypyc/lib-rt/mypyc_util.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,10 @@ static inline CPyTagged CPyTagged_ShortFromSsize_t(Py_ssize_t x) {
156156
return x << 1;
157157
}
158158

159+
static inline int CPyTagged_IsNegative(CPyTagged x) {
160+
return ((Py_ssize_t)x) < 0;
161+
}
162+
159163
// Are we targeting Python 3.X or newer?
160164
#define CPY_3_11_FEATURES (PY_VERSION_HEX >= 0x030b0000)
161165
#define CPY_3_12_FEATURES (PY_VERSION_HEX >= 0x030c0000)

‎mypyc/test-data/run-lists.test‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,68 @@ def test_list_build() -> None:
171171
l3.append('a')
172172
assert l3 == ['a']
173173

174+
class C:
175+
def __init__(self, s: str) -> None:
176+
self.s = s
177+
178+
a = [C("a"), C("b"), C("c")]
179+
180+
def test_get_item() -> None:
181+
zero = int()
182+
assert a[zero].s == "a"
183+
assert a[zero + 1].s == "b"
184+
assert a[zero + 2].s == "c"
185+
assert a[zero - 1].s == "c"
186+
assert a[zero - 2].s == "b"
187+
assert a[zero - 3].s == "a"
188+
with assertRaises(IndexError):
189+
a[zero + 3]
190+
with assertRaises(IndexError):
191+
a[zero - 4]
192+
# TODO: Raise IndexError?
193+
with assertRaises(OverflowError):
194+
a[zero + 2**90]
195+
with assertRaises(OverflowError):
196+
a[zero - 2**90]
197+
198+
def test_get_item_literal() -> None:
199+
assert a[0].s == "a"
200+
assert a[1].s == "b"
201+
assert a[2].s == "c"
202+
assert a[-1].s == "c"
203+
assert a[-2].s == "b"
204+
assert a[-3].s == "a"
205+
with assertRaises(IndexError):
206+
a[3]
207+
with assertRaises(IndexError):
208+
a[-4]
209+
# TODO: Raise IndexError?
210+
with assertRaises(OverflowError):
211+
a[2**90]
212+
with assertRaises(OverflowError):
213+
a[-2**90]
214+
215+
def test_set_item() -> None:
216+
l = a.copy()
217+
zero = int()
218+
zero2 = int()
219+
l[zero] = C("x")
220+
assert l[zero2].s == "x"
221+
l[zero + 2] = C("y")
222+
assert l[zero2 + 2].s == "y"
223+
l[zero - 1] = C("t")
224+
assert l[zero2 - 1].s == "t"
225+
l[zero - 3] = C("u")
226+
assert l[zero2 - 3].s == "u"
227+
with assertRaises(IndexError):
228+
a[zero + 3] = C("z")
229+
with assertRaises(IndexError):
230+
a[zero - 4] = C("z")
231+
with assertRaises(OverflowError):
232+
a[zero + 2**90] = C("z")
233+
with assertRaises(OverflowError):
234+
a[zero - 2**90] = C("z")
235+
174236
def test_append() -> None:
175237
l = [1, 2]
176238
l.append(10)

0 commit comments

Comments
 (0)