From d9ffc53859dcd64289a858e9312289f21026b250 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 2 Mar 2024 09:59:55 +0900 Subject: [PATCH 01/12] gh-112087: Make list_{slice, ass_slice, subscript} to be threadsafe --- Objects/listobject.c | 147 ++++++++++++++++++++++++++----------------- 1 file changed, 91 insertions(+), 56 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 87effb1b3a65fa..f264c21baeda5c 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -515,29 +515,32 @@ list_item(PyObject *aa, Py_ssize_t i) } static PyObject * -list_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) +list_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t len) { - PyListObject *np; - PyObject **src, **dest; - Py_ssize_t i, len; - len = ihigh - ilow; - if (len <= 0) { - return PyList_New(0); - } - np = (PyListObject *) list_new_prealloc(len); - if (np == NULL) + PyListObject *np = (PyListObject *) list_new_prealloc(len); + if (np == NULL) { return NULL; - - src = a->ob_item + ilow; - dest = np->ob_item; - for (i = 0; i < len; i++) { + } + PyObject **src = a->ob_item + ilow; + PyObject **dest = np->ob_item; + for (Py_ssize_t i = 0; i < len; i++) { PyObject *v = src[i]; - dest[i] = Py_NewRef(v); + FT_ATOMIC_STORE_PTR_RELAXED(dest[i], Py_NewRef(v)); } Py_SET_SIZE(np, len); return (PyObject *)np; } +static PyObject * +list_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) +{ + Py_ssize_t len = ihigh - ilow; + if (len <= 0) { + return PyList_New(0); + } + return list_slice_lock_held(a, ilow, len); +} + PyObject * PyList_GetSlice(PyObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) { @@ -718,7 +721,7 @@ list_clear_slot(PyObject *self) * guaranteed the call cannot fail. */ static int -list_ass_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) +list_ass_slice_impl(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) { /* Because [X]DECREF can recursively invoke list operations on this list, we must postpone all [X]DECREF activity until @@ -741,15 +744,6 @@ list_ass_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) if (v == NULL) n = 0; else { - if (a == b) { - /* Special case "a[i:j] = a" -- copy b first */ - v = list_slice(b, 0, Py_SIZE(b)); - if (v == NULL) - return result; - result = list_ass_slice(a, ilow, ihigh, v); - Py_DECREF(v); - return result; - } v_as_SF = PySequence_Fast(v, "can only assign an iterable"); if(v_as_SF == NULL) goto Error; @@ -823,6 +817,34 @@ list_ass_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) #undef b } +static int +list_ass_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) +{ + int ret; + if (a == (PyListObject *)v) { + Py_BEGIN_CRITICAL_SECTION(a); + Py_ssize_t n = PyList_GET_SIZE(a); + PyObject *copy = list_slice_lock_held(a, 0, n); + if (copy == NULL) { + return -1; + } + ret = list_ass_slice_impl(a, ilow, ihigh, copy); + Py_DECREF(copy); + Py_END_CRITICAL_SECTION(); + } + else if (v != NULL && PyList_CheckExact(v)) { + Py_BEGIN_CRITICAL_SECTION2(a, v); + ret = list_ass_slice_impl(a, ilow, ihigh, v); + Py_END_CRITICAL_SECTION2(); + } + else { + Py_BEGIN_CRITICAL_SECTION(a); + ret = list_ass_slice_impl(a, ilow, ihigh, v); + Py_END_CRITICAL_SECTION(); + } + return ret; +} + int PyList_SetSlice(PyObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) { @@ -2884,8 +2906,7 @@ list_remove_impl(PyListObject *self, PyObject *value) int cmp = PyObject_RichCompareBool(obj, value, Py_EQ); Py_DECREF(obj); if (cmp > 0) { - if (list_ass_slice(self, i, i+1, - (PyObject *)NULL) == 0) + if (list_ass_slice_impl(self, i, i+1, NULL) == 0) Py_RETURN_NONE; return NULL; } @@ -3085,6 +3106,47 @@ static PySequenceMethods list_as_sequence = { list_inplace_repeat, /* sq_inplace_repeat */ }; +static PyObject * +list_slice_step_lock_held(PyListObject *a, Py_ssize_t start, Py_ssize_t step, Py_ssize_t len) +{ + PyObject *obj = list_new_prealloc(len); + if (obj == NULL) { + return NULL; + } + PyListObject *np = (PyListObject *)obj; + size_t cur; + Py_ssize_t i; + PyObject **src = a->ob_item; + PyObject **dest = np->ob_item; + for (cur = start, i = 0; i < len; + cur += (size_t)step, i++) { + PyObject *v = src[cur]; + FT_ATOMIC_STORE_PTR_RELAXED(dest[i], Py_NewRef(v)); + } + Py_SET_SIZE(np, len); + return (PyObject *)np; +} + +static PyObject * +list_slice_wrap(PyListObject *aa, Py_ssize_t start, Py_ssize_t stop, Py_ssize_t step) +{ + PyObject *res = NULL; + PyListObject *a = (PyListObject *)aa; + Py_BEGIN_CRITICAL_SECTION(a); + Py_ssize_t len = PySlice_AdjustIndices(Py_SIZE(a), &start, &stop, step); + if (len <= 0) { + res = PyList_New(0); + } + else if (step == 1) { + res = list_slice_lock_held(a, start, len); + } + else { + res = list_slice_step_lock_held(a, start, step, len); + } + Py_END_CRITICAL_SECTION(); + return res; +} + static PyObject * list_subscript(PyObject* _self, PyObject* item) { @@ -3099,38 +3161,11 @@ list_subscript(PyObject* _self, PyObject* item) return list_item((PyObject *)self, i); } else if (PySlice_Check(item)) { - Py_ssize_t start, stop, step, slicelength, i; - size_t cur; - PyObject* result; - PyObject* it; - PyObject **src, **dest; - + Py_ssize_t start, stop, step; if (PySlice_Unpack(item, &start, &stop, &step) < 0) { return NULL; } - slicelength = PySlice_AdjustIndices(Py_SIZE(self), &start, &stop, - step); - - if (slicelength <= 0) { - return PyList_New(0); - } - else if (step == 1) { - return list_slice(self, start, stop); - } - else { - result = list_new_prealloc(slicelength); - if (!result) return NULL; - - src = self->ob_item; - dest = ((PyListObject *)result)->ob_item; - for (cur = start, i = 0; i < slicelength; - cur += (size_t)step, i++) { - it = Py_NewRef(src[cur]); - dest[i] = it; - } - Py_SET_SIZE(result, slicelength); - return result; - } + return list_slice_wrap(self, start, stop, step); } else { PyErr_Format(PyExc_TypeError, From cab2d99b4859b6d01cb6528b5c67e02d9ecda0e0 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 08:09:35 +0900 Subject: [PATCH 02/12] Remove unnecessary atomic operations --- Objects/listobject.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index f264c21baeda5c..bbd6697392c350 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -514,8 +514,8 @@ list_item(PyObject *aa, Py_ssize_t i) return item; } -static PyObject * -list_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t len) +static inline PyObject * +list_slice_impl(PyListObject *a, Py_ssize_t ilow, Py_ssize_t len) { PyListObject *np = (PyListObject *) list_new_prealloc(len); if (np == NULL) { @@ -525,7 +525,7 @@ list_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t len) PyObject **dest = np->ob_item; for (Py_ssize_t i = 0; i < len; i++) { PyObject *v = src[i]; - FT_ATOMIC_STORE_PTR_RELAXED(dest[i], Py_NewRef(v)); + dest[i] = Py_NewRef(v); } Py_SET_SIZE(np, len); return (PyObject *)np; @@ -538,7 +538,7 @@ list_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) if (len <= 0) { return PyList_New(0); } - return list_slice_lock_held(a, ilow, len); + return list_slice_impl(a, ilow, len); } PyObject * @@ -587,13 +587,13 @@ list_concat_lock_held(PyListObject *a, PyListObject *b) dest = np->ob_item; for (i = 0; i < Py_SIZE(a); i++) { PyObject *v = src[i]; - FT_ATOMIC_STORE_PTR_RELAXED(dest[i], Py_NewRef(v)); + dest[i] = Py_NewRef(v); } src = b->ob_item; dest = np->ob_item + Py_SIZE(a); for (i = 0; i < Py_SIZE(b); i++) { PyObject *v = src[i]; - FT_ATOMIC_STORE_PTR_RELAXED(dest[i], Py_NewRef(v)); + dest[i] = Py_NewRef(v); } Py_SET_SIZE(np, size); return (PyObject *)np; @@ -639,7 +639,7 @@ list_repeat_lock_held(PyListObject *a, Py_ssize_t n) _Py_RefcntAdd(elem, n); PyObject **dest_end = dest + output_size; while (dest < dest_end) { - FT_ATOMIC_STORE_PTR_RELAXED(*dest++, elem); + *dest++ = elem; } } else { @@ -647,7 +647,7 @@ list_repeat_lock_held(PyListObject *a, Py_ssize_t n) PyObject **src_end = src + input_size; while (src < src_end) { _Py_RefcntAdd(*src, n); - FT_ATOMIC_STORE_PTR_RELAXED(*dest++, *src++); + *dest++ = *src++; } _Py_memory_repeat((char *)np->ob_item, sizeof(PyObject *)*output_size, @@ -824,7 +824,7 @@ list_ass_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) if (a == (PyListObject *)v) { Py_BEGIN_CRITICAL_SECTION(a); Py_ssize_t n = PyList_GET_SIZE(a); - PyObject *copy = list_slice_lock_held(a, 0, n); + PyObject *copy = list_slice_impl(a, 0, n); if (copy == NULL) { return -1; } @@ -3138,7 +3138,7 @@ list_slice_wrap(PyListObject *aa, Py_ssize_t start, Py_ssize_t stop, Py_ssize_t res = PyList_New(0); } else if (step == 1) { - res = list_slice_lock_held(a, start, len); + res = list_slice_impl(a, start, len); } else { res = list_slice_step_lock_held(a, start, step, len); From 8b0120d2e61dcfa3dae06c370dfabce71f4dee2b Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 08:14:54 +0900 Subject: [PATCH 03/12] Update --- Objects/listobject.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index bbd6697392c350..337efd43fcf449 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3106,14 +3106,13 @@ static PySequenceMethods list_as_sequence = { list_inplace_repeat, /* sq_inplace_repeat */ }; -static PyObject * -list_slice_step_lock_held(PyListObject *a, Py_ssize_t start, Py_ssize_t step, Py_ssize_t len) +static inline PyObject * +list_slice_step_impl(PyListObject *a, Py_ssize_t start, Py_ssize_t step, Py_ssize_t len) { - PyObject *obj = list_new_prealloc(len); - if (obj == NULL) { + PyListObject *np = (PyListObject *)list_new_prealloc(len); + if (np == NULL) { return NULL; } - PyListObject *np = (PyListObject *)obj; size_t cur; Py_ssize_t i; PyObject **src = a->ob_item; @@ -3121,7 +3120,7 @@ list_slice_step_lock_held(PyListObject *a, Py_ssize_t start, Py_ssize_t step, Py for (cur = start, i = 0; i < len; cur += (size_t)step, i++) { PyObject *v = src[cur]; - FT_ATOMIC_STORE_PTR_RELAXED(dest[i], Py_NewRef(v)); + dest[i] = Py_NewRef(v); } Py_SET_SIZE(np, len); return (PyObject *)np; @@ -3141,7 +3140,7 @@ list_slice_wrap(PyListObject *aa, Py_ssize_t start, Py_ssize_t stop, Py_ssize_t res = list_slice_impl(a, start, len); } else { - res = list_slice_step_lock_held(a, start, step, len); + res = list_slice_step_impl(a, start, step, len); } Py_END_CRITICAL_SECTION(); return res; From 1e19253357fedc9e0f56bb8148e129d6268cd138 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 09:21:31 +0900 Subject: [PATCH 04/12] Address code review --- Objects/listobject.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 337efd43fcf449..766c1a10bf6f4e 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -515,7 +515,7 @@ list_item(PyObject *aa, Py_ssize_t i) } static inline PyObject * -list_slice_impl(PyListObject *a, Py_ssize_t ilow, Py_ssize_t len) +list_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t len) { PyListObject *np = (PyListObject *) list_new_prealloc(len); if (np == NULL) { @@ -538,7 +538,11 @@ list_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) if (len <= 0) { return PyList_New(0); } - return list_slice_impl(a, ilow, len); + PyObject *ret; + Py_BEGIN_CRITICAL_SECTION(a); + ret = list_slice_lock_held(a, ilow, len); + Py_END_CRITICAL_SECTION(); + return ret; } PyObject * @@ -824,7 +828,7 @@ list_ass_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) if (a == (PyListObject *)v) { Py_BEGIN_CRITICAL_SECTION(a); Py_ssize_t n = PyList_GET_SIZE(a); - PyObject *copy = list_slice_impl(a, 0, n); + PyObject *copy = list_slice_lock_held(a, 0, n); if (copy == NULL) { return -1; } @@ -978,7 +982,11 @@ static PyObject * list_copy_impl(PyListObject *self) /*[clinic end generated code: output=ec6b72d6209d418e input=81c54b0c7bb4f73d]*/ { - return list_slice(self, 0, Py_SIZE(self)); + Py_ssize_t n = Py_SIZE(self); + if (n <= 0) { + return (PyObject *) PyList_New(0); + } + return list_slice_lock_held(self, 0, n); } /*[clinic input] @@ -3107,7 +3115,7 @@ static PySequenceMethods list_as_sequence = { }; static inline PyObject * -list_slice_step_impl(PyListObject *a, Py_ssize_t start, Py_ssize_t step, Py_ssize_t len) +list_slice_step_lock_held(PyListObject *a, Py_ssize_t start, Py_ssize_t step, Py_ssize_t len) { PyListObject *np = (PyListObject *)list_new_prealloc(len); if (np == NULL) { @@ -3137,10 +3145,10 @@ list_slice_wrap(PyListObject *aa, Py_ssize_t start, Py_ssize_t stop, Py_ssize_t res = PyList_New(0); } else if (step == 1) { - res = list_slice_impl(a, start, len); + res = list_slice_lock_held(a, start, len); } else { - res = list_slice_step_impl(a, start, step, len); + res = list_slice_step_lock_held(a, start, step, len); } Py_END_CRITICAL_SECTION(); return res; @@ -3275,8 +3283,14 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value) /* protect against a[::-1] = a */ if (self == (PyListObject*)value) { - seq = list_slice((PyListObject*)value, 0, - PyList_GET_SIZE(value)); + Py_BEGIN_CRITICAL_SECTION(value); + Py_ssize_t size = Py_SIZE(value); + if (size <= 0) { + seq = PyList_New(0); + } else { + seq = list_slice_lock_held((PyListObject*)value, 0, size); + } + Py_END_CRITICAL_SECTION(); } else { seq = PySequence_Fast(value, From c25ca795b834e7e9778130cf2a884b7416cefe5d Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 09:23:02 +0900 Subject: [PATCH 05/12] nit --- Objects/listobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 766c1a10bf6f4e..9c92d77ec2ceab 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -984,7 +984,7 @@ list_copy_impl(PyListObject *self) { Py_ssize_t n = Py_SIZE(self); if (n <= 0) { - return (PyObject *) PyList_New(0); + return (PyObject *)PyList_New(0); } return list_slice_lock_held(self, 0, n); } From 1ed33293449f7c5ef78786476e9d886e5fcef226 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 09:34:50 +0900 Subject: [PATCH 06/12] Change name --- Objects/listobject.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 9c92d77ec2ceab..1d9f4b23fb7903 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -725,7 +725,7 @@ list_clear_slot(PyObject *self) * guaranteed the call cannot fail. */ static int -list_ass_slice_impl(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) +list_ass_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) { /* Because [X]DECREF can recursively invoke list operations on this list, we must postpone all [X]DECREF activity until @@ -832,18 +832,18 @@ list_ass_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v) if (copy == NULL) { return -1; } - ret = list_ass_slice_impl(a, ilow, ihigh, copy); + ret = list_ass_slice_lock_held(a, ilow, ihigh, copy); Py_DECREF(copy); Py_END_CRITICAL_SECTION(); } else if (v != NULL && PyList_CheckExact(v)) { Py_BEGIN_CRITICAL_SECTION2(a, v); - ret = list_ass_slice_impl(a, ilow, ihigh, v); + ret = list_ass_slice_lock_held(a, ilow, ihigh, v); Py_END_CRITICAL_SECTION2(); } else { Py_BEGIN_CRITICAL_SECTION(a); - ret = list_ass_slice_impl(a, ilow, ihigh, v); + ret = list_ass_slice_lock_held(a, ilow, ihigh, v); Py_END_CRITICAL_SECTION(); } return ret; @@ -2914,7 +2914,7 @@ list_remove_impl(PyListObject *self, PyObject *value) int cmp = PyObject_RichCompareBool(obj, value, Py_EQ); Py_DECREF(obj); if (cmp > 0) { - if (list_ass_slice_impl(self, i, i+1, NULL) == 0) + if (list_ass_slice_lock_held(self, i, i+1, NULL) == 0) Py_RETURN_NONE; return NULL; } From 033c14194c5c02da0e0fb4f3b6cb7c4f38bc5937 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 09:40:07 +0900 Subject: [PATCH 07/12] Fix --- Objects/listobject.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 1d9f4b23fb7903..5b087bef879c84 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -531,20 +531,6 @@ list_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t len) return (PyObject *)np; } -static PyObject * -list_slice(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) -{ - Py_ssize_t len = ihigh - ilow; - if (len <= 0) { - return PyList_New(0); - } - PyObject *ret; - Py_BEGIN_CRITICAL_SECTION(a); - ret = list_slice_lock_held(a, ilow, len); - Py_END_CRITICAL_SECTION(); - return ret; -} - PyObject * PyList_GetSlice(PyObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) { @@ -566,7 +552,12 @@ PyList_GetSlice(PyObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) else if (ihigh > Py_SIZE(a)) { ihigh = Py_SIZE(a); } - ret = list_slice((PyListObject *)a, ilow, ihigh); + if ((ihigh - ilow) <= 0) { + ret = PyList_New(0); + } + else { + ret = list_slice_lock_held((PyListObject *)a, ilow, ihigh); + } Py_END_CRITICAL_SECTION(); return ret; } From 12529a899f43b71723577cd8e8b58df0aacf2911 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 09:45:51 +0900 Subject: [PATCH 08/12] Remove inline --- Objects/listobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 5b087bef879c84..b5ef436d09edaa 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -514,7 +514,7 @@ list_item(PyObject *aa, Py_ssize_t i) return item; } -static inline PyObject * +static PyObject * list_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t len) { PyListObject *np = (PyListObject *) list_new_prealloc(len); From 33352111adcdeb35830fe15770ea93dbfdec3739 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 10:52:59 +0900 Subject: [PATCH 09/12] fix --- Objects/listobject.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index b5ef436d09edaa..a96c6725cf656a 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -552,11 +552,12 @@ PyList_GetSlice(PyObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) else if (ihigh > Py_SIZE(a)) { ihigh = Py_SIZE(a); } - if ((ihigh - ilow) <= 0) { + Py_ssize_t len = ihigh - ilow; + if (len <= 0) { ret = PyList_New(0); } else { - ret = list_slice_lock_held((PyListObject *)a, ilow, ihigh); + ret = list_slice_lock_held((PyListObject *)a, ilow, len); } Py_END_CRITICAL_SECTION(); return ret; From 11b3017b5eded28409d699d101ba0f8e9a69867b Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 12:25:17 +0900 Subject: [PATCH 10/12] Address code review --- Objects/listobject.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index a96c6725cf656a..bf10375cff691b 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -515,15 +515,22 @@ list_item(PyObject *aa, Py_ssize_t i) } static PyObject * -list_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t len) +list_slice_lock_held(PyListObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) { - PyListObject *np = (PyListObject *) list_new_prealloc(len); - if (np == NULL) { - return NULL; + PyListObject *np; + PyObject **src, **dest; + Py_ssize_t i, len; + len = ihigh - ilow; + if (len <= 0) { + return PyList_New(0); } - PyObject **src = a->ob_item + ilow; - PyObject **dest = np->ob_item; - for (Py_ssize_t i = 0; i < len; i++) { + np = (PyListObject *) list_new_prealloc(len); + if (np == NULL) + return NULL; + + src = a->ob_item + ilow; + dest = np->ob_item; + for (i = 0; i < len; i++) { PyObject *v = src[i]; dest[i] = Py_NewRef(v); } @@ -552,13 +559,7 @@ PyList_GetSlice(PyObject *a, Py_ssize_t ilow, Py_ssize_t ihigh) else if (ihigh > Py_SIZE(a)) { ihigh = Py_SIZE(a); } - Py_ssize_t len = ihigh - ilow; - if (len <= 0) { - ret = PyList_New(0); - } - else { - ret = list_slice_lock_held((PyListObject *)a, ilow, len); - } + ret = list_slice_lock_held((PyListObject *)a, ilow, ihigh); Py_END_CRITICAL_SECTION(); return ret; } @@ -3137,7 +3138,16 @@ list_slice_wrap(PyListObject *aa, Py_ssize_t start, Py_ssize_t stop, Py_ssize_t res = PyList_New(0); } else if (step == 1) { - res = list_slice_lock_held(a, start, len); + res = list_new_prealloc(len); + if (!res) { + return NULL; + } + PyObject **src = aa->ob_item; + PyObject **dest = ((PyListObject *)res)->ob_item; + for (Py_ssize_t cur = start, i = 0; i < len; cur += (size_t)step, i++) { + dest[i] = Py_NewRef(src[cur]); + } + Py_SET_SIZE(res, len); } else { res = list_slice_step_lock_held(a, start, step, len); From 9ac76cadb58a94446573e0cdc269eb27977c3e4e Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 13:13:23 +0900 Subject: [PATCH 11/12] Address code review --- Objects/listobject.c | 32 +++++++------------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index bf10375cff691b..79d61878c381bc 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -975,11 +975,7 @@ static PyObject * list_copy_impl(PyListObject *self) /*[clinic end generated code: output=ec6b72d6209d418e input=81c54b0c7bb4f73d]*/ { - Py_ssize_t n = Py_SIZE(self); - if (n <= 0) { - return (PyObject *)PyList_New(0); - } - return list_slice_lock_held(self, 0, n); + return list_slice_lock_held(self, 0, Py_SIZE(self)); } /*[clinic input] @@ -3131,26 +3127,16 @@ static PyObject * list_slice_wrap(PyListObject *aa, Py_ssize_t start, Py_ssize_t stop, Py_ssize_t step) { PyObject *res = NULL; - PyListObject *a = (PyListObject *)aa; - Py_BEGIN_CRITICAL_SECTION(a); - Py_ssize_t len = PySlice_AdjustIndices(Py_SIZE(a), &start, &stop, step); + Py_BEGIN_CRITICAL_SECTION(aa); + Py_ssize_t len = PySlice_AdjustIndices(Py_SIZE(aa), &start, &stop, step); if (len <= 0) { res = PyList_New(0); } else if (step == 1) { - res = list_new_prealloc(len); - if (!res) { - return NULL; - } - PyObject **src = aa->ob_item; - PyObject **dest = ((PyListObject *)res)->ob_item; - for (Py_ssize_t cur = start, i = 0; i < len; cur += (size_t)step, i++) { - dest[i] = Py_NewRef(src[cur]); - } - Py_SET_SIZE(res, len); + res = list_slice_lock_held(aa, start, stop); } else { - res = list_slice_step_lock_held(a, start, step, len); + res = list_slice_step_lock_held(aa, start, step, len); } Py_END_CRITICAL_SECTION(); return res; @@ -3286,12 +3272,8 @@ list_ass_subscript(PyObject* _self, PyObject* item, PyObject* value) /* protect against a[::-1] = a */ if (self == (PyListObject*)value) { Py_BEGIN_CRITICAL_SECTION(value); - Py_ssize_t size = Py_SIZE(value); - if (size <= 0) { - seq = PyList_New(0); - } else { - seq = list_slice_lock_held((PyListObject*)value, 0, size); - } + seq = list_slice_lock_held((PyListObject*)value, 0, + Py_SIZE(value)); Py_END_CRITICAL_SECTION(); } else { From 6c037d412e84d4bae230c0edd0368d77f7a477a9 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 5 Mar 2024 13:33:22 +0900 Subject: [PATCH 12/12] Add test case --- Lib/test/test_list.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Lib/test/test_list.py b/Lib/test/test_list.py index 2969c6e2f98a23..0601b33e79ebb6 100644 --- a/Lib/test/test_list.py +++ b/Lib/test/test_list.py @@ -96,6 +96,11 @@ def imul(a, b): a *= b self.assertRaises((MemoryError, OverflowError), mul, lst, n) self.assertRaises((MemoryError, OverflowError), imul, lst, n) + def test_empty_slice(self): + x = [] + x[:] = x + self.assertEqual(x, []) + def test_list_resize_overflow(self): # gh-97616: test new_allocated * sizeof(PyObject*) overflow # check in list_resize()