From c3ec341c59f48dd0950ca29e0f75d83f63ba81c9 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sun, 5 May 2024 14:52:53 -0700 Subject: [PATCH 1/2] Add proper error check for framelocalsproxy --- Objects/frameobject.c | 143 +++++++++++++++++++++++++++++------------- 1 file changed, 98 insertions(+), 45 deletions(-) diff --git a/Objects/frameobject.c b/Objects/frameobject.c index fdac86961f860c..8a8da6cd676bdd 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -189,7 +189,7 @@ framelocalsproxy_setitem(PyObject *self, PyObject *key, PyObject *value) assert(PyDict_Check(extra)); - return PyDict_SetItem(extra, key, value) < 0; + return PyDict_SetItem(extra, key, value); } static int @@ -200,19 +200,19 @@ framelocalsproxy_merge(PyObject* self, PyObject* other) } PyObject *keys = PyMapping_Keys(other); - PyObject *iter = NULL; - PyObject *key = NULL; - PyObject *value = NULL; - - assert(keys != NULL); + if (keys == NULL) { + return -1; + } - iter = PyObject_GetIter(keys); + PyObject *iter = PyObject_GetIter(keys); Py_DECREF(keys); - if (iter == NULL) { return -1; } + PyObject *key = NULL; + PyObject *value = NULL; + while ((key = PyIter_Next(iter)) != NULL) { value = PyObject_GetItem(other, key); if (value == NULL) { @@ -238,12 +238,11 @@ framelocalsproxy_merge(PyObject* self, PyObject* other) } static PyObject * -framelocalsproxy_keys(PyObject *self, PyObject *__unused) +framelocalsproxy_keys(PyObject *self, void *Py_UNUSED(ignored)) { - PyObject *names = PyList_New(0); PyFrameObject *frame = ((PyFrameLocalsProxyObject*)self)->frame; PyCodeObject *co = _PyFrame_GetCode(frame->f_frame); - + PyObject *names = PyList_New(0); if (names == NULL) { return NULL; } @@ -260,12 +259,13 @@ framelocalsproxy_keys(PyObject *self, PyObject *__unused) } // Iterate through the extra locals - Py_ssize_t i = 0; - PyObject *key = NULL; - PyObject *value = NULL; - if (frame->f_extra_locals) { assert(PyDict_Check(frame->f_extra_locals)); + + Py_ssize_t i = 0; + PyObject *key = NULL; + PyObject *value = NULL; + while (PyDict_Next(frame->f_extra_locals, &i, &key, &value)) { if (PyList_Append(names, key) < 0) { Py_DECREF(names); @@ -341,9 +341,16 @@ framelocalsproxy_richcompare(PyObject *self, PyObject *other, int op) } } else if (PyDict_Check(other)) { PyObject *dct = PyDict_New(); - PyObject *result = NULL; - PyDict_Update(dct, self); - result = PyObject_RichCompare(dct, other, op); + if (dct == NULL) { + return NULL; + } + + if (PyDict_Update(dct, self) < 0) { + Py_DECREF(dct); + return NULL; + } + + PyObject *result = PyObject_RichCompare(dct, other, op); Py_DECREF(dct); return result; } @@ -360,11 +367,19 @@ framelocalsproxy_repr(PyObject *self) } PyObject *dct = PyDict_New(); - PyObject *repr = NULL; + if (dct == NULL) { + Py_ReprLeave(self); + return NULL; + } - if (PyDict_Update(dct, self) == 0) { - repr = PyObject_Repr(dct); + if (PyDict_Update(dct, self) < 0) { + Py_DECREF(dct); + Py_ReprLeave(self); + return NULL; } + + PyObject *repr = PyObject_Repr(dct); + Py_ReprLeave(self); Py_DECREF(dct); @@ -379,6 +394,10 @@ framelocalsproxy_or(PyObject *self, PyObject *other) } PyObject *result = PyDict_New(); + if (result == NULL) { + return NULL; + } + if (PyDict_Update(result, self) < 0) { Py_DECREF(result); return NULL; @@ -407,27 +426,35 @@ framelocalsproxy_inplace_or(PyObject *self, PyObject *other) } static PyObject* -framelocalsproxy_values(PyObject *self, PyObject *__unused) +framelocalsproxy_values(PyObject *self, void *Py_UNUSED(ignored)) { - PyObject *values = PyList_New(0); PyFrameObject *frame = ((PyFrameLocalsProxyObject*)self)->frame; PyCodeObject *co = _PyFrame_GetCode(frame->f_frame); + PyObject *values = PyList_New(0); + if (values == NULL) { + return NULL; + } for (int i = 0; i < co->co_nlocalsplus; i++) { PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i); if (value) { - PyList_Append(values, value); + if (PyList_Append(values, value) < 0) { + Py_DECREF(values); + return NULL; + } } } // Iterate through the extra locals - Py_ssize_t j = 0; - PyObject *key = NULL; - PyObject *value = NULL; - if (frame->f_extra_locals) { + Py_ssize_t j = 0; + PyObject *key = NULL; + PyObject *value = NULL; while (PyDict_Next(frame->f_extra_locals, &j, &key, &value)) { - PyList_Append(values, value); + if (PyList_Append(values, value) < 0) { + Py_DECREF(values); + return NULL; + } } } @@ -435,11 +462,14 @@ framelocalsproxy_values(PyObject *self, PyObject *__unused) } static PyObject * -framelocalsproxy_items(PyObject *self, PyObject *__unused) +framelocalsproxy_items(PyObject *self, void *Py_UNUSED(ignored)) { - PyObject *items = PyList_New(0); PyFrameObject *frame = ((PyFrameLocalsProxyObject*)self)->frame; PyCodeObject *co = _PyFrame_GetCode(frame->f_frame); + PyObject *items = PyList_New(0); + if (items == NULL) { + return NULL; + } for (int i = 0; i < co->co_nlocalsplus; i++) { PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i); @@ -447,20 +477,39 @@ framelocalsproxy_items(PyObject *self, PyObject *__unused) if (value) { PyObject *pair = PyTuple_Pack(2, name, value); - PyList_Append(items, pair); + if (pair == NULL) { + Py_DECREF(items); + return NULL; + } + + if (PyList_Append(items, pair) < 0) { + Py_DECREF(items); + Py_DECREF(pair); + return NULL; + } + Py_DECREF(pair); } } // Iterate through the extra locals - Py_ssize_t j = 0; - PyObject *key = NULL; - PyObject *value = NULL; - if (frame->f_extra_locals) { + Py_ssize_t j = 0; + PyObject *key = NULL; + PyObject *value = NULL; while (PyDict_Next(frame->f_extra_locals, &j, &key, &value)) { PyObject *pair = PyTuple_Pack(2, key, value); - PyList_Append(items, pair); + if (pair == NULL) { + Py_DECREF(items); + return NULL; + } + + if (PyList_Append(items, pair) < 0) { + Py_DECREF(items); + Py_DECREF(pair); + return NULL; + } + Py_DECREF(pair); } } @@ -584,7 +633,7 @@ framelocalsproxy_setdefault(PyObject* self, PyObject *const *args, Py_ssize_t na } static PyObject* -framelocalsproxy_reversed(PyObject *self, PyObject *__unused) +framelocalsproxy_reversed(PyObject *self, void *Py_UNUSED(ignored)) { PyObject *result = framelocalsproxy_keys(self, NULL); @@ -615,19 +664,19 @@ static PyMethodDef framelocalsproxy_methods[] = { NULL}, {"__getitem__", framelocalsproxy_getitem, METH_O | METH_COEXIST, NULL}, - {"__reversed__", framelocalsproxy_reversed, METH_NOARGS, + {"update", framelocalsproxy_update, METH_O, NULL}, - {"keys", framelocalsproxy_keys, METH_NOARGS, + {"__reversed__", _PyCFunction_CAST(framelocalsproxy_reversed), METH_NOARGS, NULL}, - {"values", framelocalsproxy_values, METH_NOARGS, + {"keys", _PyCFunction_CAST(framelocalsproxy_keys), METH_NOARGS, NULL}, - {"items", framelocalsproxy_items, METH_NOARGS, + {"values", _PyCFunction_CAST(framelocalsproxy_values), METH_NOARGS, NULL}, - {"update", framelocalsproxy_update, METH_O, + {"items", _PyCFunction_CAST(framelocalsproxy_items), METH_NOARGS, NULL}, - {"get", _PyCFunction_CAST(framelocalsproxy_get), METH_FASTCALL, + {"get", _PyCFunction_CAST(framelocalsproxy_get), METH_FASTCALL, NULL}, - {"setdefault", _PyCFunction_CAST(framelocalsproxy_setdefault), METH_FASTCALL, + {"setdefault", _PyCFunction_CAST(framelocalsproxy_setdefault), METH_FASTCALL, NULL}, {NULL, NULL} /* sentinel */ }; @@ -657,6 +706,10 @@ PyObject * _PyFrameLocalsProxy_New(PyFrameObject *frame) { PyObject* args = PyTuple_Pack(1, frame); + if (args == NULL) { + return NULL; + } + PyObject* proxy = (PyObject*)framelocalsproxy_new(&PyFrameLocalsProxy_Type, args, NULL); Py_DECREF(args); return proxy; From 2d2b9d0bee23983ada658e0c9a852f238b723a48 Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Sun, 5 May 2024 18:07:34 -0700 Subject: [PATCH 2/2] Reorder statements --- Objects/frameobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 8a8da6cd676bdd..9d6ee1467760ab 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -379,10 +379,10 @@ framelocalsproxy_repr(PyObject *self) } PyObject *repr = PyObject_Repr(dct); + Py_DECREF(dct); Py_ReprLeave(self); - Py_DECREF(dct); return repr; }