bpo-42006: Stop using PyDict_GetItem, PyDict_GetItemString and _PyDict_GetItemId. (GH-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.

6 files changed, 74 insertions, 39 deletions

diff --git a/Python/ceval.c b/Python/ceval.c
index fafbf75..7338be5 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -1446,11 +1446,18 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
     }
 
 #ifdef LLTRACE
-    lltrace = _PyDict_GetItemId(f->f_globals, &PyId___ltrace__) != NULL;
+    {
+        int r = _PyDict_ContainsId(f->f_globals, &PyId___ltrace__);
+        if (r < 0) {
+            goto exit_eval_frame;
+        }
+        lltrace = r;
+    }
 #endif
 
-    if (throwflag) /* support for generator.throw() */
+    if (throwflag) { /* support for generator.throw() */
         goto error;
+    }
 
 #ifdef Py_DEBUG
     /* _PyEval_EvalFrameDefault() must not be called with an exception set,
diff --git a/Python/compile.c b/Python/compile.c
index ddd2a04..a0089b2 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -489,8 +489,8 @@ dictbytype(PyObject *src, int scope_type, int flag, Py_ssize_t offset)
         /* XXX this should probably be a macro in symtable.h */
         long vi;
         k = PyList_GET_ITEM(sorted_keys, key_i);
-        v = PyDict_GetItem(src, k);
-        assert(PyLong_Check(v));
+        v = PyDict_GetItemWithError(src, k);
+        assert(v && PyLong_Check(v));
         vi = PyLong_AS_LONG(v);
         scope = (vi >> SCOPE_OFFSET) & SCOPE_MASK;
 
@@ -1889,7 +1889,7 @@ static int
 compiler_lookup_arg(PyObject *dict, PyObject *name)
 {
     PyObject *v;
-    v = PyDict_GetItem(dict, name);
+    v = PyDict_GetItemWithError(dict, name);
     if (v == NULL)
         return -1;
     return PyLong_AS_LONG(v);
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 75d5780..774a4f9 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -160,7 +160,7 @@ init_importlib(PyThreadState *tstate, PyObject *sysmod)
     interp->importlib = importlib;
     Py_INCREF(interp->importlib);
 
-    interp->import_func = PyDict_GetItemString(interp->builtins, "__import__");
+    interp->import_func = _PyDict_GetItemStringWithError(interp->builtins, "__import__");
     if (interp->import_func == NULL)
         return _PyStatus_ERR("__import__ not found");
     Py_INCREF(interp->import_func);
@@ -1683,7 +1683,10 @@ add_main_module(PyInterpreterState *interp)
     }
     Py_DECREF(ann_dict);
 
-    if (PyDict_GetItemString(d, "__builtins__") == NULL) {
+    if (_PyDict_GetItemStringWithError(d, "__builtins__") == NULL) {
+        if (PyErr_Occurred()) {
+            return _PyStatus_ERR("Failed to test __main__.__builtins__");
+        }
         PyObject *bimod = PyImport_ImportModule("builtins");
         if (bimod == NULL) {
             return _PyStatus_ERR("Failed to retrieve builtins module");
@@ -1700,8 +1703,11 @@ add_main_module(PyInterpreterState *interp)
      * be set if __main__ gets further initialized later in the startup
      * process.
      */
-    loader = PyDict_GetItemString(d, "__loader__");
+    loader = _PyDict_GetItemStringWithError(d, "__loader__");
     if (loader == NULL || loader == Py_None) {
+        if (PyErr_Occurred()) {
+            return _PyStatus_ERR("Failed to test __main__.__loader__");
+        }
         PyObject *loader = PyObject_GetAttrString(interp->importlib,
                                                   "BuiltinImporter");
         if (loader == NULL) {
diff --git a/Python/pythonrun.c b/Python/pythonrun.c
index a45ca3b..bd49c40 100644
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -351,7 +351,10 @@ PyRun_SimpleFileExFlags(FILE *fp, const char *filename, int closeit,
         return -1;
     Py_INCREF(m);
     d = PyModule_GetDict(m);
-    if (PyDict_GetItemString(d, "__file__") == NULL) {
+    if (_PyDict_GetItemStringWithError(d, "__file__") == NULL) {
+        if (PyErr_Occurred()) {
+            goto done;
+        }
         PyObject *f;
         f = PyUnicode_DecodeFSDefault(filename);
         if (f == NULL)
@@ -1116,9 +1119,11 @@ run_eval_code_obj(PyThreadState *tstate, PyCodeObject *co, PyObject *globals, Py
     _Py_UnhandledKeyboardInterrupt = 0;
 
     /* Set globals['__builtins__'] if it doesn't exist */
-    if (globals != NULL && PyDict_GetItemString(globals, "__builtins__") == NULL) {
-        if (PyDict_SetItemString(globals, "__builtins__",
-                                 tstate->interp->builtins) < 0) {
+    if (globals != NULL && _PyDict_GetItemStringWithError(globals, "__builtins__") == NULL) {
+        if (PyErr_Occurred() ||
+            PyDict_SetItemString(globals, "__builtins__",
+                                 tstate->interp->builtins) < 0)
+        {
             return NULL;
         }
     }
diff --git a/Python/symtable.c b/Python/symtable.c
index 4a98e79..0464cd8 100644
--- a/Python/symtable.c
+++ b/Python/symtable.c
@@ -392,7 +392,7 @@ PySymtable_Lookup(struct symtable *st, void *key)
 static long
 _PyST_GetSymbol(PySTEntryObject *ste, PyObject *name)
 {
-    PyObject *v = PyDict_GetItem(ste->ste_symbols, name);
+    PyObject *v = PyDict_GetItemWithError(ste->ste_symbols, name);
     if (!v)
         return 0;
     assert(PyLong_Check(v));
@@ -634,7 +634,7 @@ update_symbols(PyObject *symbols, PyObject *scopes,
         long scope, flags;
         assert(PyLong_Check(v));
         flags = PyLong_AS_LONG(v);
-        v_scope = PyDict_GetItem(scopes, name);
+        v_scope = PyDict_GetItemWithError(scopes, name);
         assert(v_scope && PyLong_Check(v_scope));
         scope = PyLong_AS_LONG(v_scope);
         flags |= (scope << SCOPE_OFFSET);
@@ -1071,9 +1071,12 @@ symtable_add_def_helper(struct symtable *st, PyObject *name, int flag, struct _s
         /* XXX need to update DEF_GLOBAL for other flags too;
            perhaps only DEF_FREE_GLOBAL */
         val = flag;
-        if ((o = PyDict_GetItem(st->st_global, mangled))) {
+        if ((o = PyDict_GetItemWithError(st->st_global, mangled))) {
             val |= PyLong_AS_LONG(o);
         }
+        else if (PyErr_Occurred()) {
+            goto error;
+        }
         o = PyLong_FromLong(val);
         if (o == NULL)
             goto error;
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
index bfcf4e8..749b964 100644
--- a/Python/sysmodule.c
+++ b/Python/sysmodule.c
@@ -68,7 +68,13 @@ sys_get_object_id(PyThreadState *tstate, _Py_Identifier *key)
     if (sd == NULL) {
         return NULL;
     }
-    return _PyDict_GetItemId(sd, key);
+    PyObject *exc_type, *exc_value, *exc_tb;
+    _PyErr_Fetch(tstate, &exc_type, &exc_value, &exc_tb);
+    PyObject *value = _PyDict_GetItemIdWithError(sd, key);
+    /* XXX Suppress a new exception if it was raised and restore
+     * the old one. */
+    _PyErr_Restore(tstate, exc_type, exc_value, exc_tb);
+    return value;
 }
 
 PyObject *
@@ -86,26 +92,41 @@ PySys_GetObject(const char *name)
     if (sd == NULL) {
         return NULL;
     }
-    return PyDict_GetItemString(sd, name);
+    PyObject *exc_type, *exc_value, *exc_tb;
+    _PyErr_Fetch(tstate, &exc_type, &exc_value, &exc_tb);
+    PyObject *value = _PyDict_GetItemStringWithError(sd, name);
+    /* XXX Suppress a new exception if it was raised and restore
+     * the old one. */
+    _PyErr_Restore(tstate, exc_type, exc_value, exc_tb);
+    return value;
 }
 
 static int
-sys_set_object_id(PyThreadState *tstate, _Py_Identifier *key, PyObject *v)
+sys_set_object(PyThreadState *tstate, PyObject *key, PyObject *v)
 {
+    if (key == NULL) {
+        return -1;
+    }
     PyObject *sd = tstate->interp->sysdict;
     if (v == NULL) {
-        if (_PyDict_GetItemId(sd, key) == NULL) {
-            return 0;
-        }
-        else {
-            return _PyDict_DelItemId(sd, key);
+        v = _PyDict_Pop(sd, key, Py_None);
+        if (v == NULL) {
+            return -1;
         }
+        Py_DECREF(v);
+        return 0;
     }
     else {
-        return _PyDict_SetItemId(sd, key, v);
+        return PyDict_SetItem(sd, key, v);
     }
 }
 
+static int
+sys_set_object_id(PyThreadState *tstate, _Py_Identifier *key, PyObject *v)
+{
+    return sys_set_object(tstate, _PyUnicode_FromId(key), v);
+}
+
 int
 _PySys_SetObjectId(_Py_Identifier *key, PyObject *v)
 {
@@ -114,27 +135,20 @@ _PySys_SetObjectId(_Py_Identifier *key, PyObject *v)
 }
 
 static int
-sys_set_object(PyThreadState *tstate, const char *name, PyObject *v)
+sys_set_object_str(PyThreadState *tstate, const char *name, PyObject *v)
 {
-    PyObject *sd = tstate->interp->sysdict;
-    if (v == NULL) {
-        if (PyDict_GetItemString(sd, name) == NULL) {
-            return 0;
-        }
-        else {
-            return PyDict_DelItemString(sd, name);
-        }
-    }
-    else {
-        return PyDict_SetItemString(sd, name, v);
-    }
+    PyObject *key = v ? PyUnicode_InternFromString(name)
+                      : PyUnicode_FromString(name);
+    int r = sys_set_object(tstate, key, v);
+    Py_XDECREF(key);
+    return r;
 }
 
 int
 PySys_SetObject(const char *name, PyObject *v)
 {
     PyThreadState *tstate = _PyThreadState_GET();
-    return sys_set_object(tstate, name, v);
+    return sys_set_object_str(tstate, name, v);
 }
 
 
@@ -3083,7 +3097,7 @@ PySys_SetArgvEx(int argc, wchar_t **argv, int updatepath)
     if (av == NULL) {
         Py_FatalError("no mem for sys.argv");
     }
-    if (sys_set_object(tstate, "argv", av) != 0) {
+    if (sys_set_object_str(tstate, "argv", av) != 0) {
         Py_DECREF(av);
         Py_FatalError("can't assign sys.argv");
     }