pythongh-106672: C API: Report indiscriminately ignored errors (pytho…

…nGH-106674)

Functions which indiscriminately ignore all errors now report them as
unraisable errors.

Doc/whatsnew/3.13.rst

@@ -980,6 +980,15 @@ Changes in the Python API
   The result is now the same if ``wantobjects`` is set to ``0``.
   (Contributed by Serhiy Storchaka in :gh:`97928`.)
 
+* Functions :c:func:`PyDict_GetItem`, :c:func:`PyDict_GetItemString`,
+  :c:func:`PyMapping_HasKey`, :c:func:`PyMapping_HasKeyString`,
+  :c:func:`PyObject_HasAttr`, :c:func:`PyObject_HasAttrString`, and
+  :c:func:`PySys_GetObject`, which clear all errors occurred during calling
+  the function, report now them using :func:`sys.unraisablehook`.
+  You can consider to replace these functions with other functions as
+  recomended in the documentation.
+  (Contributed by Serhiy Storchaka in :gh:`106672`.)
+
 
 Build Changes
 =============

Lib/test/test_capi/test_abstract.py

@@ -1,5 +1,6 @@
 import unittest
 from collections import OrderedDict
+from test import support
 from test.support import import_helper
 
 _testcapi = import_helper.import_module('_testcapi')
@@ -109,8 +110,18 @@ def test_object_hasattr(self):
         self.assertFalse(xhasattr(obj, 'b'))
         self.assertTrue(xhasattr(obj, '\U0001f40d'))
 
-        self.assertFalse(xhasattr(obj, 'evil'))
-        self.assertFalse(xhasattr(obj, 1))
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(xhasattr(obj, 'evil'))
+            self.assertEqual(cm.unraisable.exc_type, RuntimeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             'do not get evil')
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(xhasattr(obj, 1))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "attribute name must be string, not 'int'")
+
         # CRASHES xhasattr(obj, NULL)
         # CRASHES xhasattr(NULL, 'a')
 
@@ -123,8 +134,18 @@ def test_object_hasattrstring(self):
         self.assertFalse(hasattrstring(obj, b'b'))
         self.assertTrue(hasattrstring(obj, '\U0001f40d'.encode()))
 
-        self.assertFalse(hasattrstring(obj, b'evil'))
-        self.assertFalse(hasattrstring(obj, b'\xff'))
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(hasattrstring(obj, b'evil'))
+            self.assertEqual(cm.unraisable.exc_type, RuntimeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             'do not get evil')
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(hasattrstring(obj, b'\xff'))
+            self.assertEqual(cm.unraisable.exc_type, UnicodeDecodeError)
+            self.assertRegex(str(cm.unraisable.exc_value),
+                             "'utf-8' codec can't decode")
+
         # CRASHES hasattrstring(obj, NULL)
         # CRASHES hasattrstring(NULL, b'a')
 
@@ -342,12 +363,41 @@ def test_mapping_haskey(self):
 
         self.assertTrue(haskey(['a', 'b', 'c'], 1))
 
-        self.assertFalse(haskey(42, 'a'))
-        self.assertFalse(haskey({}, []))  # unhashable
-        self.assertFalse(haskey({}, NULL))
-        self.assertFalse(haskey([], 1))
-        self.assertFalse(haskey([], 'a'))
-        self.assertFalse(haskey(NULL, 'a'))
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskey(42, 'a'))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "'int' object is not subscriptable")
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskey({}, []))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "unhashable type: 'list'")
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskey([], 1))
+            self.assertEqual(cm.unraisable.exc_type, IndexError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             'list index out of range')
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskey([], 'a'))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             'list indices must be integers or slices, not str')
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskey({}, NULL))
+            self.assertEqual(cm.unraisable.exc_type, SystemError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             'null argument to internal routine')
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskey(NULL, 'a'))
+            self.assertEqual(cm.unraisable.exc_type, SystemError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             'null argument to internal routine')
 
     def test_mapping_haskeystring(self):
         haskeystring = _testcapi.mapping_haskeystring
@@ -360,11 +410,35 @@ def test_mapping_haskeystring(self):
         self.assertTrue(haskeystring(dct2, b'a'))
         self.assertFalse(haskeystring(dct2, b'b'))
 
-        self.assertFalse(haskeystring(42, b'a'))
-        self.assertFalse(haskeystring({}, b'\xff'))
-        self.assertFalse(haskeystring({}, NULL))
-        self.assertFalse(haskeystring([], b'a'))
-        self.assertFalse(haskeystring(NULL, b'a'))
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskeystring(42, b'a'))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "'int' object is not subscriptable")
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskeystring({}, b'\xff'))
+            self.assertEqual(cm.unraisable.exc_type, UnicodeDecodeError)
+            self.assertRegex(str(cm.unraisable.exc_value),
+                             "'utf-8' codec can't decode")
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskeystring({}, NULL))
+            self.assertEqual(cm.unraisable.exc_type, SystemError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "null argument to internal routine")
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskeystring([], b'a'))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             'list indices must be integers or slices, not str')
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(haskeystring(NULL, b'a'))
+            self.assertEqual(cm.unraisable.exc_type, SystemError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "null argument to internal routine")
 
     def test_mapping_haskeywitherror(self):
         haskey = _testcapi.mapping_haskeywitherror

Lib/test/test_capi/test_dict.py

@@ -1,6 +1,7 @@
 import unittest
 from collections import OrderedDict, UserDict
 from types import MappingProxyType
+from test import support
 import _testcapi
 
 
@@ -30,7 +31,7 @@ def test_dict_check(self):
         self.assertFalse(check(UserDict({1: 2})))
         self.assertFalse(check([1, 2]))
         self.assertFalse(check(object()))
-        #self.assertFalse(check(NULL))
+        # CRASHES check(NULL)
 
     def test_dict_checkexact(self):
         check = _testcapi.dict_checkexact
@@ -39,7 +40,7 @@ def test_dict_checkexact(self):
         self.assertFalse(check(UserDict({1: 2})))
         self.assertFalse(check([1, 2]))
         self.assertFalse(check(object()))
-        #self.assertFalse(check(NULL))
+        # CRASHES check(NULL)
 
     def test_dict_new(self):
         dict_new = _testcapi.dict_new
@@ -118,7 +119,12 @@ def test_dict_getitem(self):
         self.assertEqual(getitem(dct2, 'a'), 1)
         self.assertIs(getitem(dct2, 'b'), KeyError)
 
-        self.assertIs(getitem({}, []), KeyError)  # unhashable
+        with support.catch_unraisable_exception() as cm:
+            self.assertIs(getitem({}, []), KeyError)  # unhashable
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "unhashable type: 'list'")
+
         self.assertIs(getitem(42, 'a'), KeyError)
         self.assertIs(getitem([1], 0), KeyError)
         # CRASHES getitem({}, NULL)
@@ -135,7 +141,12 @@ def test_dict_getitemstring(self):
         self.assertEqual(getitemstring(dct2, b'a'), 1)
         self.assertIs(getitemstring(dct2, b'b'), KeyError)
 
-        self.assertIs(getitemstring({}, INVALID_UTF8), KeyError)
+        with support.catch_unraisable_exception() as cm:
+            self.assertIs(getitemstring({}, INVALID_UTF8), KeyError)
+            self.assertEqual(cm.unraisable.exc_type, UnicodeDecodeError)
+            self.assertRegex(str(cm.unraisable.exc_value),
+                             "'utf-8' codec can't decode")
+
         self.assertIs(getitemstring(42, b'a'), KeyError)
         self.assertIs(getitemstring([], b'a'), KeyError)
         # CRASHES getitemstring({}, NULL)

Lib/test/test_capi/test_sys.py

@@ -30,7 +30,11 @@ def test_sys_getobject(self):
             self.assertEqual(getobject('\U0001f40d'.encode()), 42)
 
         self.assertIs(getobject(b'nonexisting'), AttributeError)
-        self.assertIs(getobject(b'\xff'), AttributeError)
+        with support.catch_unraisable_exception() as cm:
+            self.assertIs(getobject(b'\xff'), AttributeError)
+            self.assertEqual(cm.unraisable.exc_type, UnicodeDecodeError)
+            self.assertRegex(str(cm.unraisable.exc_value),
+                             "'utf-8' codec can't decode")
         # CRASHES getobject(NULL)
 
     @support.cpython_only

Misc/NEWS.d/next/C API/2023-07-12-12-14-52.gh-issue-106672.fkRjmi.rst

@@ -0,0 +1,5 @@
+Functions :c:func:`PyDict_GetItem`, :c:func:`PyDict_GetItemString`,
+:c:func:`PyMapping_HasKey`, :c:func:`PyMapping_HasKeyString`,
+:c:func:`PyObject_HasAttr`, :c:func:`PyObject_HasAttrString`, and
+:c:func:`PySys_GetObject`, which clear all errors occurred during calling
+the function, report now them using :func:`sys.unraisablehook`.

Objects/abstract.c

@@ -2468,31 +2468,53 @@ PyMapping_HasKeyWithError(PyObject *obj, PyObject *key)
 }
 
 int
-PyMapping_HasKeyString(PyObject *o, const char *key)
+PyMapping_HasKeyString(PyObject *obj, const char *key)
 {
-    PyObject *v;
-
-    v = PyMapping_GetItemString(o, key);
-    if (v) {
-        Py_DECREF(v);
-        return 1;
+    PyObject *value;
+    int rc;
+    if (obj == NULL) {
+        // For backward compatibility.
+        // PyMapping_GetOptionalItemString() crashes if obj is NULL.
+        null_error();
+        rc = -1;
     }
-    PyErr_Clear();
-    return 0;
+    else {
+        rc = PyMapping_GetOptionalItemString(obj, key, &value);
+    }
+    if (rc < 0) {
+        PyErr_FormatUnraisable(
+            "Exception ignored in PyMapping_HasKeyString(); consider using "
+            "PyMapping_HasKeyStringWithError(), "
+            "PyMapping_GetOptionalItemString() or PyMapping_GetItemString()");
+        return 0;
+    }
+    Py_XDECREF(value);
+    return rc;
 }
 
 int
-PyMapping_HasKey(PyObject *o, PyObject *key)
+PyMapping_HasKey(PyObject *obj, PyObject *key)
 {
-    PyObject *v;
-
-    v = PyObject_GetItem(o, key);
-    if (v) {
-        Py_DECREF(v);
-        return 1;
+    PyObject *value;
+    int rc;
+    if (obj == NULL || key == NULL) {
+        // For backward compatibility.
+        // PyMapping_GetOptionalItem() crashes if any of them is NULL.
+        null_error();
+        rc = -1;
     }
-    PyErr_Clear();
-    return 0;
+    else {
+        rc = PyMapping_GetOptionalItem(obj, key, &value);
+    }
+    if (rc < 0) {
+        PyErr_FormatUnraisable(
+            "Exception ignored in PyMapping_HasKey(); consider using "
+            "PyMapping_HasKeyWithError(), "
+            "PyMapping_GetOptionalItem() or PyObject_GetItem()");
+        return 0;
+    }
+    Py_XDECREF(value);
+    return rc;
 }
 
 /* This function is quite similar to PySequence_Fast(), but specialized to be

Objects/dictobject.c

@@ -1663,8 +1663,8 @@ _PyDict_FromItems(PyObject *const *keys, Py_ssize_t keys_offset,
  * function hits a stack-depth error, which can cause this to return NULL
  * even if the key is present.
  */
-PyObject *
-PyDict_GetItem(PyObject *op, PyObject *key)
+static PyObject *
+dict_getitem(PyObject *op, PyObject *key, const char *warnmsg)
 {
     if (!PyDict_Check(op)) {
         return NULL;
@@ -1675,7 +1675,7 @@ PyDict_GetItem(PyObject *op, PyObject *key)
     if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
         hash = PyObject_Hash(key);
         if (hash == -1) {
-            PyErr_Clear();
+            PyErr_FormatUnraisable(warnmsg);
             return NULL;
         }
     }
@@ -1696,12 +1696,24 @@ PyDict_GetItem(PyObject *op, PyObject *key)
     ix = _Py_dict_lookup(mp, key, hash, &value);
 
     /* Ignore any exception raised by the lookup */
+    PyObject *exc2 = _PyErr_Occurred(tstate);
+    if (exc2 && !PyErr_GivenExceptionMatches(exc2, PyExc_KeyError)) {
+        PyErr_FormatUnraisable(warnmsg);
+    }
     _PyErr_SetRaisedException(tstate, exc);
 
     assert(ix >= 0 || value == NULL);
     return value;  // borrowed reference
 }
 
+PyObject *
+PyDict_GetItem(PyObject *op, PyObject *key)
+{
+    return dict_getitem(op, key,
+            "Exception ignored in PyDict_GetItem(); consider using "
+            "PyDict_GetItemRef() or PyDict_GetItemWithError()");
+}
+
 Py_ssize_t
 _PyDict_LookupIndex(PyDictObject *mp, PyObject *key)
 {
@@ -3925,10 +3937,14 @@ PyDict_GetItemString(PyObject *v, const char *key)
     PyObject *kv, *rv;
     kv = PyUnicode_FromString(key);
     if (kv == NULL) {
-        PyErr_Clear();
+        PyErr_FormatUnraisable(
+            "Exception ignored in PyDict_GetItemString(); consider using "
+            "PyDict_GetItemRefString()");
         return NULL;
     }
-    rv = PyDict_GetItem(v, kv);
+    rv = dict_getitem(v, kv,
+            "Exception ignored in PyDict_GetItemString(); consider using "
+            "PyDict_GetItemRefString()");
     Py_DECREF(kv);
     return rv;  // borrowed reference
 }

Objects/object.c

@@ -1040,7 +1040,10 @@ PyObject_HasAttrString(PyObject *obj, const char *name)
 {
     int rc = PyObject_HasAttrStringWithError(obj, name);
     if (rc < 0) {
-        PyErr_Clear();
+        PyErr_FormatUnraisable(
+            "Exception ignored in PyObject_HasAttrString(); consider using "
+            "PyObject_HasAttrStringWithError(), "
+            "PyObject_GetOptionalAttrString() or PyObject_GetAttrString()");
         return 0;
     }
     return rc;
@@ -1275,7 +1278,10 @@ PyObject_HasAttr(PyObject *obj, PyObject *name)
 {
     int rc = PyObject_HasAttrWithError(obj, name);
     if (rc < 0) {
-        PyErr_Clear();
+        PyErr_FormatUnraisable(
+            "Exception ignored in PyObject_HasAttr(); consider using "
+            "PyObject_HasAttrWithError(), "
+            "PyObject_GetOptionalAttr() or PyObject_GetAttr()");
         return 0;
     }
     return rc;

Python/sysmodule.c

@@ -110,6 +110,9 @@ PySys_GetObject(const char *name)
     PyObject *value = _PySys_GetObject(tstate->interp, name);
     /* XXX Suppress a new exception if it was raised and restore
      * the old one. */
+    if (_PyErr_Occurred(tstate)) {
+        PyErr_FormatUnraisable("Exception ignored in PySys_GetObject()");
+    }
     _PyErr_SetRaisedException(tstate, exc);
     return value;
 }