gh-118609: Add proper error check for framelocalsproxy (#118615)

1 files changed, 99 insertions, 46 deletions

diff --git a/Objects/frameobject.c b/Objects/frameobject.c
index aac809b..869cdec 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,14 +367,22 @@ 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;
     }
-    Py_ReprLeave(self);
 
+    PyObject *repr = PyObject_Repr(dct);
     Py_DECREF(dct);
+
+    Py_ReprLeave(self);
+
     return repr;
 }
 
@@ -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);
         }
     }
@@ -588,7 +637,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);
 
@@ -623,19 +672,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 */
 };
@@ -666,6 +715,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;