pythongh-106033: Get rid of new occurrences of PyDict_GetItem and PyO…

…bject_HasAttr These functions are broken by design because they discard any exceptions raised inside, including MemoryError and KeyboardInterrupt. They should not be used in new code.
serhiy-storchaka · Jun 23, 2023 · c593435 · c593435
1 parent 04492cb
commit c593435
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 31 deletions.
diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c
@@ -383,14 +383,15 @@ py_digest_by_digestmod(PyObject *module, PyObject *digestmod, enum Py_hash_type
     } else {
         _hashlibstate *state = get_hashlib_state(module);
         // borrowed ref
-        name_obj = PyDict_GetItem(state->constructs, digestmod);
+        name_obj = PyDict_GetItemWithError(state->constructs, digestmod);
     }
     if (name_obj == NULL) {
-        _hashlibstate *state = get_hashlib_state(module);
-        PyErr_Clear();
-        PyErr_Format(
-            state->unsupported_digestmod_error,
-            "Unsupported digestmod %R", digestmod);
+        if (!PyErr_Occurred()) {
+            _hashlibstate *state = get_hashlib_state(module);
+            PyErr_Format(
+                state->unsupported_digestmod_error,
+                "Unsupported digestmod %R", digestmod);
+        }
         return NULL;
     }
 

diff --git a/Modules/_testcapi/code.c b/Modules/_testcapi/code.c
@@ -9,7 +9,7 @@ get_code_extra_index(PyInterpreterState* interp) {
     PyObject *interp_dict = PyInterpreterState_GetDict(interp); // borrowed
     assert(interp_dict);  // real users would handle missing dict... somehow
 
-    PyObject *index_obj = PyDict_GetItemString(interp_dict, key); // borrowed
+    PyObject *index_obj = _PyDict_GetItemStringWithError(interp_dict, key); // borrowed
     Py_ssize_t index = 0;
     if (!index_obj) {
         if (PyErr_Occurred()) {

diff --git a/Objects/exceptions.c b/Objects/exceptions.c
@@ -207,20 +207,19 @@ BaseException_add_note(PyObject *self, PyObject *note)
         return NULL;
     }
 
-    if (!PyObject_HasAttr(self, &_Py_ID(__notes__))) {
-        PyObject *new_notes = PyList_New(0);
-        if (new_notes == NULL) {
+    PyObject *notes;
+    if (_PyObject_LookupAttr(self, &_Py_ID(__notes__), &notes) < 0) {
+        return NULL;
+    }
+    if (!notes) {
+        notes = PyList_New(0);
+        if (notes == NULL) {
             return NULL;
         }
-        if (PyObject_SetAttr(self, &_Py_ID(__notes__), new_notes) < 0) {
-            Py_DECREF(new_notes);
+        if (PyObject_SetAttr(self, &_Py_ID(__notes__), notes) < 0) {
+            Py_DECREF(notes);
             return NULL;
         }
-        Py_DECREF(new_notes);
-    }
-    PyObject *notes = PyObject_GetAttr(self, &_Py_ID(__notes__));
-    if (notes == NULL) {
-        return NULL;
     }
     if (!PyList_Check(notes)) {
         Py_DECREF(notes);
@@ -941,11 +940,11 @@ exceptiongroup_subset(
     PyException_SetContext(eg, PyException_GetContext(orig));
     PyException_SetCause(eg, PyException_GetCause(orig));
 
-    if (PyObject_HasAttr(orig, &_Py_ID(__notes__))) {
-        PyObject *notes = PyObject_GetAttr(orig, &_Py_ID(__notes__));
-        if (notes == NULL) {
-            goto error;
-        }
+    PyObject *notes;
+    if (_PyObject_LookupAttr(orig, &_Py_ID(__notes__), &notes) < 0) {
+        goto error;
+    }
+    if (notes) {
         if (PySequence_Check(notes)) {
             /* Make a copy so the parts have independent notes lists. */
             PyObject *notes_copy = PySequence_List(notes);

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -1508,11 +1508,14 @@ type_set_annotations(PyTypeObject *type, PyObject *value, void *context)
 static PyObject *
 type_get_type_params(PyTypeObject *type, void *context)
 {
-    PyObject *params = PyDict_GetItem(lookup_tp_dict(type), &_Py_ID(__type_params__));
+    PyObject *params = PyDict_GetItemWithError(lookup_tp_dict(type), &_Py_ID(__type_params__));
 
     if (params) {
         return Py_NewRef(params);
     }
+    if (PyErr_Occurred()) {
+        return NULL;
+    }
 
     return PyTuple_New(0);
 }

diff --git a/Python/pythonrun.c b/Python/pythonrun.c
@@ -1134,15 +1134,13 @@ print_exception_notes(struct exception_print_context *ctx, PyObject *value)
         return 0;
     }
 
-    if (!PyObject_HasAttr(value, &_Py_ID(__notes__))) {
-        return 0;
-    }
-    PyObject *notes = PyObject_GetAttr(value, &_Py_ID(__notes__));
-    if (notes == NULL) {
-        return -1;
+    PyObject *notes;
+    int res = _PyObject_LookupAttr(value, &_Py_ID(__notes__), &notes);
+    if (res <= 0) {
+        return res;
     }
     if (!PySequence_Check(notes) || PyUnicode_Check(notes) || PyBytes_Check(notes)) {
-        int res = 0;
+        res = 0;
         if (write_indented_margin(ctx, f) < 0) {
             res = -1;
         }
@@ -1155,10 +1153,10 @@ print_exception_notes(struct exception_print_context *ctx, PyObject *value)
             res = PyFile_WriteObject(s, f, Py_PRINT_RAW);
             Py_DECREF(s);
         }
-        Py_DECREF(notes);
         if (PyFile_WriteString("\n", f) < 0) {
             res = -1;
         }
+        Py_DECREF(notes);
         return res;
     }
     Py_ssize_t num_notes = PySequence_Length(notes);