From 7186522c96c47f2a807d588a5de496149767b867 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 1 Feb 2024 11:45:22 +0900 Subject: [PATCH 01/12] gh-112087: Make __sizeof__ and listiter_{len, next} to be threadsafe --- Objects/listobject.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 80a1f1da55b8bc..66d54be7d6055d 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -20,6 +20,14 @@ class list "PyListObject *" "&PyList_Type" _Py_DECLARE_STR(list_err, "list index out of range"); +#ifdef Py_GIL_DISABLED + #define LOAD_SSIZE_ATOMIC_AS_POSSBILE(value) _Py_atomic_load_ssize_relaxed(&value) + #define STORE_SSIZE_ATOMIC_AS_POSSBILE(value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value) +#else + #define LOAD_SSIZE_ATOMIC_AS_POSSBILE(value) value + #define STORE_SSIZE_ATOMIC_AS_POSSBILE(value, new_value) value = new_value +#endif + #ifdef WITH_FREELISTS static struct _Py_list_state * get_list_state(void) @@ -2948,7 +2956,8 @@ list___sizeof___impl(PyListObject *self) /*[clinic end generated code: output=3417541f95f9a53e input=b8030a5d5ce8a187]*/ { size_t res = _PyObject_SIZE(Py_TYPE(self)); - res += (size_t)self->allocated * sizeof(void*); + Py_ssize_t allocated = LOAD_SSIZE_ATOMIC_AS_POSSBILE(self->allocated); + res += (size_t)allocated * sizeof(void*); return PyLong_FromSize_t(res); } @@ -3375,8 +3384,9 @@ listiter_len(PyObject *self, PyObject *Py_UNUSED(ignored)) { _PyListIterObject *it = (_PyListIterObject *)self; Py_ssize_t len; + Py_ssize_t index = LOAD_SSIZE_ATOMIC_AS_POSSBILE(it->it_index); if (it->it_seq) { - len = PyList_GET_SIZE(it->it_seq) - it->it_index; + len = PyList_GET_SIZE(it->it_seq) - index; if (len >= 0) return PyLong_FromSsize_t(len); } @@ -3514,13 +3524,13 @@ listreviter_next(PyObject *self) } assert(PyList_Check(seq)); - index = it->it_index; + index = LOAD_SSIZE_ATOMIC_AS_POSSBILE(it->it_index); if (index>=0 && index < PyList_GET_SIZE(seq)) { item = PyList_GET_ITEM(seq, index); it->it_index--; return Py_NewRef(item); } - it->it_index = -1; + STORE_SSIZE_ATOMIC_AS_POSSBILE(it->it_index, -1); it->it_seq = NULL; Py_DECREF(seq); return NULL; From e8be089dae5b906ead97f15902192d377909d276 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 1 Feb 2024 13:12:00 +0900 Subject: [PATCH 02/12] Fix typo --- Objects/listobject.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 66d54be7d6055d..5aeedcc319f3ff 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -21,11 +21,11 @@ class list "PyListObject *" "&PyList_Type" _Py_DECLARE_STR(list_err, "list index out of range"); #ifdef Py_GIL_DISABLED - #define LOAD_SSIZE_ATOMIC_AS_POSSBILE(value) _Py_atomic_load_ssize_relaxed(&value) - #define STORE_SSIZE_ATOMIC_AS_POSSBILE(value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value) + #define LOAD_SSIZE_ATOMIC_AS_POSSIBLE(value) _Py_atomic_load_ssize_relaxed(&value) + #define STORE_SSIZE_ATOMIC_AS_POSSIBLE(value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value) #else - #define LOAD_SSIZE_ATOMIC_AS_POSSBILE(value) value - #define STORE_SSIZE_ATOMIC_AS_POSSBILE(value, new_value) value = new_value + #define LOAD_SSIZE_ATOMIC_AS_POSSIBLE(value) value + #define LOAD_SSIZE_ATOMIC_AS_POSSIBLE(value, new_value) value = new_value #endif #ifdef WITH_FREELISTS @@ -2956,7 +2956,7 @@ list___sizeof___impl(PyListObject *self) /*[clinic end generated code: output=3417541f95f9a53e input=b8030a5d5ce8a187]*/ { size_t res = _PyObject_SIZE(Py_TYPE(self)); - Py_ssize_t allocated = LOAD_SSIZE_ATOMIC_AS_POSSBILE(self->allocated); + Py_ssize_t allocated = LOAD_SSIZE_ATOMIC_AS_POSSIBLE(self->allocated); res += (size_t)allocated * sizeof(void*); return PyLong_FromSize_t(res); } @@ -3384,7 +3384,7 @@ listiter_len(PyObject *self, PyObject *Py_UNUSED(ignored)) { _PyListIterObject *it = (_PyListIterObject *)self; Py_ssize_t len; - Py_ssize_t index = LOAD_SSIZE_ATOMIC_AS_POSSBILE(it->it_index); + Py_ssize_t index = LOAD_SSIZE_ATOMIC_AS_POSSIBLE(it->it_index); if (it->it_seq) { len = PyList_GET_SIZE(it->it_seq) - index; if (len >= 0) @@ -3524,13 +3524,13 @@ listreviter_next(PyObject *self) } assert(PyList_Check(seq)); - index = LOAD_SSIZE_ATOMIC_AS_POSSBILE(it->it_index); + index = LOAD_SSIZE_ATOMIC_AS_POSSIBLE(it->it_index); if (index>=0 && index < PyList_GET_SIZE(seq)) { item = PyList_GET_ITEM(seq, index); it->it_index--; return Py_NewRef(item); } - STORE_SSIZE_ATOMIC_AS_POSSBILE(it->it_index, -1); + STORE_SSIZE_ATOMIC_AS_POSSIBLE(it->it_index, -1); it->it_seq = NULL; Py_DECREF(seq); return NULL; From 777c9d54f70ee0e18a33aa0c6cc7f770e7ec7bfd Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 1 Feb 2024 13:25:02 +0900 Subject: [PATCH 03/12] nit --- Objects/listobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 5aeedcc319f3ff..2e07ed683ffd8a 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -25,7 +25,7 @@ _Py_DECLARE_STR(list_err, "list index out of range"); #define STORE_SSIZE_ATOMIC_AS_POSSIBLE(value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value) #else #define LOAD_SSIZE_ATOMIC_AS_POSSIBLE(value) value - #define LOAD_SSIZE_ATOMIC_AS_POSSIBLE(value, new_value) value = new_value + #define STORE_SSIZE_ATOMIC_AS_POSSIBLE(value, new_value) value = new_value #endif #ifdef WITH_FREELISTS From 671a1bc8a8aa579dd857a515d8ec37a21e38a8d4 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 3 Feb 2024 19:37:58 +0900 Subject: [PATCH 04/12] Address code review --- Objects/listobject.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 2e07ed683ffd8a..2d1368e4cbc258 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -21,11 +21,11 @@ class list "PyListObject *" "&PyList_Type" _Py_DECLARE_STR(list_err, "list index out of range"); #ifdef Py_GIL_DISABLED - #define LOAD_SSIZE_ATOMIC_AS_POSSIBLE(value) _Py_atomic_load_ssize_relaxed(&value) - #define STORE_SSIZE_ATOMIC_AS_POSSIBLE(value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value) + #define LOAD_SSIZE(value) _Py_atomic_load_ssize_relaxed(&value) + #define STORE_SSIZE(value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value) #else - #define LOAD_SSIZE_ATOMIC_AS_POSSIBLE(value) value - #define STORE_SSIZE_ATOMIC_AS_POSSIBLE(value, new_value) value = new_value + #define LOAD_SSIZE(value) value + #define STORE_SSIZE(value, new_value) value = new_value #endif #ifdef WITH_FREELISTS @@ -2956,7 +2956,7 @@ list___sizeof___impl(PyListObject *self) /*[clinic end generated code: output=3417541f95f9a53e input=b8030a5d5ce8a187]*/ { size_t res = _PyObject_SIZE(Py_TYPE(self)); - Py_ssize_t allocated = LOAD_SSIZE_ATOMIC_AS_POSSIBLE(self->allocated); + Py_ssize_t allocated = LOAD_SSIZE(self->allocated); res += (size_t)allocated * sizeof(void*); return PyLong_FromSize_t(res); } @@ -3384,7 +3384,7 @@ listiter_len(PyObject *self, PyObject *Py_UNUSED(ignored)) { _PyListIterObject *it = (_PyListIterObject *)self; Py_ssize_t len; - Py_ssize_t index = LOAD_SSIZE_ATOMIC_AS_POSSIBLE(it->it_index); + Py_ssize_t index = LOAD_SSIZE(it->it_index); if (it->it_seq) { len = PyList_GET_SIZE(it->it_seq) - index; if (len >= 0) @@ -3524,13 +3524,13 @@ listreviter_next(PyObject *self) } assert(PyList_Check(seq)); - index = LOAD_SSIZE_ATOMIC_AS_POSSIBLE(it->it_index); + index = LOAD_SSIZE(it->it_index); if (index>=0 && index < PyList_GET_SIZE(seq)) { item = PyList_GET_ITEM(seq, index); it->it_index--; return Py_NewRef(item); } - STORE_SSIZE_ATOMIC_AS_POSSIBLE(it->it_index, -1); + STORE_SSIZE(it->it_index, -1); it->it_seq = NULL; Py_DECREF(seq); return NULL; From a93faf684a5bebdcb1d855d3e86cc761993dd60e Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 8 Feb 2024 14:32:06 +0900 Subject: [PATCH 05/12] Address code review --- Lib/test/support/__init__.py | 27 ++++++------ Lib/test/test_iter.py | 2 +- Objects/listobject.c | 83 ++++++++++++++---------------------- Python/bytecodes.c | 5 +-- Python/generated_cases.c.h | 5 +-- 5 files changed, 51 insertions(+), 71 deletions(-) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 5b091fb2fd32dc..1d03ec0f5bd12b 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -1727,19 +1727,22 @@ def _check_tracemalloc(): def check_free_after_iterating(test, iter, cls, args=()): - class A(cls): - def __del__(self): - nonlocal done - done = True - try: - next(it) - except StopIteration: - pass - done = False - it = iter(A(*args)) - # Issue 26494: Shouldn't crash - test.assertRaises(StopIteration, next, it) + def wrapper(): + class A(cls): + def __del__(self): + nonlocal done + done = True + try: + next(it) + except StopIteration: + pass + + it = iter(A(*args)) + # Issue 26494: Shouldn't crash + test.assertRaises(StopIteration, next, it) + + wrapper() # The sequence should be deallocated just after the end of iterating gc_collect() test.assertTrue(done) diff --git a/Lib/test/test_iter.py b/Lib/test/test_iter.py index 30aedb0db3bb3d..9606d5beab71cb 100644 --- a/Lib/test/test_iter.py +++ b/Lib/test/test_iter.py @@ -302,7 +302,7 @@ def __eq__(self, other): # listiter_reduce_general self.assertEqual( run("reversed", orig["reversed"](list(range(8)))), - (iter, ([],)) + (reversed, ([],)) ) for case in types: diff --git a/Objects/listobject.c b/Objects/listobject.c index 30bc5f026b648e..d4b8bfb9ea1fe2 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3392,34 +3392,29 @@ static PyObject * listiter_next(PyObject *self) { _PyListIterObject *it = (_PyListIterObject *)self; - PyListObject *seq; - PyObject *item; - - assert(it != NULL); - seq = it->it_seq; - if (seq == NULL) + Py_ssize_t index = LOAD_SSIZE(it->it_index); + if (index < 0) { return NULL; - assert(PyList_Check(seq)); - - if (it->it_index < PyList_GET_SIZE(seq)) { - item = PyList_GET_ITEM(seq, it->it_index); - ++it->it_index; - return Py_NewRef(item); } - it->it_seq = NULL; - Py_DECREF(seq); - return NULL; + PyObject *item = list_get_item_ref(it->it_seq, index); + if (item == NULL) { + // out-of-bounds + STORE_SSIZE(it->it_index, -1); + return NULL; + } + STORE_SSIZE(it->it_index, index + 1); + return item; } static PyObject * listiter_len(PyObject *self, PyObject *Py_UNUSED(ignored)) { + assert(self != NULL); _PyListIterObject *it = (_PyListIterObject *)self; - Py_ssize_t len; Py_ssize_t index = LOAD_SSIZE(it->it_index); - if (it->it_seq) { - len = PyList_GET_SIZE(it->it_seq) - index; + if (index >= 0) { + Py_ssize_t len = PyList_GET_SIZE(it->it_seq) - index; if (len >= 0) return PyLong_FromSsize_t(len); } @@ -3440,8 +3435,8 @@ listiter_setstate(PyObject *self, PyObject *state) if (index == -1 && PyErr_Occurred()) return NULL; if (it->it_seq != NULL) { - if (index < 0) - index = 0; + if (index < -1) + index = -1; else if (index > PyList_GET_SIZE(it->it_seq)) index = PyList_GET_SIZE(it->it_seq); /* iterator exhausted */ it->it_index = index; @@ -3546,26 +3541,19 @@ static PyObject * listreviter_next(PyObject *self) { listreviterobject *it = (listreviterobject *)self; - PyObject *item; - Py_ssize_t index; - PyListObject *seq; - assert(it != NULL); - seq = it->it_seq; - if (seq == NULL) { - return NULL; - } + PyListObject *seq = it->it_seq; assert(PyList_Check(seq)); - index = LOAD_SSIZE(it->it_index); + Py_ssize_t index = LOAD_SSIZE(it->it_index); if (index>=0 && index < PyList_GET_SIZE(seq)) { - item = PyList_GET_ITEM(seq, index); - it->it_index--; - return Py_NewRef(item); + PyObject *item = list_get_item_ref(seq, index); + if (item != NULL) { + STORE_SSIZE(it->it_index, index - 1); + return item; + } } STORE_SSIZE(it->it_index, -1); - it->it_seq = NULL; - Py_DECREF(seq); return NULL; } @@ -3573,8 +3561,10 @@ static PyObject * listreviter_len(PyObject *self, PyObject *Py_UNUSED(ignored)) { listreviterobject *it = (listreviterobject *)self; - Py_ssize_t len = it->it_index + 1; - if (it->it_seq == NULL || PyList_GET_SIZE(it->it_seq) < len) + Py_ssize_t index = LOAD_SSIZE(it->it_index); + Py_ssize_t len; + STORE_SSIZE(len, index + 1); + if (PyList_GET_SIZE(it->it_seq) < len) len = 0; return PyLong_FromSsize_t(len); } @@ -3608,6 +3598,7 @@ static PyObject * listiter_reduce_general(void *_it, int forward) { PyObject *list; + PyObject *iter; /* _PyEval_GetBuiltin can invoke arbitrary code, * call must be before access of iterator pointers. @@ -3615,29 +3606,21 @@ listiter_reduce_general(void *_it, int forward) /* the objects are not the same, index is of different types! */ if (forward) { - PyObject *iter = _PyEval_GetBuiltin(&_Py_ID(iter)); - if (!iter) { - return NULL; - } + iter = _PyEval_GetBuiltin(&_Py_ID(iter)); _PyListIterObject *it = (_PyListIterObject *)_it; - if (it->it_seq) { + if (it->it_index >= 0) { return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); } - Py_DECREF(iter); } else { - PyObject *reversed = _PyEval_GetBuiltin(&_Py_ID(reversed)); - if (!reversed) { - return NULL; - } + iter = _PyEval_GetBuiltin(&_Py_ID(reversed)); listreviterobject *it = (listreviterobject *)_it; - if (it->it_seq) { - return Py_BuildValue("N(O)n", reversed, it->it_seq, it->it_index); + if (it->it_index >= 0) { + return Py_BuildValue("N(O)n", iter, it->it_seq, it->it_index); } - Py_DECREF(reversed); } /* empty iterator, create an empty list */ list = PyList_New(0); if (list == NULL) return NULL; - return Py_BuildValue("N(N)", _PyEval_GetBuiltin(&_Py_ID(iter)), list); + return Py_BuildValue("N(N)", iter, list); } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 6fb4d719e43991..f92e09554cff0c 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2615,10 +2615,7 @@ dummy_func( STAT_INC(FOR_ITER, hit); PyListObject *seq = it->it_seq; if (seq == NULL || it->it_index >= PyList_GET_SIZE(seq)) { - if (seq != NULL) { - it->it_seq = NULL; - Py_DECREF(seq); - } + it->it_index = -1; Py_DECREF(iter); STACK_SHRINK(1); /* Jump forward oparg, then skip following END_FOR and POP_TOP instructions */ diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 16f1db30620d72..7a9e5efd5127e7 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2553,10 +2553,7 @@ STAT_INC(FOR_ITER, hit); PyListObject *seq = it->it_seq; if (seq == NULL || it->it_index >= PyList_GET_SIZE(seq)) { - if (seq != NULL) { - it->it_seq = NULL; - Py_DECREF(seq); - } + it->it_index = -1; Py_DECREF(iter); STACK_SHRINK(1); /* Jump forward oparg, then skip following END_FOR and POP_TOP instructions */ From 44f20223a0d25a6636b21b0756a5f856cae62f31 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 9 Feb 2024 09:20:17 +0900 Subject: [PATCH 06/12] Address code review --- Objects/listobject.c | 24 +++++++++++++++++------- Python/bytecodes.c | 6 ++++++ Python/generated_cases.c.h | 6 ++++++ 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index d4b8bfb9ea1fe2..47a39bd57d8f45 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3393,13 +3393,18 @@ listiter_next(PyObject *self) { _PyListIterObject *it = (_PyListIterObject *)self; Py_ssize_t index = LOAD_SSIZE(it->it_index); - if (index < 0) { + if (it->it_seq == NULL || index < 0) { return NULL; } PyObject *item = list_get_item_ref(it->it_seq, index); if (item == NULL) { // out-of-bounds +#ifndef Py_GIL_DISABLED + PyListObject *seq = it->it_seq; + it->it_seq = NULL; + Py_DECREF(seq); +#endif STORE_SSIZE(it->it_index, -1); return NULL; } @@ -3546,13 +3551,18 @@ listreviter_next(PyObject *self) assert(PyList_Check(seq)); Py_ssize_t index = LOAD_SSIZE(it->it_index); - if (index>=0 && index < PyList_GET_SIZE(seq)) { - PyObject *item = list_get_item_ref(seq, index); - if (item != NULL) { - STORE_SSIZE(it->it_index, index - 1); - return item; - } + if (it->it_seq == NULL || index < 0) { + return NULL; } + PyObject *item = list_get_item_ref(seq, index); + if (item != NULL) { + STORE_SSIZE(it->it_index, index - 1); + return item; + } +#ifndef Py_GIL_DISABLED + it->it_seq = NULL; + Py_DECREF(seq); +#endif STORE_SSIZE(it->it_index, -1); return NULL; } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index f92e09554cff0c..f91b791e68741a 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2616,6 +2616,12 @@ dummy_func( PyListObject *seq = it->it_seq; if (seq == NULL || it->it_index >= PyList_GET_SIZE(seq)) { it->it_index = -1; + #ifndef Py_GIL_DISABLED + if (seq != NULL) { + it->it_seq = NULL; + Py_DECREF(seq); + } + #endif Py_DECREF(iter); STACK_SHRINK(1); /* Jump forward oparg, then skip following END_FOR and POP_TOP instructions */ diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 7a9e5efd5127e7..068eefafd1b4a4 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2554,6 +2554,12 @@ PyListObject *seq = it->it_seq; if (seq == NULL || it->it_index >= PyList_GET_SIZE(seq)) { it->it_index = -1; + #ifndef Py_GIL_DISABLED + if (seq != NULL) { + it->it_seq = NULL; + Py_DECREF(seq); + } + #endif Py_DECREF(iter); STACK_SHRINK(1); /* Jump forward oparg, then skip following END_FOR and POP_TOP instructions */ From 79989dbf9f1dfce9c774ce0b462073a6e7e731f1 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 9 Feb 2024 09:22:29 +0900 Subject: [PATCH 07/12] nit --- Python/bytecodes.c | 2 +- Python/generated_cases.c.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index f91b791e68741a..a453de5d6d7d37 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2615,13 +2615,13 @@ dummy_func( STAT_INC(FOR_ITER, hit); PyListObject *seq = it->it_seq; if (seq == NULL || it->it_index >= PyList_GET_SIZE(seq)) { - it->it_index = -1; #ifndef Py_GIL_DISABLED if (seq != NULL) { it->it_seq = NULL; Py_DECREF(seq); } #endif + it->it_index = -1; Py_DECREF(iter); STACK_SHRINK(1); /* Jump forward oparg, then skip following END_FOR and POP_TOP instructions */ diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 068eefafd1b4a4..c23c8bf99c8aff 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2553,13 +2553,13 @@ STAT_INC(FOR_ITER, hit); PyListObject *seq = it->it_seq; if (seq == NULL || it->it_index >= PyList_GET_SIZE(seq)) { - it->it_index = -1; #ifndef Py_GIL_DISABLED if (seq != NULL) { it->it_seq = NULL; Py_DECREF(seq); } #endif + it->it_index = -1; Py_DECREF(iter); STACK_SHRINK(1); /* Jump forward oparg, then skip following END_FOR and POP_TOP instructions */ From f4526f98c5cd3ccb7938cdb164247445a2c0b10d Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Fri, 9 Feb 2024 09:49:02 +0900 Subject: [PATCH 08/12] fix --- Objects/listobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 47a39bd57d8f45..1b4c1464952dd5 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3574,7 +3574,7 @@ listreviter_len(PyObject *self, PyObject *Py_UNUSED(ignored)) Py_ssize_t index = LOAD_SSIZE(it->it_index); Py_ssize_t len; STORE_SSIZE(len, index + 1); - if (PyList_GET_SIZE(it->it_seq) < len) + if (it->it_seq == NULL || PyList_GET_SIZE(it->it_seq) < len) len = 0; return PyLong_FromSsize_t(len); } From a13beedb49cf7f663196e384650d77a1e705b9f8 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 14 Feb 2024 23:22:29 +0900 Subject: [PATCH 09/12] Address code review --- Objects/listobject.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 1b4c1464952dd5..1f204ee20b9146 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -21,11 +21,11 @@ class list "PyListObject *" "&PyList_Type" _Py_DECLARE_STR(list_err, "list index out of range"); #ifdef Py_GIL_DISABLED - #define LOAD_SSIZE(value) _Py_atomic_load_ssize_relaxed(&value) - #define STORE_SSIZE(value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value) +# define LOAD_SSIZE(value) _Py_atomic_load_ssize_relaxed(&value) +# define STORE_SSIZE(value, new_value) _Py_atomic_store_ssize_relaxed(&value, new_value) #else - #define LOAD_SSIZE(value) value - #define STORE_SSIZE(value, new_value) value = new_value +# define LOAD_SSIZE(value) value +# define STORE_SSIZE(value, new_value) value = new_value #endif #ifdef WITH_FREELISTS @@ -3572,8 +3572,7 @@ listreviter_len(PyObject *self, PyObject *Py_UNUSED(ignored)) { listreviterobject *it = (listreviterobject *)self; Py_ssize_t index = LOAD_SSIZE(it->it_index); - Py_ssize_t len; - STORE_SSIZE(len, index + 1); + Py_ssize_t len = index + 1; if (it->it_seq == NULL || PyList_GET_SIZE(it->it_seq) < len) len = 0; return PyLong_FromSsize_t(len); From bec617a5012e77b8891bdf9b0b05b375dc9fcb9a Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 14 Feb 2024 23:51:56 +0900 Subject: [PATCH 10/12] Address code review --- .../2024-02-14-23-50-55.gh-issue-112087.H_4W_v.rst | 2 ++ Python/bytecodes.c | 6 ++++-- Python/executor_cases.c.h | 4 +++- Python/generated_cases.c.h | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-55.gh-issue-112087.H_4W_v.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-55.gh-issue-112087.H_4W_v.rst b/Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-55.gh-issue-112087.H_4W_v.rst new file mode 100644 index 00000000000000..f92cdafd4ba225 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-02-14-23-50-55.gh-issue-112087.H_4W_v.rst @@ -0,0 +1,2 @@ +For an empty reverse iterator for list will be reduced to :func:`reversed`. +Patch by Donghee Na diff --git a/Python/bytecodes.c b/Python/bytecodes.c index a453de5d6d7d37..ff47ade1a0757f 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2614,7 +2614,7 @@ dummy_func( assert(Py_TYPE(iter) == &PyListIter_Type); STAT_INC(FOR_ITER, hit); PyListObject *seq = it->it_seq; - if (seq == NULL || it->it_index >= PyList_GET_SIZE(seq)) { + if ((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)) { #ifndef Py_GIL_DISABLED if (seq != NULL) { it->it_seq = NULL; @@ -2635,8 +2635,10 @@ dummy_func( _PyListIterObject *it = (_PyListIterObject *)iter; assert(Py_TYPE(iter) == &PyListIter_Type); PyListObject *seq = it->it_seq; + #ifndef Py_GIL_DISABLED DEOPT_IF(seq == NULL); - DEOPT_IF(it->it_index >= PyList_GET_SIZE(seq)); + #endif + DEOPT_IF((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)); } op(_ITER_NEXT_LIST, (iter -- iter, next)) { diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 2d914b82dbf88f..1dfeafc8b1a990 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -2201,8 +2201,10 @@ _PyListIterObject *it = (_PyListIterObject *)iter; assert(Py_TYPE(iter) == &PyListIter_Type); PyListObject *seq = it->it_seq; + #ifndef Py_GIL_DISABLED if (seq == NULL) goto deoptimize; - if (it->it_index >= PyList_GET_SIZE(seq)) goto deoptimize; + #endif + if ((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)) goto deoptimize; break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index c23c8bf99c8aff..6a2adc5cbb2b3f 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2552,7 +2552,7 @@ assert(Py_TYPE(iter) == &PyListIter_Type); STAT_INC(FOR_ITER, hit); PyListObject *seq = it->it_seq; - if (seq == NULL || it->it_index >= PyList_GET_SIZE(seq)) { + if ((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)) { #ifndef Py_GIL_DISABLED if (seq != NULL) { it->it_seq = NULL; From 6e170cc130ce662951ce4216a15aaa21ac9fc226 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 15 Feb 2024 00:47:25 +0900 Subject: [PATCH 11/12] Address code review --- Objects/listobject.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Objects/listobject.c b/Objects/listobject.c index 1f204ee20b9146..4d011c30b6f4f6 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -3393,19 +3393,19 @@ listiter_next(PyObject *self) { _PyListIterObject *it = (_PyListIterObject *)self; Py_ssize_t index = LOAD_SSIZE(it->it_index); - if (it->it_seq == NULL || index < 0) { + if (index < 0) { return NULL; } PyObject *item = list_get_item_ref(it->it_seq, index); if (item == NULL) { // out-of-bounds + STORE_SSIZE(it->it_index, -1); #ifndef Py_GIL_DISABLED PyListObject *seq = it->it_seq; it->it_seq = NULL; Py_DECREF(seq); #endif - STORE_SSIZE(it->it_index, -1); return NULL; } STORE_SSIZE(it->it_index, index + 1); @@ -3551,7 +3551,7 @@ listreviter_next(PyObject *self) assert(PyList_Check(seq)); Py_ssize_t index = LOAD_SSIZE(it->it_index); - if (it->it_seq == NULL || index < 0) { + if (index < 0) { return NULL; } PyObject *item = list_get_item_ref(seq, index); @@ -3559,11 +3559,11 @@ listreviter_next(PyObject *self) STORE_SSIZE(it->it_index, index - 1); return item; } + STORE_SSIZE(it->it_index, -1); #ifndef Py_GIL_DISABLED it->it_seq = NULL; Py_DECREF(seq); #endif - STORE_SSIZE(it->it_index, -1); return NULL; } From c8a8b81b413b5f8ba2ea2014582086a2c9be4c9a Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Thu, 15 Feb 2024 00:55:21 +0900 Subject: [PATCH 12/12] Address code review --- Python/bytecodes.c | 5 +---- Python/executor_cases.c.h | 3 --- Python/generated_cases.c.h | 2 +- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index ff47ade1a0757f..ae8c16a8e897ae 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2615,13 +2615,13 @@ dummy_func( STAT_INC(FOR_ITER, hit); PyListObject *seq = it->it_seq; if ((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)) { + it->it_index = -1; #ifndef Py_GIL_DISABLED if (seq != NULL) { it->it_seq = NULL; Py_DECREF(seq); } #endif - it->it_index = -1; Py_DECREF(iter); STACK_SHRINK(1); /* Jump forward oparg, then skip following END_FOR and POP_TOP instructions */ @@ -2635,9 +2635,6 @@ dummy_func( _PyListIterObject *it = (_PyListIterObject *)iter; assert(Py_TYPE(iter) == &PyListIter_Type); PyListObject *seq = it->it_seq; - #ifndef Py_GIL_DISABLED - DEOPT_IF(seq == NULL); - #endif DEOPT_IF((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)); } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 1dfeafc8b1a990..ba479032ffd2a9 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -2201,9 +2201,6 @@ _PyListIterObject *it = (_PyListIterObject *)iter; assert(Py_TYPE(iter) == &PyListIter_Type); PyListObject *seq = it->it_seq; - #ifndef Py_GIL_DISABLED - if (seq == NULL) goto deoptimize; - #endif if ((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)) goto deoptimize; break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 6a2adc5cbb2b3f..f4c90800e19a9e 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2553,13 +2553,13 @@ STAT_INC(FOR_ITER, hit); PyListObject *seq = it->it_seq; if ((size_t)it->it_index >= (size_t)PyList_GET_SIZE(seq)) { + it->it_index = -1; #ifndef Py_GIL_DISABLED if (seq != NULL) { it->it_seq = NULL; Py_DECREF(seq); } #endif - it->it_index = -1; Py_DECREF(iter); STACK_SHRINK(1); /* Jump forward oparg, then skip following END_FOR and POP_TOP instructions */