summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSerhiy Storchaka <storchaka@gmail.com>2023-11-07 13:58:04 (GMT)
committerGitHub <noreply@github.com>2023-11-07 13:58:04 (GMT)
commitf55cb44359821e71c29903f2152b4658509dac0d (patch)
tree853f01ca01f1ab39b0bc64849296946cba1637c4
parenta077b2fbb88f5192bb47e514334f760bf08d0295 (diff)
downloadcpython-f55cb44359821e71c29903f2152b4658509dac0d.zip
cpython-f55cb44359821e71c29903f2152b4658509dac0d.tar.gz
cpython-f55cb44359821e71c29903f2152b4658509dac0d.tar.bz2
gh-106672: C API: Report indiscriminately ignored errors (GH-106674)
Functions which indiscriminately ignore all errors now report them as unraisable errors.
-rw-r--r--Doc/whatsnew/3.13.rst9
-rw-r--r--Lib/test/test_capi/test_abstract.py104
-rw-r--r--Lib/test/test_capi/test_dict.py19
-rw-r--r--Lib/test/test_capi/test_sys.py6
-rw-r--r--Misc/NEWS.d/next/C API/2023-07-12-12-14-52.gh-issue-106672.fkRjmi.rst5
-rw-r--r--Objects/abstract.c58
-rw-r--r--Objects/dictobject.c26
-rw-r--r--Objects/object.c10
-rw-r--r--Python/sysmodule.c3
9 files changed, 195 insertions, 45 deletions
diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst
index ef83f66..cca2827 100644
--- a/Doc/whatsnew/3.13.rst
+++ b/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
=============
diff --git a/Lib/test/test_capi/test_abstract.py b/Lib/test/test_capi/test_abstract.py
index 5b5dfb3..26152c3 100644
--- a/Lib/test/test_capi/test_abstract.py
+++ b/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 @@ class CAPITest(unittest.TestCase):
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 @@ class CAPITest(unittest.TestCase):
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 @@ class CAPITest(unittest.TestCase):
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 @@ class CAPITest(unittest.TestCase):
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
diff --git a/Lib/test/test_capi/test_dict.py b/Lib/test/test_capi/test_dict.py
index 11b2ca9..67f12a5 100644
--- a/Lib/test/test_capi/test_dict.py
+++ b/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 @@ class CAPITest(unittest.TestCase):
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 @@ class CAPITest(unittest.TestCase):
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 @@ class CAPITest(unittest.TestCase):
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 @@ class CAPITest(unittest.TestCase):
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)
diff --git a/Lib/test/test_capi/test_sys.py b/Lib/test/test_capi/test_sys.py
index b83a0a1..08bf037 100644
--- a/Lib/test/test_capi/test_sys.py
+++ b/Lib/test/test_capi/test_sys.py
@@ -30,7 +30,11 @@ class CAPITest(unittest.TestCase):
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
diff --git a/Misc/NEWS.d/next/C API/2023-07-12-12-14-52.gh-issue-106672.fkRjmi.rst b/Misc/NEWS.d/next/C API/2023-07-12-12-14-52.gh-issue-106672.fkRjmi.rst
new file mode 100644
index 0000000..420f431
--- /dev/null
+++ b/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`.
diff --git a/Objects/abstract.c b/Objects/abstract.c
index 070e762..b676289 100644
--- a/Objects/abstract.c
+++ b/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
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 5aa2c0d..719d438 100644
--- a/Objects/dictobject.c
+++ b/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
}
diff --git a/Objects/object.c b/Objects/object.c
index 1cc3f91..b561da7 100644
--- a/Objects/object.c
+++ b/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;
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
index 4008a28..e285232 100644
--- a/Python/sysmodule.c
+++ b/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;
}