gh-127271: Remove the PyCell_Get usage for framelocalsproxy (#130383)

Author: gaogaotiantian

Objects/frameobject.c

@@ -28,7 +28,7 @@
 #define OFF(x) offsetof(PyFrameObject, x)
 
 
-// Returns borrowed reference or NULL
+// Returns new reference or NULL
 static PyObject *
 framelocalsproxy_getval(_PyInterpreterFrame *frame, PyCodeObject *co, int i)
 {
@@ -57,7 +57,10 @@ framelocalsproxy_getval(_PyInterpreterFrame *frame, PyCodeObject *co, int i)
     }
 
     if (cell != NULL) {
-        value = PyCell_GET(cell);
+        value = PyCell_GetRef((PyCellObject *)cell);
+    }
+    else {
+        Py_XINCREF(value);
     }
 
     if (value == NULL) {
@@ -67,24 +70,42 @@ framelocalsproxy_getval(_PyInterpreterFrame *frame, PyCodeObject *co, int i)
     return value;
 }
 
+static bool
+framelocalsproxy_hasval(_PyInterpreterFrame *frame, PyCodeObject *co, int i)
+{
+    PyObject *value = framelocalsproxy_getval(frame, co, i);
+    if (value == NULL) {
+        return false;
+    }
+    Py_DECREF(value);
+    return true;
+}
+
 static int
-framelocalsproxy_getkeyindex(PyFrameObject *frame, PyObject* key, bool read)
+framelocalsproxy_getkeyindex(PyFrameObject *frame, PyObject *key, bool read, PyObject **value_ptr)
 {
     /*
      * Returns -2 (!) if an error occurred; exception will be set.
      * Returns the fast locals index of the key on success:
      *   - if read == true, returns the index if the value is not NULL
      *   - if read == false, returns the index if the value is not hidden
      * Otherwise returns -1.
+     *
+     * If read == true and value_ptr is not NULL, *value_ptr is set to
+     * the value of the key if it is found (with a new reference).
      */
 
+    // value_ptr should only be given if we are reading the value
+    assert(read || value_ptr == NULL);
+
     PyCodeObject *co = _PyFrame_GetCode(frame->f_frame);
 
     // Ensure that the key is hashable.
     Py_hash_t key_hash = PyObject_Hash(key);
     if (key_hash == -1) {
         return -2;
     }
+
     bool found = false;
 
     // We do 2 loops here because it's highly possible the key is interned
@@ -93,7 +114,14 @@ framelocalsproxy_getkeyindex(PyFrameObject *frame, PyObject* key, bool read)
         PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
         if (name == key) {
             if (read) {
-                if (framelocalsproxy_getval(frame->f_frame, co, i) != NULL) {
+                PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i);
+                if (value != NULL) {
+                    if (value_ptr != NULL) {
+                        *value_ptr = value;
+                    }
+                    else {
+                        Py_DECREF(value);
+                    }
                     return i;
                 }
             } else {
@@ -124,7 +152,14 @@ framelocalsproxy_getkeyindex(PyFrameObject *frame, PyObject* key, bool read)
         }
         if (same) {
             if (read) {
-                if (framelocalsproxy_getval(frame->f_frame, co, i) != NULL) {
+                PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i);
+                if (value != NULL) {
+                    if (value_ptr != NULL) {
+                        *value_ptr = value;
+                    }
+                    else {
+                        Py_DECREF(value);
+                    }
                     return i;
                 }
             } else {
@@ -142,25 +177,27 @@ static PyObject *
 framelocalsproxy_getitem(PyObject *self, PyObject *key)
 {
     PyFrameObject *frame = PyFrameLocalsProxyObject_CAST(self)->frame;
-    PyCodeObject *co = _PyFrame_GetCode(frame->f_frame);
+    PyObject *value = NULL;
 
-    int i = framelocalsproxy_getkeyindex(frame, key, true);
+    int i = framelocalsproxy_getkeyindex(frame, key, true, &value);
     if (i == -2) {
         return NULL;
     }
     if (i >= 0) {
-        PyObject *value = framelocalsproxy_getval(frame->f_frame, co, i);
         assert(value != NULL);
-        return Py_NewRef(value);
+        return value;
     }
+    assert(value == NULL);
 
     // Okay not in the fast locals, try extra locals
 
     PyObject *extra = frame->f_extra_locals;
     if (extra != NULL) {
-        PyObject *value = PyDict_GetItem(extra, key);
+        if (PyDict_GetItemRef(extra, key, &value) < 0) {
+            return NULL;
+        }
         if (value != NULL) {
-            return Py_NewRef(value);
+            return value;
         }
     }
 
@@ -176,7 +213,7 @@ framelocalsproxy_setitem(PyObject *self, PyObject *key, PyObject *value)
     _PyStackRef *fast = _PyFrame_GetLocalsArray(frame->f_frame);
     PyCodeObject *co = _PyFrame_GetCode(frame->f_frame);
 
-    int i = framelocalsproxy_getkeyindex(frame, key, false);
+    int i = framelocalsproxy_getkeyindex(frame, key, false, NULL);
     if (i == -2) {
         return -1;
     }
@@ -297,8 +334,7 @@ framelocalsproxy_keys(PyObject *self, PyObject *Py_UNUSED(ignored))
     }
 
     for (int i = 0; i < co->co_nlocalsplus; i++) {
-        PyObject *val = framelocalsproxy_getval(frame->f_frame, co, i);
-        if (val) {
+        if (framelocalsproxy_hasval(frame->f_frame, co, i)) {
             PyObject *name = PyTuple_GET_ITEM(co->co_localsplusnames, i);
             if (PyList_Append(names, name) < 0) {
                 Py_DECREF(names);
@@ -511,8 +547,10 @@ framelocalsproxy_values(PyObject *self, PyObject *Py_UNUSED(ignored))
         if (value) {
             if (PyList_Append(values, value) < 0) {
                 Py_DECREF(values);
+                Py_DECREF(value);
                 return NULL;
             }
+            Py_DECREF(value);
         }
     }
 
@@ -550,16 +588,19 @@ framelocalsproxy_items(PyObject *self, PyObject *Py_UNUSED(ignored))
             PyObject *pair = PyTuple_Pack(2, name, value);
             if (pair == NULL) {
                 Py_DECREF(items);
+                Py_DECREF(value);
                 return NULL;
             }
 
             if (PyList_Append(items, pair) < 0) {
                 Py_DECREF(items);
                 Py_DECREF(pair);
+                Py_DECREF(value);
                 return NULL;
             }
 
             Py_DECREF(pair);
+            Py_DECREF(value);
         }
     }
 
@@ -601,7 +642,7 @@ framelocalsproxy_length(PyObject *self)
     }
 
     for (int i = 0; i < co->co_nlocalsplus; i++) {
-        if (framelocalsproxy_getval(frame->f_frame, co, i) != NULL) {
+        if (framelocalsproxy_hasval(frame->f_frame, co, i)) {
             size++;
         }
     }
@@ -613,7 +654,7 @@ framelocalsproxy_contains(PyObject *self, PyObject *key)
 {
     PyFrameObject *frame = PyFrameLocalsProxyObject_CAST(self)->frame;
 
-    int i = framelocalsproxy_getkeyindex(frame, key, true);
+    int i = framelocalsproxy_getkeyindex(frame, key, true, NULL);
     if (i == -2) {
         return -1;
     }
@@ -724,7 +765,7 @@ framelocalsproxy_pop(PyObject* self, PyObject *const *args, Py_ssize_t nargs)
 
     PyFrameObject *frame = PyFrameLocalsProxyObject_CAST(self)->frame;
 
-    int i = framelocalsproxy_getkeyindex(frame, key, false);
+    int i = framelocalsproxy_getkeyindex(frame, key, false, NULL);
     if (i == -2) {
         return NULL;
     }
@@ -2066,9 +2107,7 @@ _PyFrame_HasHiddenLocals(_PyInterpreterFrame *frame)
         _PyLocals_Kind kind = _PyLocals_GetKind(co->co_localspluskinds, i);
 
         if (kind & CO_FAST_HIDDEN) {
-            PyObject* value = framelocalsproxy_getval(frame, co, i);
-
-            if (value != NULL) {
+            if (framelocalsproxy_hasval(frame, co, i)) {
                 return true;
             }
         }