bpo-42006: Stop using PyDict_GetItem, PyDict_GetItemString and _PyDic…

…t_GetItemId. (pythonGH-22648)

These functions are considered not safe because they suppress all internal errors
and can return wrong result.  PyDict_GetItemString and _PyDict_GetItemId can
also silence current exception in rare cases.

Remove no longer used _PyDict_GetItemId.
Add _PyDict_ContainsId and rename _PyDict_Contains into
_PyDict_Contains_KnownHash.

Include/cpython/dictobject.h

@@ -46,7 +46,8 @@ PyAPI_FUNC(int) _PyDict_Next(
 
 /* Get the number of items of a dictionary. */
 #define PyDict_GET_SIZE(mp)  (assert(PyDict_Check(mp)),((PyDictObject *)mp)->ma_used)
-PyAPI_FUNC(int) _PyDict_Contains(PyObject *mp, PyObject *key, Py_hash_t hash);
+PyAPI_FUNC(int) _PyDict_Contains_KnownHash(PyObject *, PyObject *, Py_hash_t);
+PyAPI_FUNC(int) _PyDict_ContainsId(PyObject *, struct _Py_Identifier *);
 PyAPI_FUNC(PyObject *) _PyDict_NewPresized(Py_ssize_t minused);
 PyAPI_FUNC(void) _PyDict_MaybeUntrack(PyObject *mp);
 PyAPI_FUNC(int) _PyDict_HasOnlyStringKeys(PyObject *mp);
@@ -63,7 +64,6 @@ PyObject *_PyDict_FromKeys(PyObject *, PyObject *, PyObject *);
    argument is raised.
 */
 PyAPI_FUNC(int) _PyDict_MergeEx(PyObject *mp, PyObject *other, int override);
-PyAPI_FUNC(PyObject *) _PyDict_GetItemId(PyObject *dp, struct _Py_Identifier *key);
 PyAPI_FUNC(int) _PyDict_SetItemId(PyObject *dp, struct _Py_Identifier *key, PyObject *item);
 
 PyAPI_FUNC(int) _PyDict_DelItemId(PyObject *mp, struct _Py_Identifier *key);

Modules/_decimal/_decimal.c

@@ -3186,6 +3186,31 @@ dotsep_as_utf8(const char *s)
     return utf8;
 }
 
+static int
+dict_get_item_string(PyObject *dict, const char *key, PyObject **valueobj, const char **valuestr)
+{
+    *valueobj = NULL;
+    PyObject *keyobj = PyUnicode_FromString(key);
+    if (keyobj == NULL) {
+        return -1;
+    }
+    PyObject *value = PyDict_GetItemWithError(dict, keyobj);
+    Py_DECREF(keyobj);
+    if (value == NULL) {
+        if (PyErr_Occurred()) {
+            return -1;
+        }
+        return 0;
+    }
+    value = PyUnicode_AsUTF8String(value);
+    if (value == NULL) {
+        return -1;
+    }
+    *valueobj = value;
+    *valuestr = PyBytes_AS_STRING(value);
+    return 0;
+}
+
 /* Formatted representation of a PyDecObject. */
 static PyObject *
 dec_format(PyObject *dec, PyObject *args)
@@ -3256,23 +3281,11 @@ dec_format(PyObject *dec, PyObject *args)
                 "optional argument must be a dict");
             goto finish;
         }
-        if ((dot = PyDict_GetItemString(override, "decimal_point"))) {
-            if ((dot = PyUnicode_AsUTF8String(dot)) == NULL) {
-                goto finish;
-            }
-            spec.dot = PyBytes_AS_STRING(dot);
-        }
-        if ((sep = PyDict_GetItemString(override, "thousands_sep"))) {
-            if ((sep = PyUnicode_AsUTF8String(sep)) == NULL) {
-                goto finish;
-            }
-            spec.sep = PyBytes_AS_STRING(sep);
-        }
-        if ((grouping = PyDict_GetItemString(override, "grouping"))) {
-            if ((grouping = PyUnicode_AsUTF8String(grouping)) == NULL) {
-                goto finish;
-            }
-            spec.grouping = PyBytes_AS_STRING(grouping);
+        if (dict_get_item_string(override, "decimal_point", &dot, &spec.dot) ||
+            dict_get_item_string(override, "thousands_sep", &sep, &spec.sep) ||
+            dict_get_item_string(override, "grouping", &grouping, &spec.grouping))
+        {
+            goto finish;
         }
         if (mpd_validate_lconv(&spec) < 0) {
             PyErr_SetString(PyExc_ValueError,

Modules/_threadmodule.c

@@ -816,10 +816,14 @@ local_clear(localobject *self)
         for(tstate = PyInterpreterState_ThreadHead(tstate->interp);
             tstate;
             tstate = PyThreadState_Next(tstate))
-            if (tstate->dict && PyDict_GetItem(tstate->dict, self->key)) {
-                if (PyDict_DelItem(tstate->dict, self->key)) {
+            if (tstate->dict) {
+                PyObject *v = _PyDict_Pop(tstate->dict, self->key, Py_None);
+                if (v == NULL) {
                     PyErr_Clear();
                 }
+                else {
+                    Py_DECREF(v);
+                }
             }
     }
     return 0;

Modules/_zoneinfo.c

@@ -722,17 +722,16 @@ zoneinfo__unpickle(PyTypeObject *cls, PyObject *args)
 static PyObject *
 load_timedelta(long seconds)
 {
-    PyObject *rv = NULL;
+    PyObject *rv;
     PyObject *pyoffset = PyLong_FromLong(seconds);
     if (pyoffset == NULL) {
         return NULL;
     }
-    int contains = PyDict_Contains(TIMEDELTA_CACHE, pyoffset);
-    if (contains == -1) {
-        goto error;
-    }
-
-    if (!contains) {
+    rv = PyDict_GetItemWithError(TIMEDELTA_CACHE, pyoffset);
+    if (rv == NULL) {
+        if (PyErr_Occurred()) {
+            goto error;
+        }
         PyObject *tmp = PyDateTimeAPI->Delta_FromDelta(
             0, seconds, 0, 1, PyDateTimeAPI->DeltaType);
 
@@ -743,12 +742,9 @@ load_timedelta(long seconds)
         rv = PyDict_SetDefault(TIMEDELTA_CACHE, pyoffset, tmp);
         Py_DECREF(tmp);
     }
-    else {
-        rv = PyDict_GetItem(TIMEDELTA_CACHE, pyoffset);
-    }
 
+    Py_XINCREF(rv);
     Py_DECREF(pyoffset);
-    Py_INCREF(rv);
     return rv;
 error:
     Py_DECREF(pyoffset);

Modules/signalmodule.c

@@ -1427,10 +1427,9 @@ signal_exec(PyObject *m)
          return -1;
 #endif
 
-    IntHandler = PyDict_GetItemString(d, "default_int_handler");
+    IntHandler = PyMapping_GetItemString(d, "default_int_handler");
     if (!IntHandler)
         return -1;
-    Py_INCREF(IntHandler);
 
     _Py_atomic_store_relaxed(&Handlers[0].tripped, 0);
     for (int i = 1; i < NSIG; i++) {

Modules/socketmodule.c

@@ -324,7 +324,7 @@ static FlagRuntimeInfo win_runtime_flags[] = {
     {14393, "TCP_FASTOPEN"}
 };
 
-static void
+static int
 remove_unusable_flags(PyObject *m)
 {
     PyObject *dict;
@@ -333,7 +333,7 @@ remove_unusable_flags(PyObject *m)
 
     dict = PyModule_GetDict(m);
     if (dict == NULL) {
-        return;
+        return -1;
     }
 
     /* set to Windows 10, except BuildNumber. */
@@ -359,19 +359,19 @@ remove_unusable_flags(PyObject *m)
             break;
         }
         else {
-            if (PyDict_GetItemString(
-                    dict,
-                    win_runtime_flags[i].flag_name) != NULL)
-            {
-                if (PyDict_DelItemString(
-                        dict,
-                        win_runtime_flags[i].flag_name))
-                {
-                    PyErr_Clear();
-                }
+            PyObject *flag_name = PyUnicode_FromString(win_runtime_flags[i].flag_name);
+            if (flag_name == NULL) {
+                return -1;
+            }
+            PyObject *v = _PyDict_Pop(dict, flag_name, Py_None);
+            Py_DECREF(flag_name);
+            if (v == NULL) {
+                return -1;
             }
+            Py_DECREF(v);
         }
     }
+    return 0;
 }
 
 #endif
@@ -8382,7 +8382,10 @@ PyInit__socket(void)
 
 #ifdef MS_WINDOWS
     /* remove some flags on older version Windows during run-time */
-    remove_unusable_flags(m);
+    if (remove_unusable_flags(m) < 0) {
+        Py_DECREF(m);
+        return NULL;
+    }
 #endif
 
     return m;

Objects/dictobject.c

@@ -3434,7 +3434,7 @@ PyDict_Contains(PyObject *op, PyObject *key)
 
 /* Internal version of PyDict_Contains used when the hash value is already known */
 int
-_PyDict_Contains(PyObject *op, PyObject *key, Py_hash_t hash)
+_PyDict_Contains_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash)
 {
     PyDictObject *mp = (PyDictObject *)op;
     PyObject *value;
@@ -3446,6 +3446,16 @@ _PyDict_Contains(PyObject *op, PyObject *key, Py_hash_t hash)
     return (ix != DKIX_EMPTY && value != NULL);
 }
 
+int
+_PyDict_ContainsId(PyObject *op, struct _Py_Identifier *key)
+{
+    PyObject *kv = _PyUnicode_FromId(key); /* borrowed */
+    if (kv == NULL) {
+        return -1;
+    }
+    return PyDict_Contains(op, kv);
+}
+
 /* Hack to implement "key in dict" */
 static PySequenceMethods dict_as_sequence = {
     0,                          /* sq_length */
@@ -3590,18 +3600,6 @@ PyTypeObject PyDict_Type = {
     .tp_vectorcall = dict_vectorcall,
 };
 
-PyObject *
-_PyDict_GetItemId(PyObject *dp, struct _Py_Identifier *key)
-{
-    PyObject *kv;
-    kv = _PyUnicode_FromId(key); /* borrowed */
-    if (kv == NULL) {
-        PyErr_Clear();
-        return NULL;
-    }
-    return PyDict_GetItem(dp, kv);
-}
-
 /* For backward compatibility with old dictionary interface */
 
 PyObject *

Objects/moduleobject.c

@@ -477,10 +477,12 @@ PyModule_GetNameObject(PyObject *m)
     }
     d = ((PyModuleObject *)m)->md_dict;
     if (d == NULL ||
-        (name = _PyDict_GetItemId(d, &PyId___name__)) == NULL ||
+        (name = _PyDict_GetItemIdWithError(d, &PyId___name__)) == NULL ||
         !PyUnicode_Check(name))
     {
-        PyErr_SetString(PyExc_SystemError, "nameless module");
+        if (!PyErr_Occurred()) {
+            PyErr_SetString(PyExc_SystemError, "nameless module");
+        }
         return NULL;
     }
     Py_INCREF(name);
@@ -509,10 +511,12 @@ PyModule_GetFilenameObject(PyObject *m)
     }
     d = ((PyModuleObject *)m)->md_dict;
     if (d == NULL ||
-        (fileobj = _PyDict_GetItemId(d, &PyId___file__)) == NULL ||
+        (fileobj = _PyDict_GetItemIdWithError(d, &PyId___file__)) == NULL ||
         !PyUnicode_Check(fileobj))
     {
-        PyErr_SetString(PyExc_SystemError, "module filename missing");
+        if (!PyErr_Occurred()) {
+            PyErr_SetString(PyExc_SystemError, "module filename missing");
+        }
         return NULL;
     }
     Py_INCREF(fileobj);
@@ -721,14 +725,21 @@ module_getattro(PyModuleObject *m, PyObject *name)
     PyErr_Clear();
     if (m->md_dict) {
         _Py_IDENTIFIER(__getattr__);
-        getattr = _PyDict_GetItemId(m->md_dict, &PyId___getattr__);
+        getattr = _PyDict_GetItemIdWithError(m->md_dict, &PyId___getattr__);
         if (getattr) {
             return PyObject_CallOneArg(getattr, name);
         }
-        mod_name = _PyDict_GetItemId(m->md_dict, &PyId___name__);
+        if (PyErr_Occurred()) {
+            return NULL;
+        }
+        mod_name = _PyDict_GetItemIdWithError(m->md_dict, &PyId___name__);
         if (mod_name && PyUnicode_Check(mod_name)) {
             Py_INCREF(mod_name);
-            PyObject *spec = _PyDict_GetItemId(m->md_dict, &PyId___spec__);
+            PyObject *spec = _PyDict_GetItemIdWithError(m->md_dict, &PyId___spec__);
+            if (spec == NULL && PyErr_Occurred()) {
+                Py_DECREF(mod_name);
+                return NULL;
+            }
             Py_XINCREF(spec);
             if (_PyModuleSpec_IsInitializing(spec)) {
                 PyErr_Format(PyExc_AttributeError,
@@ -746,6 +757,9 @@ module_getattro(PyModuleObject *m, PyObject *name)
             Py_DECREF(mod_name);
             return NULL;
         }
+        else if (PyErr_Occurred()) {
+            return NULL;
+        }
     }
     PyErr_Format(PyExc_AttributeError,
                 "module has no attribute '%U'", name);

Objects/setobject.c

@@ -1498,7 +1498,7 @@ set_difference(PySetObject *so, PyObject *other)
         while (set_next(so, &pos, &entry)) {
             key = entry->key;
             hash = entry->hash;
-            rv = _PyDict_Contains(other, key, hash);
+            rv = _PyDict_Contains_KnownHash(other, key, hash);
             if (rv < 0) {
                 Py_DECREF(result);
                 return NULL;