From 0d1b08a7ef7c7593d979b03f959673af91c61b27 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 24 Nov 2022 23:11:10 +0100 Subject: [PATCH 01/29] add invalid_index --- Include/pymacro.h | 14 ++++++++++++++ Modules/_collectionsmodule.c | 9 +-------- Modules/arraymodule.c | 8 ++++---- Modules/mmapmodule.c | 8 ++++---- Objects/bytearrayobject.c | 10 +++++----- Objects/bytesobject.c | 4 ++-- Objects/codeobject.c | 3 +-- Objects/listobject.c | 25 ++++++------------------- Objects/memoryobject.c | 2 +- Objects/tupleobject.c | 6 +++--- Objects/unicodeobject.c | 6 +++--- Python/bytecodes.c | 6 +++--- 12 files changed, 47 insertions(+), 54 deletions(-) diff --git a/Include/pymacro.h b/Include/pymacro.h index e37cda44c5ebf1..d7dcd6b20b2350 100644 --- a/Include/pymacro.h +++ b/Include/pymacro.h @@ -157,4 +157,18 @@ // "comparison of unsigned expression in '< 0' is always false". #define _Py_IS_TYPE_SIGNED(type) ((type)(-1) <= 0) +/// Return 1 if the specified index is outside the range [0, limit) +static inline int +invalid_index(Py_ssize_t i, Py_ssize_t limit) +{ + /* The cast to size_t lets us use just a single comparison + to check whether i is in the range: 0 <= i < limit. + + See: Section 14.2 "Bounds Checking" in the Agner Fog + optimization manual found at: + https://www.agner.org/optimize/optimizing_cpp.pdf + */ + return (size_t)i >= (size_t)limit; +} + #endif /* Py_PYMACRO_H */ diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 5fa583821889f3..b771970b7d091a 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1149,13 +1149,6 @@ PyDoc_STRVAR(insert_doc, PyDoc_STRVAR(remove_doc, "D.remove(value) -- remove first occurrence of value."); -static int -valid_index(Py_ssize_t i, Py_ssize_t limit) -{ - /* The cast to size_t lets us use just a single comparison - to check whether i is in the range: 0 <= i < limit */ - return (size_t) i < (size_t) limit; -} static PyObject * deque_item(dequeobject *deque, Py_ssize_t i) @@ -1164,7 +1157,7 @@ deque_item(dequeobject *deque, Py_ssize_t i) PyObject *item; Py_ssize_t n, index=i; - if (!valid_index(i, Py_SIZE(deque))) { + if (invalid_index(i, Py_SIZE(deque))) { PyErr_SetString(PyExc_IndexError, "deque index out of range"); return NULL; } diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index dcf510e9788ee5..5dbfd41401087c 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -804,7 +804,7 @@ array_length(arrayobject *a) static PyObject * array_item(arrayobject *a, Py_ssize_t i) { - if (i < 0 || i >= Py_SIZE(a)) { + if (invalid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "array index out of range"); return NULL; } @@ -966,7 +966,7 @@ array_del_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) static int array_ass_item(arrayobject *a, Py_ssize_t i, PyObject *v) { - if (i < 0 || i >= Py_SIZE(a)) { + if (invalid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); return -1; @@ -1249,7 +1249,7 @@ array_array_pop_impl(arrayobject *self, Py_ssize_t i) } if (i < 0) i += Py_SIZE(self); - if (i < 0 || i >= Py_SIZE(self)) { + if (invalid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); return NULL; } @@ -2407,7 +2407,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) return -1; if (i < 0) i += Py_SIZE(self); - if (i < 0 || i >= Py_SIZE(self)) { + if (invalid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); return -1; diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 2311840e22105f..0b412b7d4f5bd1 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -920,7 +920,7 @@ static PyObject * mmap_item(mmap_object *self, Py_ssize_t i) { CHECK_VALID(NULL); - if (i < 0 || i >= self->size) { + if (invalid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return NULL; } @@ -937,7 +937,7 @@ mmap_subscript(mmap_object *self, PyObject *item) return NULL; if (i < 0) i += self->size; - if (i < 0 || i >= self->size) { + if (invalid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return NULL; @@ -988,7 +988,7 @@ mmap_ass_item(mmap_object *self, Py_ssize_t i, PyObject *v) const char *buf; CHECK_VALID(-1); - if (i < 0 || i >= self->size) { + if (invalid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return -1; } @@ -1025,7 +1025,7 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value) return -1; if (i < 0) i += self->size; - if (i < 0 || i >= self->size) { + if (invalid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return -1; diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index 0ba6fb5b76ccc7..5d7793dea8c3bf 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -358,7 +358,7 @@ bytearray_irepeat(PyByteArrayObject *self, Py_ssize_t count) static PyObject * bytearray_getitem(PyByteArrayObject *self, Py_ssize_t i) { - if (i < 0 || i >= Py_SIZE(self)) { + if (invalid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return NULL; } @@ -377,7 +377,7 @@ bytearray_subscript(PyByteArrayObject *self, PyObject *index) if (i < 0) i += PyByteArray_GET_SIZE(self); - if (i < 0 || i >= Py_SIZE(self)) { + if (invalid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return NULL; } @@ -572,7 +572,7 @@ bytearray_setitem(PyByteArrayObject *self, Py_ssize_t i, PyObject *value) i += Py_SIZE(self); } - if (i < 0 || i >= Py_SIZE(self)) { + if (invalid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return -1; } @@ -612,7 +612,7 @@ bytearray_ass_subscript(PyByteArrayObject *self, PyObject *index, PyObject *valu i += PyByteArray_GET_SIZE(self); } - if (i < 0 || i >= Py_SIZE(self)) { + if (invalid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return -1; } @@ -1809,7 +1809,7 @@ bytearray_pop_impl(PyByteArrayObject *self, Py_ssize_t index) } if (index < 0) index += Py_SIZE(self); - if (index < 0 || index >= Py_SIZE(self)) { + if (invalid_index(index, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); return NULL; } diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index a63f396e022f71..726463ed13cd74 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -1484,7 +1484,7 @@ bytes_contains(PyObject *self, PyObject *arg) static PyObject * bytes_item(PyBytesObject *a, Py_ssize_t i) { - if (i < 0 || i >= Py_SIZE(a)) { + if (invalid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "index out of range"); return NULL; } @@ -1591,7 +1591,7 @@ bytes_subscript(PyBytesObject* self, PyObject* item) return NULL; if (i < 0) i += PyBytes_GET_SIZE(self); - if (i < 0 || i >= PyBytes_GET_SIZE(self)) { + if (invalid_index(i, PyBytes_GET_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "index out of range"); return NULL; diff --git a/Objects/codeobject.c b/Objects/codeobject.c index fc1db72977aa01..2837544b2adbcf 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1346,8 +1346,7 @@ _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) { PyInterpreterState *interp = _PyInterpreterState_GET(); - if (!PyCode_Check(code) || index < 0 || - index >= interp->co_extra_user_count) { + if (!PyCode_Check(code) || invalid_index(index, interp->co_extra_user_count)) { PyErr_BadInternalCall(); return -1; } diff --git a/Objects/listobject.c b/Objects/listobject.c index da623c9719aeb8..378526c719cae2 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -226,18 +226,6 @@ PyList_Size(PyObject *op) return Py_SIZE(op); } -static inline int -valid_index(Py_ssize_t i, Py_ssize_t limit) -{ - /* The cast to size_t lets us use just a single comparison - to check whether i is in the range: 0 <= i < limit. - - See: Section 14.2 "Bounds Checking" in the Agner Fog - optimization manual found at: - https://www.agner.org/optimize/optimizing_cpp.pdf - */ - return (size_t) i < (size_t) limit; -} PyObject * PyList_GetItem(PyObject *op, Py_ssize_t i) @@ -246,7 +234,7 @@ PyList_GetItem(PyObject *op, Py_ssize_t i) PyErr_BadInternalCall(); return NULL; } - if (!valid_index(i, Py_SIZE(op))) { + if (invalid_index(i, Py_SIZE(op))) { _Py_DECLARE_STR(list_err, "list index out of range"); PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); return NULL; @@ -264,7 +252,7 @@ PyList_SetItem(PyObject *op, Py_ssize_t i, PyErr_BadInternalCall(); return -1; } - if (!valid_index(i, Py_SIZE(op))) { + if (invalid_index(i, Py_SIZE(op))) { Py_XDECREF(newitem); PyErr_SetString(PyExc_IndexError, "list assignment index out of range"); @@ -455,7 +443,7 @@ list_contains(PyListObject *a, PyObject *el) static PyObject * list_item(PyListObject *a, Py_ssize_t i) { - if (!valid_index(i, Py_SIZE(a))) { + if (invalid_index(i, Py_SIZE(a))) { PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); return NULL; } @@ -767,7 +755,7 @@ list_inplace_repeat(PyListObject *self, Py_ssize_t n) static int list_ass_item(PyListObject *a, Py_ssize_t i, PyObject *v) { - if (!valid_index(i, Py_SIZE(a))) { + if (invalid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "list assignment index out of range"); return -1; @@ -1018,7 +1006,7 @@ list_pop_impl(PyListObject *self, Py_ssize_t index) } if (index < 0) index += Py_SIZE(self); - if (!valid_index(index, Py_SIZE(self))) { + if (invalid_index(index, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); return NULL; } @@ -2584,9 +2572,8 @@ list_index_impl(PyListObject *self, PyObject *value, Py_ssize_t start, } if (stop < 0) { stop += Py_SIZE(self); - if (stop < 0) - stop = 0; } + for (i = start; i < stop && i < Py_SIZE(self); i++) { PyObject *obj = self->ob_item[i]; Py_INCREF(obj); diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 1d6cc3b508448d..ca822609474321 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -2299,7 +2299,7 @@ lookup_dimension(const Py_buffer *view, char *ptr, int dim, Py_ssize_t index) if (index < 0) { index += nitems; } - if (index < 0 || index >= nitems) { + if (invalid_index(index, nitems)) { PyErr_Format(PyExc_IndexError, "index out of bounds on dimension %d", dim + 1); return NULL; diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 4405125d45e7cc..61dfee46d8bd85 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -100,7 +100,7 @@ PyTuple_GetItem(PyObject *op, Py_ssize_t i) PyErr_BadInternalCall(); return NULL; } - if (i < 0 || i >= Py_SIZE(op)) { + if (invalid_index(i, Py_SIZE(op))) { PyErr_SetString(PyExc_IndexError, "tuple index out of range"); return NULL; } @@ -116,7 +116,7 @@ PyTuple_SetItem(PyObject *op, Py_ssize_t i, PyObject *newitem) PyErr_BadInternalCall(); return -1; } - if (i < 0 || i >= Py_SIZE(op)) { + if (invalid_index(i, Py_SIZE(op))) { Py_XDECREF(newitem); PyErr_SetString(PyExc_IndexError, "tuple assignment index out of range"); @@ -361,7 +361,7 @@ tuplecontains(PyTupleObject *a, PyObject *el) static PyObject * tupleitem(PyTupleObject *a, Py_ssize_t i) { - if (i < 0 || i >= Py_SIZE(a)) { + if (invalid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "tuple index out of range"); return NULL; } diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 55f029dd504ca0..3082b1d4bf9328 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -1436,11 +1436,11 @@ PyUnicode_CopyCharacters(PyObject *to, Py_ssize_t to_start, return -1; } - if ((size_t)from_start > (size_t)PyUnicode_GET_LENGTH(from)) { + if (invalid_index(from_start, PyUnicode_GET_LENGTH(from))) { PyErr_SetString(PyExc_IndexError, "string index out of range"); return -1; } - if ((size_t)to_start > (size_t)PyUnicode_GET_LENGTH(to)) { + if (invalid_index(to_start, PyUnicode_GET_LENGTH(to))) { PyErr_SetString(PyExc_IndexError, "string index out of range"); return -1; } @@ -3721,7 +3721,7 @@ PyUnicode_ReadChar(PyObject *unicode, Py_ssize_t index) PyErr_BadArgument(); return (Py_UCS4)-1; } - if (index < 0 || index >= PyUnicode_GET_LENGTH(unicode)) { + if (invalid_index(index, PyUnicode_GET_LENGTH(unicode))) { PyErr_SetString(PyExc_IndexError, "string index out of range"); return (Py_UCS4)-1; } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index a1f910da8ed54a..254068ee2b4da8 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -384,7 +384,7 @@ dummy_func( // Deopt unless 0 <= sub < PyList_Size(list) Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); + DEOPT_IF(invalid_index(signed_magnitude, 2), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyList_GET_SIZE(list), BINARY_SUBSCR); @@ -403,7 +403,7 @@ dummy_func( // Deopt unless 0 <= sub < PyTuple_Size(list) Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); + DEOPT_IF(invalid_index(signed_magnitude, 2), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyTuple_GET_SIZE(tuple), BINARY_SUBSCR); @@ -507,7 +507,7 @@ dummy_func( DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - DEOPT_IF(((size_t)Py_SIZE(sub)) > 1, STORE_SUBSCR); + DEOPT_IF(invalid_index(Py_SIZE(sub), 2), STORE_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); From be6143fdf6b796569293ef4b535172b6ce16c8bf Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 25 Nov 2022 09:55:27 +0100 Subject: [PATCH 02/29] replace valid_index --- Modules/_collectionsmodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index b771970b7d091a..5aeb85da42f58f 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1252,7 +1252,7 @@ deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v) block *b; Py_ssize_t n, len=Py_SIZE(deque), halflen=(len+1)>>1, index=i; - if (!valid_index(i, len)) { + if (invalid_index(i, len)) { PyErr_SetString(PyExc_IndexError, "deque index out of range"); return -1; } @@ -2413,7 +2413,7 @@ tuplegetter_descr_get(PyObject *self, PyObject *obj, PyObject *type) return NULL; } - if (!valid_index(index, PyTuple_GET_SIZE(obj))) { + if (invalid_index(index, PyTuple_GET_SIZE(obj))) { PyErr_SetString(PyExc_IndexError, "tuple index out of range"); return NULL; } From bcaec8800242c062c3eef1605418bdbb6ae033b5 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Fri, 25 Nov 2022 09:34:58 +0000 Subject: [PATCH 03/29] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2022-11-25-09-34-57.gh-issue-9761.sRqH37.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-9761.sRqH37.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-9761.sRqH37.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-9761.sRqH37.rst new file mode 100644 index 00000000000000..316a474fabf2b1 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-9761.sRqH37.rst @@ -0,0 +1 @@ +Add method to calculate invalid_index From 6dd6fec845a2537b0bdab6391cbcaf02465760a7 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 25 Nov 2022 10:38:59 +0100 Subject: [PATCH 04/29] make regen-all --- Python/generated_cases.c.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index ae8fdd5e99c3dc..bfe8b01c5d0001 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -368,7 +368,7 @@ // Deopt unless 0 <= sub < PyList_Size(list) Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); + DEOPT_IF(invalid_index(signed_magnitude, 2), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyList_GET_SIZE(list), BINARY_SUBSCR); @@ -394,7 +394,7 @@ // Deopt unless 0 <= sub < PyTuple_Size(list) Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); + DEOPT_IF(invalid_index(signed_magnitude, 2), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyTuple_GET_SIZE(tuple), BINARY_SUBSCR); @@ -518,7 +518,7 @@ DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - DEOPT_IF(((size_t)Py_SIZE(sub)) > 1, STORE_SUBSCR); + DEOPT_IF(invalid_index(Py_SIZE(sub), 2), STORE_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); From 1e4448f527783cad6303654193f8a858255f53ea Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 25 Nov 2022 10:48:14 +0100 Subject: [PATCH 05/29] Rename 2022-11-25-09-34-57.gh-issue-9761.sRqH37.rst to 2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst --- ...1.sRqH37.rst => 2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Misc/NEWS.d/next/Core and Builtins/{2022-11-25-09-34-57.gh-issue-9761.sRqH37.rst => 2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst} (100%) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-9761.sRqH37.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst similarity index 100% rename from Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-9761.sRqH37.rst rename to Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst From 1d58ef00ae02dc322c20698e7ad3e942c64ae41b Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 25 Nov 2022 20:15:33 +0100 Subject: [PATCH 06/29] add method for int check --- Include/internal/pycore_long.h | 8 ++++++++ Python/bytecodes.c | 8 +++----- Python/generated_cases.c.h | 8 +++----- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h index 30c97b7edc98e1..6ffa2085072f1e 100644 --- a/Include/internal/pycore_long.h +++ b/Include/internal/pycore_long.h @@ -78,6 +78,14 @@ static inline PyObject* _PyLong_FromUnsignedChar(unsigned char i) return Py_NewRef((PyObject *)&_PyLong_SMALL_INTS[_PY_NSMALLNEGINTS+i]); } +/// Return 0 for nonnegative, zero-or-one-digit ints, 1 otherwise +static inline int +_PyLong_Negative_or_multi_digit_int(PyObject* sub) { + assert(PyLong_CheckExact(sub)); + Py_ssize_t signed_magnitude = Py_SIZE(sub); + return ((size_t)signed_magnitude) > 1; +} + PyObject *_PyLong_Add(PyLongObject *left, PyLongObject *right); PyObject *_PyLong_Multiply(PyLongObject *left, PyLongObject *right); PyObject *_PyLong_Subtract(PyLongObject *left, PyLongObject *right); diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 254068ee2b4da8..89a2b17251491f 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -383,8 +383,7 @@ dummy_func( DEOPT_IF(!PyList_CheckExact(list), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyList_Size(list) - Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(invalid_index(signed_magnitude, 2), BINARY_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyList_GET_SIZE(list), BINARY_SUBSCR); @@ -402,8 +401,7 @@ dummy_func( DEOPT_IF(!PyTuple_CheckExact(tuple), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyTuple_Size(list) - Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(invalid_index(signed_magnitude, 2), BINARY_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyTuple_GET_SIZE(tuple), BINARY_SUBSCR); @@ -507,7 +505,7 @@ dummy_func( DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - DEOPT_IF(invalid_index(Py_SIZE(sub), 2), STORE_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), STORE_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index bfe8b01c5d0001..97480e70e0661b 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -367,8 +367,7 @@ DEOPT_IF(!PyList_CheckExact(list), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyList_Size(list) - Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(invalid_index(signed_magnitude, 2), BINARY_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyList_GET_SIZE(list), BINARY_SUBSCR); @@ -393,8 +392,7 @@ DEOPT_IF(!PyTuple_CheckExact(tuple), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyTuple_Size(list) - Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(invalid_index(signed_magnitude, 2), BINARY_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyTuple_GET_SIZE(tuple), BINARY_SUBSCR); @@ -518,7 +516,7 @@ DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - DEOPT_IF(invalid_index(Py_SIZE(sub), 2), STORE_SUBSCR); + DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), STORE_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); From 76ecde6e786539cb27a2a3fbbfa6bc6dfd99ccf9 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 26 Nov 2022 22:11:10 +0100 Subject: [PATCH 07/29] change invalid_index to valid_index --- Include/pymacro.h | 4 ++-- Modules/_collectionsmodule.c | 6 +++--- Modules/arraymodule.c | 8 ++++---- Objects/bytearrayobject.c | 10 +++++----- Objects/bytesobject.c | 4 ++-- Objects/codeobject.c | 2 +- Objects/listobject.c | 10 +++++----- Objects/memoryobject.c | 2 +- Objects/tupleobject.c | 6 +++--- Objects/unicodeobject.c | 6 +++--- 10 files changed, 29 insertions(+), 29 deletions(-) diff --git a/Include/pymacro.h b/Include/pymacro.h index d7dcd6b20b2350..f858ff5bf90087 100644 --- a/Include/pymacro.h +++ b/Include/pymacro.h @@ -159,7 +159,7 @@ /// Return 1 if the specified index is outside the range [0, limit) static inline int -invalid_index(Py_ssize_t i, Py_ssize_t limit) +valid_index(Py_ssize_t i, Py_ssize_t limit) { /* The cast to size_t lets us use just a single comparison to check whether i is in the range: 0 <= i < limit. @@ -168,7 +168,7 @@ invalid_index(Py_ssize_t i, Py_ssize_t limit) optimization manual found at: https://www.agner.org/optimize/optimizing_cpp.pdf */ - return (size_t)i >= (size_t)limit; + return (size_t)i < (size_t)limit; } #endif /* Py_PYMACRO_H */ diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 5aeb85da42f58f..10723de7729a56 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1157,7 +1157,7 @@ deque_item(dequeobject *deque, Py_ssize_t i) PyObject *item; Py_ssize_t n, index=i; - if (invalid_index(i, Py_SIZE(deque))) { + if (!valid_index(i, Py_SIZE(deque))) { PyErr_SetString(PyExc_IndexError, "deque index out of range"); return NULL; } @@ -1252,7 +1252,7 @@ deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v) block *b; Py_ssize_t n, len=Py_SIZE(deque), halflen=(len+1)>>1, index=i; - if (invalid_index(i, len)) { + if (!valid_index(i, len)) { PyErr_SetString(PyExc_IndexError, "deque index out of range"); return -1; } @@ -2413,7 +2413,7 @@ tuplegetter_descr_get(PyObject *self, PyObject *obj, PyObject *type) return NULL; } - if (invalid_index(index, PyTuple_GET_SIZE(obj))) { + if (!valid_index(index, PyTuple_GET_SIZE(obj))) { PyErr_SetString(PyExc_IndexError, "tuple index out of range"); return NULL; } diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 5dbfd41401087c..db0fefcc28a4cc 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -804,7 +804,7 @@ array_length(arrayobject *a) static PyObject * array_item(arrayobject *a, Py_ssize_t i) { - if (invalid_index(i, Py_SIZE(a))) { + if (!valid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "array index out of range"); return NULL; } @@ -966,7 +966,7 @@ array_del_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) static int array_ass_item(arrayobject *a, Py_ssize_t i, PyObject *v) { - if (invalid_index(i, Py_SIZE(a))) { + if (!valid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); return -1; @@ -1249,7 +1249,7 @@ array_array_pop_impl(arrayobject *self, Py_ssize_t i) } if (i < 0) i += Py_SIZE(self); - if (invalid_index(i, Py_SIZE(self))) { + if (!valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); return NULL; } @@ -2407,7 +2407,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) return -1; if (i < 0) i += Py_SIZE(self); - if (invalid_index(i, Py_SIZE(self))) { + if (!valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); return -1; diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index 5d7793dea8c3bf..ab7a7a6614af7f 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -358,7 +358,7 @@ bytearray_irepeat(PyByteArrayObject *self, Py_ssize_t count) static PyObject * bytearray_getitem(PyByteArrayObject *self, Py_ssize_t i) { - if (invalid_index(i, Py_SIZE(self))) { + if (!valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return NULL; } @@ -377,7 +377,7 @@ bytearray_subscript(PyByteArrayObject *self, PyObject *index) if (i < 0) i += PyByteArray_GET_SIZE(self); - if (invalid_index(i, Py_SIZE(self))) { + if (!valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return NULL; } @@ -572,7 +572,7 @@ bytearray_setitem(PyByteArrayObject *self, Py_ssize_t i, PyObject *value) i += Py_SIZE(self); } - if (invalid_index(i, Py_SIZE(self))) { + if (!valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return -1; } @@ -612,7 +612,7 @@ bytearray_ass_subscript(PyByteArrayObject *self, PyObject *index, PyObject *valu i += PyByteArray_GET_SIZE(self); } - if (invalid_index(i, Py_SIZE(self))) { + if (!valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return -1; } @@ -1809,7 +1809,7 @@ bytearray_pop_impl(PyByteArrayObject *self, Py_ssize_t index) } if (index < 0) index += Py_SIZE(self); - if (invalid_index(index, Py_SIZE(self))) { + if (!valid_index(index, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); return NULL; } diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 726463ed13cd74..75aa99cd23686a 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -1484,7 +1484,7 @@ bytes_contains(PyObject *self, PyObject *arg) static PyObject * bytes_item(PyBytesObject *a, Py_ssize_t i) { - if (invalid_index(i, Py_SIZE(a))) { + if (!valid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "index out of range"); return NULL; } @@ -1591,7 +1591,7 @@ bytes_subscript(PyBytesObject* self, PyObject* item) return NULL; if (i < 0) i += PyBytes_GET_SIZE(self); - if (invalid_index(i, PyBytes_GET_SIZE(self))) { + if (!valid_index(i, PyBytes_GET_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "index out of range"); return NULL; diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 2837544b2adbcf..3e1898f816cf9e 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1346,7 +1346,7 @@ _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) { PyInterpreterState *interp = _PyInterpreterState_GET(); - if (!PyCode_Check(code) || invalid_index(index, interp->co_extra_user_count)) { + if (!PyCode_Check(code) || !valid_index(index, interp->co_extra_user_count)) { PyErr_BadInternalCall(); return -1; } diff --git a/Objects/listobject.c b/Objects/listobject.c index 378526c719cae2..4fc7a5b1cb18ef 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -234,7 +234,7 @@ PyList_GetItem(PyObject *op, Py_ssize_t i) PyErr_BadInternalCall(); return NULL; } - if (invalid_index(i, Py_SIZE(op))) { + if (!valid_index(i, Py_SIZE(op))) { _Py_DECLARE_STR(list_err, "list index out of range"); PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); return NULL; @@ -252,7 +252,7 @@ PyList_SetItem(PyObject *op, Py_ssize_t i, PyErr_BadInternalCall(); return -1; } - if (invalid_index(i, Py_SIZE(op))) { + if (!valid_index(i, Py_SIZE(op))) { Py_XDECREF(newitem); PyErr_SetString(PyExc_IndexError, "list assignment index out of range"); @@ -443,7 +443,7 @@ list_contains(PyListObject *a, PyObject *el) static PyObject * list_item(PyListObject *a, Py_ssize_t i) { - if (invalid_index(i, Py_SIZE(a))) { + if (!valid_index(i, Py_SIZE(a))) { PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); return NULL; } @@ -755,7 +755,7 @@ list_inplace_repeat(PyListObject *self, Py_ssize_t n) static int list_ass_item(PyListObject *a, Py_ssize_t i, PyObject *v) { - if (invalid_index(i, Py_SIZE(a))) { + if (!valid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "list assignment index out of range"); return -1; @@ -1006,7 +1006,7 @@ list_pop_impl(PyListObject *self, Py_ssize_t index) } if (index < 0) index += Py_SIZE(self); - if (invalid_index(index, Py_SIZE(self))) { + if (!valid_index(index, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); return NULL; } diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index ca822609474321..34d09274761d3e 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -2299,7 +2299,7 @@ lookup_dimension(const Py_buffer *view, char *ptr, int dim, Py_ssize_t index) if (index < 0) { index += nitems; } - if (invalid_index(index, nitems)) { + if (!valid_index(index, nitems)) { PyErr_Format(PyExc_IndexError, "index out of bounds on dimension %d", dim + 1); return NULL; diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 61dfee46d8bd85..0e128f0c03c3f5 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -100,7 +100,7 @@ PyTuple_GetItem(PyObject *op, Py_ssize_t i) PyErr_BadInternalCall(); return NULL; } - if (invalid_index(i, Py_SIZE(op))) { + if (!valid_index(i, Py_SIZE(op))) { PyErr_SetString(PyExc_IndexError, "tuple index out of range"); return NULL; } @@ -116,7 +116,7 @@ PyTuple_SetItem(PyObject *op, Py_ssize_t i, PyObject *newitem) PyErr_BadInternalCall(); return -1; } - if (invalid_index(i, Py_SIZE(op))) { + if (!valid_index(i, Py_SIZE(op))) { Py_XDECREF(newitem); PyErr_SetString(PyExc_IndexError, "tuple assignment index out of range"); @@ -361,7 +361,7 @@ tuplecontains(PyTupleObject *a, PyObject *el) static PyObject * tupleitem(PyTupleObject *a, Py_ssize_t i) { - if (invalid_index(i, Py_SIZE(a))) { + if (!valid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "tuple index out of range"); return NULL; } diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 3082b1d4bf9328..abfe89877bb440 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -1436,11 +1436,11 @@ PyUnicode_CopyCharacters(PyObject *to, Py_ssize_t to_start, return -1; } - if (invalid_index(from_start, PyUnicode_GET_LENGTH(from))) { + if (!valid_index(from_start, PyUnicode_GET_LENGTH(from))) { PyErr_SetString(PyExc_IndexError, "string index out of range"); return -1; } - if (invalid_index(to_start, PyUnicode_GET_LENGTH(to))) { + if (!valid_index(to_start, PyUnicode_GET_LENGTH(to))) { PyErr_SetString(PyExc_IndexError, "string index out of range"); return -1; } @@ -3721,7 +3721,7 @@ PyUnicode_ReadChar(PyObject *unicode, Py_ssize_t index) PyErr_BadArgument(); return (Py_UCS4)-1; } - if (invalid_index(index, PyUnicode_GET_LENGTH(unicode))) { + if (!valid_index(index, PyUnicode_GET_LENGTH(unicode))) { PyErr_SetString(PyExc_IndexError, "string index out of range"); return (Py_UCS4)-1; } From c3561307dc70af528ce2168c13b4e18bdd3f92d4 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 26 Nov 2022 22:13:12 +0100 Subject: [PATCH 08/29] change invalid_index to valid_index --- Modules/mmapmodule.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 0b412b7d4f5bd1..6cdc88c888ee08 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -920,7 +920,7 @@ static PyObject * mmap_item(mmap_object *self, Py_ssize_t i) { CHECK_VALID(NULL); - if (invalid_index(i, self->size)) { + if (!valid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return NULL; } @@ -937,7 +937,7 @@ mmap_subscript(mmap_object *self, PyObject *item) return NULL; if (i < 0) i += self->size; - if (invalid_index(i, self->size)) { + if (!valid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return NULL; @@ -988,7 +988,7 @@ mmap_ass_item(mmap_object *self, Py_ssize_t i, PyObject *v) const char *buf; CHECK_VALID(-1); - if (invalid_index(i, self->size)) { + if (!valid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return -1; } @@ -1025,7 +1025,7 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value) return -1; if (i < 0) i += self->size; - if (invalid_index(i, self->size)) { + if (!valid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return -1; From 329d6edd49456158ebd8cffd2555d021ced653b5 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 26 Nov 2022 22:13:32 +0100 Subject: [PATCH 09/29] revert changes to _collectionsmodule.c on request of the author --- Modules/_collectionsmodule.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 10723de7729a56..81c0bc41d6e61c 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1149,6 +1149,13 @@ PyDoc_STRVAR(insert_doc, PyDoc_STRVAR(remove_doc, "D.remove(value) -- remove first occurrence of value."); +static int +valid_index(Py_ssize_t i, Py_ssize_t limit) +{ + /* The cast to size_t lets us use just a single comparison + to check whether i is in the range: 0 <= i < limit */ + return (size_t)i < (size_t)limit; +} static PyObject * deque_item(dequeobject *deque, Py_ssize_t i) From bbdb3b5ffa950cfd5680a2d6b8d1a4664a8b354f Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 26 Nov 2022 22:20:20 +0100 Subject: [PATCH 10/29] make valid_index private --- Modules/arraymodule.c | 1 + Modules/mmapmodule.c | 1 + Objects/codeobject.c | 1 + 3 files changed, 3 insertions(+) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index db0fefcc28a4cc..656175c6923acf 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -9,6 +9,7 @@ #define PY_SSIZE_T_CLEAN #include "Python.h" +#include "pycore_abstract.h" // valid_index #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_bytesobject.h" // _PyBytes_Repeat #include "structmember.h" // PyMemberDef diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 6cdc88c888ee08..a60bafce965378 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -24,6 +24,7 @@ #define PY_SSIZE_T_CLEAN #include +#include "pycore_abstract.h" // valid_index #include "pycore_bytesobject.h" // _PyBytes_Find() #include "pycore_fileutils.h" // _Py_stat_struct #include "structmember.h" // PyMemberDef diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 3e1898f816cf9e..0b7eb995a7fe50 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -3,6 +3,7 @@ #include "Python.h" #include "opcode.h" #include "structmember.h" // PyMemberDef +#include "pycore_abstract.h" // valid_index #include "pycore_code.h" // _PyCodeConstructor #include "pycore_frame.h" // FRAME_SPECIALS_SIZE #include "pycore_interp.h" // PyInterpreterState.co_extra_freefuncs From 45d4abf36fb4167f0a06e5d33fc4733941c82b64 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 26 Nov 2022 22:35:36 +0100 Subject: [PATCH 11/29] change static inline to macro --- Include/internal/pycore_abstract.h | 10 ++++++++++ Include/internal/pycore_long.h | 10 +++------- Include/pymacro.h | 14 -------------- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/Include/internal/pycore_abstract.h b/Include/internal/pycore_abstract.h index b1afb2dc7be65e..5b53819f40971a 100644 --- a/Include/internal/pycore_abstract.h +++ b/Include/internal/pycore_abstract.h @@ -8,6 +8,16 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif +/* Return 1 if the specified index is outside the range[0, limit) */ +#define valid_index(index, limit) ((size_t)index < (size_t)limit) + /* The cast to size_t lets us use just a single comparison + to check whether index is in the range: 0 <= index < limit. + + See: Section 14.2 "Bounds Checking" in the Agner Fog + optimization manual found at: + https://www.agner.org/optimize/optimizing_cpp.pdf + */ + // Fast inlined version of PyIndex_Check() static inline int _PyIndex_Check(PyObject *obj) diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h index 6ffa2085072f1e..bf4cc3e482d40e 100644 --- a/Include/internal/pycore_long.h +++ b/Include/internal/pycore_long.h @@ -78,13 +78,9 @@ static inline PyObject* _PyLong_FromUnsignedChar(unsigned char i) return Py_NewRef((PyObject *)&_PyLong_SMALL_INTS[_PY_NSMALLNEGINTS+i]); } -/// Return 0 for nonnegative, zero-or-one-digit ints, 1 otherwise -static inline int -_PyLong_Negative_or_multi_digit_int(PyObject* sub) { - assert(PyLong_CheckExact(sub)); - Py_ssize_t signed_magnitude = Py_SIZE(sub); - return ((size_t)signed_magnitude) > 1; -} +/* Return 1 of the argument is negative or a multi-digit int */ +#define _PyLong_Negative_or_multi_digit_int(sub) (assert(PyLong_CheckExact(sub)), \ + ((size_t)Py_SIZE(sub)) > 1) PyObject *_PyLong_Add(PyLongObject *left, PyLongObject *right); PyObject *_PyLong_Multiply(PyLongObject *left, PyLongObject *right); diff --git a/Include/pymacro.h b/Include/pymacro.h index f858ff5bf90087..e37cda44c5ebf1 100644 --- a/Include/pymacro.h +++ b/Include/pymacro.h @@ -157,18 +157,4 @@ // "comparison of unsigned expression in '< 0' is always false". #define _Py_IS_TYPE_SIGNED(type) ((type)(-1) <= 0) -/// Return 1 if the specified index is outside the range [0, limit) -static inline int -valid_index(Py_ssize_t i, Py_ssize_t limit) -{ - /* The cast to size_t lets us use just a single comparison - to check whether i is in the range: 0 <= i < limit. - - See: Section 14.2 "Bounds Checking" in the Agner Fog - optimization manual found at: - https://www.agner.org/optimize/optimizing_cpp.pdf - */ - return (size_t)i < (size_t)limit; -} - #endif /* Py_PYMACRO_H */ From e78d12961f470e14c58ecab8a6dfa3794adc8d72 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 26 Nov 2022 22:51:48 +0100 Subject: [PATCH 12/29] typo --- Include/internal/pycore_long.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h index bf4cc3e482d40e..f1cd009c466e43 100644 --- a/Include/internal/pycore_long.h +++ b/Include/internal/pycore_long.h @@ -78,7 +78,7 @@ static inline PyObject* _PyLong_FromUnsignedChar(unsigned char i) return Py_NewRef((PyObject *)&_PyLong_SMALL_INTS[_PY_NSMALLNEGINTS+i]); } -/* Return 1 of the argument is negative or a multi-digit int */ +/* Return 1 if the argument is negative or a multi-digit int */ #define _PyLong_Negative_or_multi_digit_int(sub) (assert(PyLong_CheckExact(sub)), \ ((size_t)Py_SIZE(sub)) > 1) From 5cc1dfc50a544126a56b29090310eea7c46582e9 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 26 Nov 2022 22:52:11 +0100 Subject: [PATCH 13/29] unto changes to _collectionsmodule.c --- Modules/_collectionsmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_collectionsmodule.c b/Modules/_collectionsmodule.c index 81c0bc41d6e61c..5fa583821889f3 100644 --- a/Modules/_collectionsmodule.c +++ b/Modules/_collectionsmodule.c @@ -1154,7 +1154,7 @@ valid_index(Py_ssize_t i, Py_ssize_t limit) { /* The cast to size_t lets us use just a single comparison to check whether i is in the range: 0 <= i < limit */ - return (size_t)i < (size_t)limit; + return (size_t) i < (size_t) limit; } static PyObject * From d04aa180b141e52e5c74d26d8bcb7a2e8b94d90c Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 28 Nov 2022 23:49:59 +0100 Subject: [PATCH 14/29] rename to static inline int _Py_is_valid_index --- Include/internal/pycore_abstract.h | 10 ++++++---- Include/internal/pycore_long.h | 8 ++++++-- Modules/arraymodule.c | 8 ++++---- Modules/mmapmodule.c | 8 ++++---- Objects/bytearrayobject.c | 10 +++++----- Objects/bytesobject.c | 4 ++-- Objects/codeobject.c | 2 +- Objects/listobject.c | 10 +++++----- Objects/memoryobject.c | 2 +- Objects/tupleobject.c | 6 +++--- Objects/unicodeobject.c | 6 +++--- 11 files changed, 40 insertions(+), 34 deletions(-) diff --git a/Include/internal/pycore_abstract.h b/Include/internal/pycore_abstract.h index 5b53819f40971a..aef0b6cb1efe95 100644 --- a/Include/internal/pycore_abstract.h +++ b/Include/internal/pycore_abstract.h @@ -8,15 +8,17 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif -/* Return 1 if the specified index is outside the range[0, limit) */ -#define valid_index(index, limit) ((size_t)index < (size_t)limit) +/// Return 1 if the specified index is outside the range [0, limit) +static inline int _Py_is_valid_index(Py_ssize_t i, Py_ssize_t limit) +{ /* The cast to size_t lets us use just a single comparison - to check whether index is in the range: 0 <= index < limit. - + to check whether i is in the range: 0 <= i < limit. See: Section 14.2 "Bounds Checking" in the Agner Fog optimization manual found at: https://www.agner.org/optimize/optimizing_cpp.pdf */ + return (size_t)i < (size_t)limit; +} // Fast inlined version of PyIndex_Check() static inline int diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h index f1cd009c466e43..8a42bf5f11e7dc 100644 --- a/Include/internal/pycore_long.h +++ b/Include/internal/pycore_long.h @@ -79,8 +79,12 @@ static inline PyObject* _PyLong_FromUnsignedChar(unsigned char i) } /* Return 1 if the argument is negative or a multi-digit int */ -#define _PyLong_Negative_or_multi_digit_int(sub) (assert(PyLong_CheckExact(sub)), \ - ((size_t)Py_SIZE(sub)) > 1) +static inline int +_PyLong_Negative_or_multi_digit_int(PyObject* sub) { + assert(PyLong_CheckExact(sub)); + Py_ssize_t signed_magnitude = Py_SIZE(sub); + return ((size_t)signed_magnitude) > 1; +} PyObject *_PyLong_Add(PyLongObject *left, PyLongObject *right); PyObject *_PyLong_Multiply(PyLongObject *left, PyLongObject *right); diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 656175c6923acf..1fe6123fd3c603 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -805,7 +805,7 @@ array_length(arrayobject *a) static PyObject * array_item(arrayobject *a, Py_ssize_t i) { - if (!valid_index(i, Py_SIZE(a))) { + if (!_Py_is_valid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "array index out of range"); return NULL; } @@ -967,7 +967,7 @@ array_del_slice(arrayobject *a, Py_ssize_t ilow, Py_ssize_t ihigh) static int array_ass_item(arrayobject *a, Py_ssize_t i, PyObject *v) { - if (!valid_index(i, Py_SIZE(a))) { + if (!_Py_is_valid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); return -1; @@ -1250,7 +1250,7 @@ array_array_pop_impl(arrayobject *self, Py_ssize_t i) } if (i < 0) i += Py_SIZE(self); - if (!valid_index(i, Py_SIZE(self))) { + if (!_Py_is_valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); return NULL; } @@ -2408,7 +2408,7 @@ array_ass_subscr(arrayobject* self, PyObject* item, PyObject* value) return -1; if (i < 0) i += Py_SIZE(self); - if (!valid_index(i, Py_SIZE(self))) { + if (!_Py_is_valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "array assignment index out of range"); return -1; diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index a60bafce965378..1fd8c5a91ba32c 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -921,7 +921,7 @@ static PyObject * mmap_item(mmap_object *self, Py_ssize_t i) { CHECK_VALID(NULL); - if (!valid_index(i, self->size)) { + if (!_Py_is_valid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return NULL; } @@ -938,7 +938,7 @@ mmap_subscript(mmap_object *self, PyObject *item) return NULL; if (i < 0) i += self->size; - if (!valid_index(i, self->size)) { + if (!_Py_is_valid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return NULL; @@ -989,7 +989,7 @@ mmap_ass_item(mmap_object *self, Py_ssize_t i, PyObject *v) const char *buf; CHECK_VALID(-1); - if (!valid_index(i, self->size)) { + if (!_Py_is_valid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return -1; } @@ -1026,7 +1026,7 @@ mmap_ass_subscript(mmap_object *self, PyObject *item, PyObject *value) return -1; if (i < 0) i += self->size; - if (!valid_index(i, self->size)) { + if (!_Py_is_valid_index(i, self->size)) { PyErr_SetString(PyExc_IndexError, "mmap index out of range"); return -1; diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index ab7a7a6614af7f..c926b5a9d143f3 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -358,7 +358,7 @@ bytearray_irepeat(PyByteArrayObject *self, Py_ssize_t count) static PyObject * bytearray_getitem(PyByteArrayObject *self, Py_ssize_t i) { - if (!valid_index(i, Py_SIZE(self))) { + if (!_Py_is_valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return NULL; } @@ -377,7 +377,7 @@ bytearray_subscript(PyByteArrayObject *self, PyObject *index) if (i < 0) i += PyByteArray_GET_SIZE(self); - if (!valid_index(i, Py_SIZE(self))) { + if (!_Py_is_valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return NULL; } @@ -572,7 +572,7 @@ bytearray_setitem(PyByteArrayObject *self, Py_ssize_t i, PyObject *value) i += Py_SIZE(self); } - if (!valid_index(i, Py_SIZE(self))) { + if (!_Py_is_valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return -1; } @@ -612,7 +612,7 @@ bytearray_ass_subscript(PyByteArrayObject *self, PyObject *index, PyObject *valu i += PyByteArray_GET_SIZE(self); } - if (!valid_index(i, Py_SIZE(self))) { + if (!_Py_is_valid_index(i, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "bytearray index out of range"); return -1; } @@ -1809,7 +1809,7 @@ bytearray_pop_impl(PyByteArrayObject *self, Py_ssize_t index) } if (index < 0) index += Py_SIZE(self); - if (!valid_index(index, Py_SIZE(self))) { + if (!_Py_is_valid_index(index, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); return NULL; } diff --git a/Objects/bytesobject.c b/Objects/bytesobject.c index 75aa99cd23686a..eb8bc2a698a017 100644 --- a/Objects/bytesobject.c +++ b/Objects/bytesobject.c @@ -1484,7 +1484,7 @@ bytes_contains(PyObject *self, PyObject *arg) static PyObject * bytes_item(PyBytesObject *a, Py_ssize_t i) { - if (!valid_index(i, Py_SIZE(a))) { + if (!_Py_is_valid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "index out of range"); return NULL; } @@ -1591,7 +1591,7 @@ bytes_subscript(PyBytesObject* self, PyObject* item) return NULL; if (i < 0) i += PyBytes_GET_SIZE(self); - if (!valid_index(i, PyBytes_GET_SIZE(self))) { + if (!_Py_is_valid_index(i, PyBytes_GET_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "index out of range"); return NULL; diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 0b7eb995a7fe50..030b5861ff8c92 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1347,7 +1347,7 @@ _PyCode_SetExtra(PyObject *code, Py_ssize_t index, void *extra) { PyInterpreterState *interp = _PyInterpreterState_GET(); - if (!PyCode_Check(code) || !valid_index(index, interp->co_extra_user_count)) { + if (!PyCode_Check(code) || !_Py_is_valid_index(index, interp->co_extra_user_count)) { PyErr_BadInternalCall(); return -1; } diff --git a/Objects/listobject.c b/Objects/listobject.c index 4fc7a5b1cb18ef..4eb7cf9fcc6ec2 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -234,7 +234,7 @@ PyList_GetItem(PyObject *op, Py_ssize_t i) PyErr_BadInternalCall(); return NULL; } - if (!valid_index(i, Py_SIZE(op))) { + if (!_Py_is_valid_index(i, Py_SIZE(op))) { _Py_DECLARE_STR(list_err, "list index out of range"); PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); return NULL; @@ -252,7 +252,7 @@ PyList_SetItem(PyObject *op, Py_ssize_t i, PyErr_BadInternalCall(); return -1; } - if (!valid_index(i, Py_SIZE(op))) { + if (!_Py_is_valid_index(i, Py_SIZE(op))) { Py_XDECREF(newitem); PyErr_SetString(PyExc_IndexError, "list assignment index out of range"); @@ -443,7 +443,7 @@ list_contains(PyListObject *a, PyObject *el) static PyObject * list_item(PyListObject *a, Py_ssize_t i) { - if (!valid_index(i, Py_SIZE(a))) { + if (!_Py_is_valid_index(i, Py_SIZE(a))) { PyErr_SetObject(PyExc_IndexError, &_Py_STR(list_err)); return NULL; } @@ -755,7 +755,7 @@ list_inplace_repeat(PyListObject *self, Py_ssize_t n) static int list_ass_item(PyListObject *a, Py_ssize_t i, PyObject *v) { - if (!valid_index(i, Py_SIZE(a))) { + if (!_Py_is_valid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "list assignment index out of range"); return -1; @@ -1006,7 +1006,7 @@ list_pop_impl(PyListObject *self, Py_ssize_t index) } if (index < 0) index += Py_SIZE(self); - if (!valid_index(index, Py_SIZE(self))) { + if (!_Py_is_valid_index(index, Py_SIZE(self))) { PyErr_SetString(PyExc_IndexError, "pop index out of range"); return NULL; } diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 34d09274761d3e..7a729f966c34a8 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -2299,7 +2299,7 @@ lookup_dimension(const Py_buffer *view, char *ptr, int dim, Py_ssize_t index) if (index < 0) { index += nitems; } - if (!valid_index(index, nitems)) { + if (!_Py_is_valid_index(index, nitems)) { PyErr_Format(PyExc_IndexError, "index out of bounds on dimension %d", dim + 1); return NULL; diff --git a/Objects/tupleobject.c b/Objects/tupleobject.c index 0e128f0c03c3f5..a53b3731353321 100644 --- a/Objects/tupleobject.c +++ b/Objects/tupleobject.c @@ -100,7 +100,7 @@ PyTuple_GetItem(PyObject *op, Py_ssize_t i) PyErr_BadInternalCall(); return NULL; } - if (!valid_index(i, Py_SIZE(op))) { + if (!_Py_is_valid_index(i, Py_SIZE(op))) { PyErr_SetString(PyExc_IndexError, "tuple index out of range"); return NULL; } @@ -116,7 +116,7 @@ PyTuple_SetItem(PyObject *op, Py_ssize_t i, PyObject *newitem) PyErr_BadInternalCall(); return -1; } - if (!valid_index(i, Py_SIZE(op))) { + if (!_Py_is_valid_index(i, Py_SIZE(op))) { Py_XDECREF(newitem); PyErr_SetString(PyExc_IndexError, "tuple assignment index out of range"); @@ -361,7 +361,7 @@ tuplecontains(PyTupleObject *a, PyObject *el) static PyObject * tupleitem(PyTupleObject *a, Py_ssize_t i) { - if (!valid_index(i, Py_SIZE(a))) { + if (!_Py_is_valid_index(i, Py_SIZE(a))) { PyErr_SetString(PyExc_IndexError, "tuple index out of range"); return NULL; } diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index abfe89877bb440..904853b7032cea 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -1436,11 +1436,11 @@ PyUnicode_CopyCharacters(PyObject *to, Py_ssize_t to_start, return -1; } - if (!valid_index(from_start, PyUnicode_GET_LENGTH(from))) { + if (!_Py_is_valid_index(from_start, PyUnicode_GET_LENGTH(from))) { PyErr_SetString(PyExc_IndexError, "string index out of range"); return -1; } - if (!valid_index(to_start, PyUnicode_GET_LENGTH(to))) { + if (!_Py_is_valid_index(to_start, PyUnicode_GET_LENGTH(to))) { PyErr_SetString(PyExc_IndexError, "string index out of range"); return -1; } @@ -3721,7 +3721,7 @@ PyUnicode_ReadChar(PyObject *unicode, Py_ssize_t index) PyErr_BadArgument(); return (Py_UCS4)-1; } - if (!valid_index(index, PyUnicode_GET_LENGTH(unicode))) { + if (!_Py_is_valid_index(index, PyUnicode_GET_LENGTH(unicode))) { PyErr_SetString(PyExc_IndexError, "string index out of range"); return (Py_UCS4)-1; } From d6c614ce62bb79501fa8cc0ae997c3247da03a7a Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 29 Nov 2022 00:02:02 +0100 Subject: [PATCH 15/29] update news item --- .../2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst index 316a474fabf2b1..cbd9961e0461ce 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst @@ -1 +1 @@ -Add method to calculate invalid_index +Add optimized method `_Py_is_valid_index` to calculate valid indices From 77e8cc475922ddcb5c6882d271724f124ab837c6 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 29 Nov 2022 00:07:59 +0100 Subject: [PATCH 16/29] double backticks --- .../2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst index cbd9961e0461ce..ba664048526dfd 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst @@ -1 +1 @@ -Add optimized method `_Py_is_valid_index` to calculate valid indices +Add optimized method ``_Py_is_valid_index`` to calculate valid indices From 6e334bca225020643112389ce3d7be37202003dc Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 29 Nov 2022 12:34:34 +0100 Subject: [PATCH 17/29] fix docstring --- Include/internal/pycore_abstract.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_abstract.h b/Include/internal/pycore_abstract.h index aef0b6cb1efe95..3e33651d75d1bb 100644 --- a/Include/internal/pycore_abstract.h +++ b/Include/internal/pycore_abstract.h @@ -8,7 +8,7 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif -/// Return 1 if the specified index is outside the range [0, limit) +/// Return 1 if the specified index is in the range [0, limit) static inline int _Py_is_valid_index(Py_ssize_t i, Py_ssize_t limit) { /* The cast to size_t lets us use just a single comparison From fe8b4623aa0b44d31f3c299a0bb399cd6d732d56 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 6 Dec 2022 22:07:36 +0100 Subject: [PATCH 18/29] Update Include/internal/pycore_abstract.h Co-authored-by: Victor Stinner --- Include/internal/pycore_abstract.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_abstract.h b/Include/internal/pycore_abstract.h index 3e33651d75d1bb..bffae5c0328746 100644 --- a/Include/internal/pycore_abstract.h +++ b/Include/internal/pycore_abstract.h @@ -8,7 +8,7 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif -/// Return 1 if the specified index is in the range [0, limit) +// Return 1 if 0 <= index < limit static inline int _Py_is_valid_index(Py_ssize_t i, Py_ssize_t limit) { /* The cast to size_t lets us use just a single comparison From 901dd4ce7d9ff64be1c596eb90876dfc987da4f2 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 6 Dec 2022 22:28:43 +0100 Subject: [PATCH 19/29] revert changes with _PyLong_negative_or_multi_digit_int --- Include/internal/pycore_long.h | 8 -------- Python/bytecodes.c | 9 ++++++--- Python/generated_cases.c.h | 9 ++++++--- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h index 8a42bf5f11e7dc..30c97b7edc98e1 100644 --- a/Include/internal/pycore_long.h +++ b/Include/internal/pycore_long.h @@ -78,14 +78,6 @@ static inline PyObject* _PyLong_FromUnsignedChar(unsigned char i) return Py_NewRef((PyObject *)&_PyLong_SMALL_INTS[_PY_NSMALLNEGINTS+i]); } -/* Return 1 if the argument is negative or a multi-digit int */ -static inline int -_PyLong_Negative_or_multi_digit_int(PyObject* sub) { - assert(PyLong_CheckExact(sub)); - Py_ssize_t signed_magnitude = Py_SIZE(sub); - return ((size_t)signed_magnitude) > 1; -} - PyObject *_PyLong_Add(PyLongObject *left, PyLongObject *right); PyObject *_PyLong_Multiply(PyLongObject *left, PyLongObject *right); PyObject *_PyLong_Subtract(PyLongObject *left, PyLongObject *right); diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 89a2b17251491f..b3c7ca42fc90cd 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -383,7 +383,8 @@ dummy_func( DEOPT_IF(!PyList_CheckExact(list), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyList_Size(list) - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); + Py_ssize_t signed_magnitude = Py_SIZE(sub); + DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyList_GET_SIZE(list), BINARY_SUBSCR); @@ -401,7 +402,8 @@ dummy_func( DEOPT_IF(!PyTuple_CheckExact(tuple), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyTuple_Size(list) - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); + Py_ssize_t signed_magnitude = Py_SIZE(sub); + DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyTuple_GET_SIZE(tuple), BINARY_SUBSCR); @@ -505,7 +507,8 @@ dummy_func( DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), STORE_SUBSCR); + Py_ssize_t signed_magnitude = Py_SIZE(sub); + DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 97480e70e0661b..e491fb6fe4e7e3 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -367,7 +367,8 @@ DEOPT_IF(!PyList_CheckExact(list), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyList_Size(list) - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); + Py_ssize_t signed_magnitude = Py_SIZE(sub); + DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyList_GET_SIZE(list), BINARY_SUBSCR); @@ -392,7 +393,8 @@ DEOPT_IF(!PyTuple_CheckExact(tuple), BINARY_SUBSCR); // Deopt unless 0 <= sub < PyTuple_Size(list) - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), BINARY_SUBSCR); + Py_ssize_t signed_magnitude = Py_SIZE(sub); + DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); assert(((PyLongObject *)_PyLong_GetZero())->ob_digit[0] == 0); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; DEOPT_IF(index >= PyTuple_GET_SIZE(tuple), BINARY_SUBSCR); @@ -516,7 +518,8 @@ DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - DEOPT_IF(_PyLong_Negative_or_multi_digit_int(sub), STORE_SUBSCR); + Py_ssize_t signed_magnitude = Py_SIZE(sub); + DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); From 3fbbaae947b94487e219e74166e6c63c5ca596e8 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 6 Dec 2022 22:30:30 +0100 Subject: [PATCH 20/29] update documentation --- Include/internal/pycore_abstract.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Include/internal/pycore_abstract.h b/Include/internal/pycore_abstract.h index bffae5c0328746..c4f70d4a42de53 100644 --- a/Include/internal/pycore_abstract.h +++ b/Include/internal/pycore_abstract.h @@ -16,6 +16,10 @@ static inline int _Py_is_valid_index(Py_ssize_t i, Py_ssize_t limit) See: Section 14.2 "Bounds Checking" in the Agner Fog optimization manual found at: https://www.agner.org/optimize/optimizing_cpp.pdf + + The function relies on twos-complement representation, and is not + affected by -fwrapv, -fno-wrapv and -ftrapv compiler options + of GCC and clang */ return (size_t)i < (size_t)limit; } From 366f2eca62e9bbf3d95c045dec34a1e77f5eff56 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 7 Dec 2022 20:51:31 +0100 Subject: [PATCH 21/29] revert changes in bytecodes.c --- Python/bytecodes.c | 3 +-- Python/generated_cases.c.h | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index b3c7ca42fc90cd..a1f910da8ed54a 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -507,8 +507,7 @@ dummy_func( DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); + DEOPT_IF(((size_t)Py_SIZE(sub)) > 1, STORE_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index e491fb6fe4e7e3..ae8fdd5e99c3dc 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -518,8 +518,7 @@ DEOPT_IF(!PyList_CheckExact(list), STORE_SUBSCR); // Ensure nonnegative, zero-or-one-digit ints. - Py_ssize_t signed_magnitude = Py_SIZE(sub); - DEOPT_IF(((size_t)signed_magnitude) > 1, BINARY_SUBSCR); + DEOPT_IF(((size_t)Py_SIZE(sub)) > 1, STORE_SUBSCR); Py_ssize_t index = ((PyLongObject*)sub)->ob_digit[0]; // Ensure index < len(list) DEOPT_IF(index >= PyList_GET_SIZE(list), STORE_SUBSCR); From 77959298861938ba2ebb47979ba25d9e69f75aa9 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 8 Dec 2022 14:10:07 +0100 Subject: [PATCH 22/29] Update Include/internal/pycore_abstract.h --- Include/internal/pycore_abstract.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_abstract.h b/Include/internal/pycore_abstract.h index c4f70d4a42de53..9b8067512f88fe 100644 --- a/Include/internal/pycore_abstract.h +++ b/Include/internal/pycore_abstract.h @@ -21,7 +21,7 @@ static inline int _Py_is_valid_index(Py_ssize_t i, Py_ssize_t limit) affected by -fwrapv, -fno-wrapv and -ftrapv compiler options of GCC and clang */ - return (size_t)i < (size_t)limit; + return (size_t)index < (size_t)limit; } // Fast inlined version of PyIndex_Check() From 87f65e67bb49fe545dcb20b0d50daa6932a560e9 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 8 Dec 2022 14:10:15 +0100 Subject: [PATCH 23/29] Update Include/internal/pycore_abstract.h --- Include/internal/pycore_abstract.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_abstract.h b/Include/internal/pycore_abstract.h index 9b8067512f88fe..8d1baa40741771 100644 --- a/Include/internal/pycore_abstract.h +++ b/Include/internal/pycore_abstract.h @@ -9,7 +9,7 @@ extern "C" { #endif // Return 1 if 0 <= index < limit -static inline int _Py_is_valid_index(Py_ssize_t i, Py_ssize_t limit) +static inline int _Py_is_valid_index(Py_ssize_t index, Py_ssize_t limit) { /* The cast to size_t lets us use just a single comparison to check whether i is in the range: 0 <= i < limit. From f7b22bf0beaa9385a0aa186b2d4b18fc3424ed18 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 8 Dec 2022 14:13:58 +0100 Subject: [PATCH 24/29] Update Objects/codeobject.c --- Objects/codeobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 030b5861ff8c92..f583289395cd8a 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -3,7 +3,7 @@ #include "Python.h" #include "opcode.h" #include "structmember.h" // PyMemberDef -#include "pycore_abstract.h" // valid_index +#include "pycore_abstract.h" // _Py_is_valid_index() #include "pycore_code.h" // _PyCodeConstructor #include "pycore_frame.h" // FRAME_SPECIALS_SIZE #include "pycore_interp.h" // PyInterpreterState.co_extra_freefuncs From 1e47517543ed1d4c5d4dc9f9c83976d3f83f9a1f Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 8 Dec 2022 14:14:08 +0100 Subject: [PATCH 25/29] Update Modules/mmapmodule.c --- Modules/mmapmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/mmapmodule.c b/Modules/mmapmodule.c index 1fd8c5a91ba32c..a8925fe3272c62 100644 --- a/Modules/mmapmodule.c +++ b/Modules/mmapmodule.c @@ -24,7 +24,7 @@ #define PY_SSIZE_T_CLEAN #include -#include "pycore_abstract.h" // valid_index +#include "pycore_abstract.h" // _Py_is_valid_index() #include "pycore_bytesobject.h" // _PyBytes_Find() #include "pycore_fileutils.h" // _Py_stat_struct #include "structmember.h" // PyMemberDef From e510943d2a64db4de852fda174a1afab7e87dc6e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 8 Dec 2022 14:14:19 +0100 Subject: [PATCH 26/29] Update Modules/arraymodule.c --- Modules/arraymodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index 1fe6123fd3c603..57f7ff870055f0 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -9,7 +9,7 @@ #define PY_SSIZE_T_CLEAN #include "Python.h" -#include "pycore_abstract.h" // valid_index +#include "pycore_abstract.h" // _Py_is_valid_index() #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_bytesobject.h" // _PyBytes_Repeat #include "structmember.h" // PyMemberDef From 61bd3a4fdef98b19f3a6c4c51711094af2d7778f Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 10 Dec 2022 21:33:49 +0100 Subject: [PATCH 27/29] remove news entry --- .../2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst deleted file mode 100644 index ba664048526dfd..00000000000000 --- a/Misc/NEWS.d/next/Core and Builtins/2022-11-25-09-34-57.gh-issue-99761.sRqH37.rst +++ /dev/null @@ -1 +0,0 @@ -Add optimized method ``_Py_is_valid_index`` to calculate valid indices From 6f0e26e5dcee6d0088d40d0b9db55d6134d42489 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 10 Dec 2022 21:37:18 +0100 Subject: [PATCH 28/29] remove comment about twos-complement --- Include/internal/pycore_abstract.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_abstract.h b/Include/internal/pycore_abstract.h index 8d1baa40741771..2694e245b61101 100644 --- a/Include/internal/pycore_abstract.h +++ b/Include/internal/pycore_abstract.h @@ -17,9 +17,8 @@ static inline int _Py_is_valid_index(Py_ssize_t index, Py_ssize_t limit) optimization manual found at: https://www.agner.org/optimize/optimizing_cpp.pdf - The function relies on twos-complement representation, and is not - affected by -fwrapv, -fno-wrapv and -ftrapv compiler options - of GCC and clang + The function is not affected by -fwrapv, -fno-wrapv and -ftrapv + compiler options of GCC and clang */ return (size_t)index < (size_t)limit; } From 4377b07ce32ec5e28ed9425a208fe22de57bdda4 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 20 Feb 2023 10:51:49 +0100 Subject: [PATCH 29/29] review comments --- Include/internal/pycore_abstract.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_abstract.h b/Include/internal/pycore_abstract.h index 2694e245b61101..8d92d8fe07651a 100644 --- a/Include/internal/pycore_abstract.h +++ b/Include/internal/pycore_abstract.h @@ -8,7 +8,8 @@ extern "C" { # error "this header requires Py_BUILD_CORE define" #endif -// Return 1 if 0 <= index < limit +// Return 1 if 0 <= index < limit and 0 otherwise +// The argument limit should be non-negative static inline int _Py_is_valid_index(Py_ssize_t index, Py_ssize_t limit) { /* The cast to size_t lets us use just a single comparison @@ -20,6 +21,7 @@ static inline int _Py_is_valid_index(Py_ssize_t index, Py_ssize_t limit) The function is not affected by -fwrapv, -fno-wrapv and -ftrapv compiler options of GCC and clang */ + assert(limit >= 0); return (size_t)index < (size_t)limit; }