From 3f8483cad2f3b94600c3ecf3f0bb220bb1e61d7d Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 28 Jun 2023 03:45:57 +0200 Subject: gh-106168: PyTuple_SET_ITEM() now checks the index (#106164) PyTuple_SET_ITEM() and PyList_SET_ITEM() now check the index argument with an assertion if Python is built in debug mode or is built with assertions. * list_extend() and _PyList_AppendTakeRef() now set the list size before calling PyList_SET_ITEM(). * PyStructSequence_GetItem() and PyStructSequence_SetItem() now check the index argument: must be lesser than REAL_SIZE(op). * PyStructSequence_GET_ITEM() and PyStructSequence_SET_ITEM() are now aliases to PyStructSequence_GetItem() and PyStructSequence_SetItem(). --- Doc/c-api/list.rst | 4 ++++ Doc/c-api/tuple.rst | 23 +++++++++++++++------- Doc/whatsnew/3.13.rst | 6 ++++++ Include/cpython/listobject.h | 2 ++ Include/cpython/tupleobject.h | 2 ++ Include/internal/pycore_list.h | 2 +- Include/structseq.h | 13 +++++------- .../2023-06-28-02-30-50.gh-issue-106168.NFOZPv.rst | 5 +++++ Objects/listobject.c | 5 +++-- Objects/structseq.c | 23 +++++++++++++++++----- 10 files changed, 62 insertions(+), 23 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2023-06-28-02-30-50.gh-issue-106168.NFOZPv.rst diff --git a/Doc/c-api/list.rst b/Doc/c-api/list.rst index dbf3561..c15cecd 100644 --- a/Doc/c-api/list.rst +++ b/Doc/c-api/list.rst @@ -86,6 +86,10 @@ List Objects Macro form of :c:func:`PyList_SetItem` without error checking. This is normally only used to fill in new lists where there is no previous content. + Bounds checking is performed as an assertion if Python is built in + :ref:`debug mode ` or :option:`with assertions + <--with-assertions>`. + .. note:: This macro "steals" a reference to *item*, and, unlike diff --git a/Doc/c-api/tuple.rst b/Doc/c-api/tuple.rst index ac62058..3fe1062 100644 --- a/Doc/c-api/tuple.rst +++ b/Doc/c-api/tuple.rst @@ -89,6 +89,9 @@ Tuple Objects Like :c:func:`PyTuple_SetItem`, but does no error checking, and should *only* be used to fill in brand new tuples. + Bounds checking is performed as an assertion if Python is built in + :ref:`debug mode ` or :option:`with assertions <--with-assertions>`. + .. note:: This function "steals" a reference to *o*, and, unlike @@ -194,12 +197,17 @@ type. .. c:function:: PyObject* PyStructSequence_GetItem(PyObject *p, Py_ssize_t pos) Return the object at position *pos* in the struct sequence pointed to by *p*. - No bounds checking is performed. + + Bounds checking is performed as an assertion if Python is built in + :ref:`debug mode ` or :option:`with assertions <--with-assertions>`. .. c:function:: PyObject* PyStructSequence_GET_ITEM(PyObject *p, Py_ssize_t pos) - Macro equivalent of :c:func:`PyStructSequence_GetItem`. + Alias to :c:func:`PyStructSequence_GetItem`. + + .. versionchanged:: 3.13 + Now implemented as an alias to :c:func:`PyStructSequence_GetItem`. .. c:function:: void PyStructSequence_SetItem(PyObject *p, Py_ssize_t pos, PyObject *o) @@ -208,6 +216,9 @@ type. :c:func:`PyTuple_SET_ITEM`, this should only be used to fill in brand new instances. + Bounds checking is performed as an assertion if Python is built in + :ref:`debug mode ` or :option:`with assertions <--with-assertions>`. + .. note:: This function "steals" a reference to *o*. @@ -215,9 +226,7 @@ type. .. c:function:: void PyStructSequence_SET_ITEM(PyObject *p, Py_ssize_t *pos, PyObject *o) - Similar to :c:func:`PyStructSequence_SetItem`, but implemented as a static - inlined function. + Alias to :c:func:`PyStructSequence_SetItem`. - .. note:: - - This function "steals" a reference to *o*. + .. versionchanged:: 3.13 + Now implemented as an alias to :c:func:`PyStructSequence_SetItem`. diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index f3460be..c0e9e92 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -441,6 +441,12 @@ New Features ``NULL`` if the referent is no longer live. (Contributed by Victor Stinner in :gh:`105927`.) +* If Python is built in :ref:`debug mode ` or :option:`with + assertions <--with-assertions>`, :c:func:`PyTuple_SET_ITEM` and + :c:func:`PyList_SET_ITEM` now check the index argument with an assertion. + If the assertion fails, make sure that the size is set before. + (Contributed by Victor Stinner in :gh:`106168`.) + Porting to Python 3.13 ---------------------- diff --git a/Include/cpython/listobject.h b/Include/cpython/listobject.h index 8fa8212..b3b2398 100644 --- a/Include/cpython/listobject.h +++ b/Include/cpython/listobject.h @@ -41,6 +41,8 @@ static inline Py_ssize_t PyList_GET_SIZE(PyObject *op) { static inline void PyList_SET_ITEM(PyObject *op, Py_ssize_t index, PyObject *value) { PyListObject *list = _PyList_CAST(op); + assert(0 <= index); + assert(index < Py_SIZE(list)); list->ob_item[index] = value; } #define PyList_SET_ITEM(op, index, value) \ diff --git a/Include/cpython/tupleobject.h b/Include/cpython/tupleobject.h index f6a1f07..370da16 100644 --- a/Include/cpython/tupleobject.h +++ b/Include/cpython/tupleobject.h @@ -31,6 +31,8 @@ static inline Py_ssize_t PyTuple_GET_SIZE(PyObject *op) { static inline void PyTuple_SET_ITEM(PyObject *op, Py_ssize_t index, PyObject *value) { PyTupleObject *tuple = _PyTuple_CAST(op); + assert(0 <= index); + assert(index < Py_SIZE(tuple)); tuple->ob_item[index] = value; } #define PyTuple_SET_ITEM(op, index, value) \ diff --git a/Include/internal/pycore_list.h b/Include/internal/pycore_list.h index 2fcbe12..b2e503c 100644 --- a/Include/internal/pycore_list.h +++ b/Include/internal/pycore_list.h @@ -49,8 +49,8 @@ _PyList_AppendTakeRef(PyListObject *self, PyObject *newitem) Py_ssize_t allocated = self->allocated; assert((size_t)len + 1 < PY_SSIZE_T_MAX); if (allocated > len) { - PyList_SET_ITEM(self, len, newitem); Py_SET_SIZE(self, len + 1); + PyList_SET_ITEM(self, len, newitem); return 0; } return _PyList_AppendTakeRefListResize(self, newitem); diff --git a/Include/structseq.h b/Include/structseq.h index 9687115..29e24fe 100644 --- a/Include/structseq.h +++ b/Include/structseq.h @@ -31,18 +31,15 @@ PyAPI_FUNC(PyTypeObject*) PyStructSequence_NewType(PyStructSequence_Desc *desc); PyAPI_FUNC(PyObject *) PyStructSequence_New(PyTypeObject* type); +PyAPI_FUNC(void) PyStructSequence_SetItem(PyObject*, Py_ssize_t, PyObject*); +PyAPI_FUNC(PyObject*) PyStructSequence_GetItem(PyObject*, Py_ssize_t); + #ifndef Py_LIMITED_API typedef PyTupleObject PyStructSequence; - -/* Macro, *only* to be used to fill in brand new objects */ -#define PyStructSequence_SET_ITEM(op, i, v) PyTuple_SET_ITEM((op), (i), (v)) - -#define PyStructSequence_GET_ITEM(op, i) PyTuple_GET_ITEM((op), (i)) +#define PyStructSequence_SET_ITEM PyStructSequence_SetItem +#define PyStructSequence_GET_ITEM PyStructSequence_GetItem #endif -PyAPI_FUNC(void) PyStructSequence_SetItem(PyObject*, Py_ssize_t, PyObject*); -PyAPI_FUNC(PyObject*) PyStructSequence_GetItem(PyObject*, Py_ssize_t); - #ifdef __cplusplus } #endif diff --git a/Misc/NEWS.d/next/C API/2023-06-28-02-30-50.gh-issue-106168.NFOZPv.rst b/Misc/NEWS.d/next/C API/2023-06-28-02-30-50.gh-issue-106168.NFOZPv.rst new file mode 100644 index 0000000..741d709 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-06-28-02-30-50.gh-issue-106168.NFOZPv.rst @@ -0,0 +1,5 @@ +If Python is built in :ref:`debug mode ` or :option:`with +assertions <--with-assertions>`, :c:func:`PyTuple_SET_ITEM` and +:c:func:`PyList_SET_ITEM` now check the index argument with an assertion. If +the assertion fails, make sure that the size is set before. Patch by Victor +Stinner. diff --git a/Objects/listobject.c b/Objects/listobject.c index f1edfb3..f1f324f 100644 --- a/Objects/listobject.c +++ b/Objects/listobject.c @@ -953,8 +953,9 @@ list_extend(PyListObject *self, PyObject *iterable) } if (Py_SIZE(self) < self->allocated) { /* steals ref */ - PyList_SET_ITEM(self, Py_SIZE(self), item); - Py_SET_SIZE(self, Py_SIZE(self) + 1); + Py_ssize_t len = Py_SIZE(self); + Py_SET_SIZE(self, len + 1); + PyList_SET_ITEM(self, len, item); } else { if (_PyList_AppendTakeRef(self, item) < 0) diff --git a/Objects/structseq.c b/Objects/structseq.c index 8b18959..4901113 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -74,15 +74,28 @@ PyStructSequence_New(PyTypeObject *type) } void -PyStructSequence_SetItem(PyObject* op, Py_ssize_t i, PyObject* v) +PyStructSequence_SetItem(PyObject *op, Py_ssize_t index, PyObject *value) { - PyStructSequence_SET_ITEM(op, i, v); + PyTupleObject *tuple = _PyTuple_CAST(op); + assert(0 <= index); +#ifndef NDEBUG + Py_ssize_t n_fields = REAL_SIZE(op); + assert(n_fields >= 0); + assert(index < n_fields); +#endif + tuple->ob_item[index] = value; } PyObject* -PyStructSequence_GetItem(PyObject* op, Py_ssize_t i) +PyStructSequence_GetItem(PyObject *op, Py_ssize_t index) { - return PyStructSequence_GET_ITEM(op, i); + assert(0 <= index); +#ifndef NDEBUG + Py_ssize_t n_fields = REAL_SIZE(op); + assert(n_fields >= 0); + assert(index < n_fields); +#endif + return PyTuple_GET_ITEM(op, index); } @@ -287,7 +300,7 @@ structseq_repr(PyStructSequence *obj) goto error; } - PyObject *value = PyStructSequence_GET_ITEM(obj, i); + PyObject *value = PyStructSequence_GetItem((PyObject*)obj, i); assert(value != NULL); PyObject *repr = PyObject_Repr(value); if (repr == NULL) { -- cgit v0.12