Skip to content

Commit f55cb44

Browse files
serhiy-storchakaserhiy-storchaka
authored
gh-106672: C API: Report indiscriminately ignored errors (GH-106674)
Functions which indiscriminately ignore all errors now report them as unraisable errors.
1 parent a077b2f commit f55cb44

File tree

9 files changed

+195
-45
lines changed

9 files changed

+195
-45
lines changed

‎Doc/whatsnew/3.13.rst

+9
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,15 @@ Changes in the Python API
980980
The result is now the same if ``wantobjects`` is set to ``0``.
981981
(Contributed by Serhiy Storchaka in :gh:`97928`.)
982982

983+
* Functions :c:func:`PyDict_GetItem`, :c:func:`PyDict_GetItemString`,
984+
:c:func:`PyMapping_HasKey`, :c:func:`PyMapping_HasKeyString`,
985+
:c:func:`PyObject_HasAttr`, :c:func:`PyObject_HasAttrString`, and
986+
:c:func:`PySys_GetObject`, which clear all errors occurred during calling
987+
the function, report now them using :func:`sys.unraisablehook`.
988+
You can consider to replace these functions with other functions as
989+
recomended in the documentation.
990+
(Contributed by Serhiy Storchaka in :gh:`106672`.)
991+
983992

984993
Build Changes
985994
=============

‎Lib/test/test_capi/test_abstract.py

+89-15
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import unittest
22
from collections import OrderedDict
3+
from test import support
34
from test.support import import_helper
45

56
_testcapi = import_helper.import_module('_testcapi')
@@ -109,8 +110,18 @@ def test_object_hasattr(self):
109110
self.assertFalse(xhasattr(obj, 'b'))
110111
self.assertTrue(xhasattr(obj, '\U0001f40d'))
111112

112-
self.assertFalse(xhasattr(obj, 'evil'))
113-
self.assertFalse(xhasattr(obj, 1))
113+
with support.catch_unraisable_exception() as cm:
114+
self.assertFalse(xhasattr(obj, 'evil'))
115+
self.assertEqual(cm.unraisable.exc_type, RuntimeError)
116+
self.assertEqual(str(cm.unraisable.exc_value),
117+
'do not get evil')
118+
119+
with support.catch_unraisable_exception() as cm:
120+
self.assertFalse(xhasattr(obj, 1))
121+
self.assertEqual(cm.unraisable.exc_type, TypeError)
122+
self.assertEqual(str(cm.unraisable.exc_value),
123+
"attribute name must be string, not 'int'")
124+
114125
# CRASHES xhasattr(obj, NULL)
115126
# CRASHES xhasattr(NULL, 'a')
116127

@@ -123,8 +134,18 @@ def test_object_hasattrstring(self):
123134
self.assertFalse(hasattrstring(obj, b'b'))
124135
self.assertTrue(hasattrstring(obj, '\U0001f40d'.encode()))
125136

126-
self.assertFalse(hasattrstring(obj, b'evil'))
127-
self.assertFalse(hasattrstring(obj, b'\xff'))
137+
with support.catch_unraisable_exception() as cm:
138+
self.assertFalse(hasattrstring(obj, b'evil'))
139+
self.assertEqual(cm.unraisable.exc_type, RuntimeError)
140+
self.assertEqual(str(cm.unraisable.exc_value),
141+
'do not get evil')
142+
143+
with support.catch_unraisable_exception() as cm:
144+
self.assertFalse(hasattrstring(obj, b'\xff'))
145+
self.assertEqual(cm.unraisable.exc_type, UnicodeDecodeError)
146+
self.assertRegex(str(cm.unraisable.exc_value),
147+
"'utf-8' codec can't decode")
148+
128149
# CRASHES hasattrstring(obj, NULL)
129150
# CRASHES hasattrstring(NULL, b'a')
130151

@@ -342,12 +363,41 @@ def test_mapping_haskey(self):
342363

343364
self.assertTrue(haskey(['a', 'b', 'c'], 1))
344365

345-
self.assertFalse(haskey(42, 'a'))
346-
self.assertFalse(haskey({}, [])) # unhashable
347-
self.assertFalse(haskey({}, NULL))
348-
self.assertFalse(haskey([], 1))
349-
self.assertFalse(haskey([], 'a'))
350-
self.assertFalse(haskey(NULL, 'a'))
366+
with support.catch_unraisable_exception() as cm:
367+
self.assertFalse(haskey(42, 'a'))
368+
self.assertEqual(cm.unraisable.exc_type, TypeError)
369+
self.assertEqual(str(cm.unraisable.exc_value),
370+
"'int' object is not subscriptable")
371+
372+
with support.catch_unraisable_exception() as cm:
373+
self.assertFalse(haskey({}, []))
374+
self.assertEqual(cm.unraisable.exc_type, TypeError)
375+
self.assertEqual(str(cm.unraisable.exc_value),
376+
"unhashable type: 'list'")
377+
378+
with support.catch_unraisable_exception() as cm:
379+
self.assertFalse(haskey([], 1))
380+
self.assertEqual(cm.unraisable.exc_type, IndexError)
381+
self.assertEqual(str(cm.unraisable.exc_value),
382+
'list index out of range')
383+
384+
with support.catch_unraisable_exception() as cm:
385+
self.assertFalse(haskey([], 'a'))
386+
self.assertEqual(cm.unraisable.exc_type, TypeError)
387+
self.assertEqual(str(cm.unraisable.exc_value),
388+
'list indices must be integers or slices, not str')
389+
390+
with support.catch_unraisable_exception() as cm:
391+
self.assertFalse(haskey({}, NULL))
392+
self.assertEqual(cm.unraisable.exc_type, SystemError)
393+
self.assertEqual(str(cm.unraisable.exc_value),
394+
'null argument to internal routine')
395+
396+
with support.catch_unraisable_exception() as cm:
397+
self.assertFalse(haskey(NULL, 'a'))
398+
self.assertEqual(cm.unraisable.exc_type, SystemError)
399+
self.assertEqual(str(cm.unraisable.exc_value),
400+
'null argument to internal routine')
351401

352402
def test_mapping_haskeystring(self):
353403
haskeystring = _testcapi.mapping_haskeystring
@@ -360,11 +410,35 @@ def test_mapping_haskeystring(self):
360410
self.assertTrue(haskeystring(dct2, b'a'))
361411
self.assertFalse(haskeystring(dct2, b'b'))
362412

363-
self.assertFalse(haskeystring(42, b'a'))
364-
self.assertFalse(haskeystring({}, b'\xff'))
365-
self.assertFalse(haskeystring({}, NULL))
366-
self.assertFalse(haskeystring([], b'a'))
367-
self.assertFalse(haskeystring(NULL, b'a'))
413+
with support.catch_unraisable_exception() as cm:
414+
self.assertFalse(haskeystring(42, b'a'))
415+
self.assertEqual(cm.unraisable.exc_type, TypeError)
416+
self.assertEqual(str(cm.unraisable.exc_value),
417+
"'int' object is not subscriptable")
418+
419+
with support.catch_unraisable_exception() as cm:
420+
self.assertFalse(haskeystring({}, b'\xff'))
421+
self.assertEqual(cm.unraisable.exc_type, UnicodeDecodeError)
422+
self.assertRegex(str(cm.unraisable.exc_value),
423+
"'utf-8' codec can't decode")
424+
425+
with support.catch_unraisable_exception() as cm:
426+
self.assertFalse(haskeystring({}, NULL))
427+
self.assertEqual(cm.unraisable.exc_type, SystemError)
428+
self.assertEqual(str(cm.unraisable.exc_value),
429+
"null argument to internal routine")
430+
431+
with support.catch_unraisable_exception() as cm:
432+
self.assertFalse(haskeystring([], b'a'))
433+
self.assertEqual(cm.unraisable.exc_type, TypeError)
434+
self.assertEqual(str(cm.unraisable.exc_value),
435+
'list indices must be integers or slices, not str')
436+
437+
with support.catch_unraisable_exception() as cm:
438+
self.assertFalse(haskeystring(NULL, b'a'))
439+
self.assertEqual(cm.unraisable.exc_type, SystemError)
440+
self.assertEqual(str(cm.unraisable.exc_value),
441+
"null argument to internal routine")
368442

369443
def test_mapping_haskeywitherror(self):
370444
haskey = _testcapi.mapping_haskeywitherror

‎Lib/test/test_capi/test_dict.py

+15-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import unittest
22
from collections import OrderedDict, UserDict
33
from types import MappingProxyType
4+
from test import support
45
import _testcapi
56

67

@@ -30,7 +31,7 @@ def test_dict_check(self):
3031
self.assertFalse(check(UserDict({1: 2})))
3132
self.assertFalse(check([1, 2]))
3233
self.assertFalse(check(object()))
33-
#self.assertFalse(check(NULL))
34+
# CRASHES check(NULL)
3435

3536
def test_dict_checkexact(self):
3637
check = _testcapi.dict_checkexact
@@ -39,7 +40,7 @@ def test_dict_checkexact(self):
3940
self.assertFalse(check(UserDict({1: 2})))
4041
self.assertFalse(check([1, 2]))
4142
self.assertFalse(check(object()))
42-
#self.assertFalse(check(NULL))
43+
# CRASHES check(NULL)
4344

4445
def test_dict_new(self):
4546
dict_new = _testcapi.dict_new
@@ -118,7 +119,12 @@ def test_dict_getitem(self):
118119
self.assertEqual(getitem(dct2, 'a'), 1)
119120
self.assertIs(getitem(dct2, 'b'), KeyError)
120121

121-
self.assertIs(getitem({}, []), KeyError) # unhashable
122+
with support.catch_unraisable_exception() as cm:
123+
self.assertIs(getitem({}, []), KeyError) # unhashable
124+
self.assertEqual(cm.unraisable.exc_type, TypeError)
125+
self.assertEqual(str(cm.unraisable.exc_value),
126+
"unhashable type: 'list'")
127+
122128
self.assertIs(getitem(42, 'a'), KeyError)
123129
self.assertIs(getitem([1], 0), KeyError)
124130
# CRASHES getitem({}, NULL)
@@ -135,7 +141,12 @@ def test_dict_getitemstring(self):
135141
self.assertEqual(getitemstring(dct2, b'a'), 1)
136142
self.assertIs(getitemstring(dct2, b'b'), KeyError)
137143

138-
self.assertIs(getitemstring({}, INVALID_UTF8), KeyError)
144+
with support.catch_unraisable_exception() as cm:
145+
self.assertIs(getitemstring({}, INVALID_UTF8), KeyError)
146+
self.assertEqual(cm.unraisable.exc_type, UnicodeDecodeError)
147+
self.assertRegex(str(cm.unraisable.exc_value),
148+
"'utf-8' codec can't decode")
149+
139150
self.assertIs(getitemstring(42, b'a'), KeyError)
140151
self.assertIs(getitemstring([], b'a'), KeyError)
141152
# CRASHES getitemstring({}, NULL)

‎Lib/test/test_capi/test_sys.py

+5-1
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,11 @@ def test_sys_getobject(self):
3030
self.assertEqual(getobject('\U0001f40d'.encode()), 42)
3131

3232
self.assertIs(getobject(b'nonexisting'), AttributeError)
33-
self.assertIs(getobject(b'\xff'), AttributeError)
33+
with support.catch_unraisable_exception() as cm:
34+
self.assertIs(getobject(b'\xff'), AttributeError)
35+
self.assertEqual(cm.unraisable.exc_type, UnicodeDecodeError)
36+
self.assertRegex(str(cm.unraisable.exc_value),
37+
"'utf-8' codec can't decode")
3438
# CRASHES getobject(NULL)
3539

3640
@support.cpython_only

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

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

‎Objects/abstract.c

+40-18
Original file line numberDiff line numberDiff line change
@@ -2468,31 +2468,53 @@ PyMapping_HasKeyWithError(PyObject *obj, PyObject *key)
24682468
}
24692469

24702470
int
2471-
PyMapping_HasKeyString(PyObject *o, const char *key)
2471+
PyMapping_HasKeyString(PyObject *obj, const char *key)
24722472
{
2473-
PyObject *v;
2474-
2475-
v = PyMapping_GetItemString(o, key);
2476-
if (v) {
2477-
Py_DECREF(v);
2478-
return 1;
2473+
PyObject *value;
2474+
int rc;
2475+
if (obj == NULL) {
2476+
// For backward compatibility.
2477+
// PyMapping_GetOptionalItemString() crashes if obj is NULL.
2478+
null_error();
2479+
rc = -1;
24792480
}
2480-
PyErr_Clear();
2481-
return 0;
2481+
else {
2482+
rc = PyMapping_GetOptionalItemString(obj, key, &value);
2483+
}
2484+
if (rc < 0) {
2485+
PyErr_FormatUnraisable(
2486+
"Exception ignored in PyMapping_HasKeyString(); consider using "
2487+
"PyMapping_HasKeyStringWithError(), "
2488+
"PyMapping_GetOptionalItemString() or PyMapping_GetItemString()");
2489+
return 0;
2490+
}
2491+
Py_XDECREF(value);
2492+
return rc;
24822493
}
24832494

24842495
int
2485-
PyMapping_HasKey(PyObject *o, PyObject *key)
2496+
PyMapping_HasKey(PyObject *obj, PyObject *key)
24862497
{
2487-
PyObject *v;
2488-
2489-
v = PyObject_GetItem(o, key);
2490-
if (v) {
2491-
Py_DECREF(v);
2492-
return 1;
2498+
PyObject *value;
2499+
int rc;
2500+
if (obj == NULL || key == NULL) {
2501+
// For backward compatibility.
2502+
// PyMapping_GetOptionalItem() crashes if any of them is NULL.
2503+
null_error();
2504+
rc = -1;
24932505
}
2494-
PyErr_Clear();
2495-
return 0;
2506+
else {
2507+
rc = PyMapping_GetOptionalItem(obj, key, &value);
2508+
}
2509+
if (rc < 0) {
2510+
PyErr_FormatUnraisable(
2511+
"Exception ignored in PyMapping_HasKey(); consider using "
2512+
"PyMapping_HasKeyWithError(), "
2513+
"PyMapping_GetOptionalItem() or PyObject_GetItem()");
2514+
return 0;
2515+
}
2516+
Py_XDECREF(value);
2517+
return rc;
24962518
}
24972519

24982520
/* This function is quite similar to PySequence_Fast(), but specialized to be

‎Objects/dictobject.c

+21-5
Original file line numberDiff line numberDiff line change
@@ -1663,8 +1663,8 @@ _PyDict_FromItems(PyObject *const *keys, Py_ssize_t keys_offset,
16631663
* function hits a stack-depth error, which can cause this to return NULL
16641664
* even if the key is present.
16651665
*/
1666-
PyObject *
1667-
PyDict_GetItem(PyObject *op, PyObject *key)
1666+
static PyObject *
1667+
dict_getitem(PyObject *op, PyObject *key, const char *warnmsg)
16681668
{
16691669
if (!PyDict_Check(op)) {
16701670
return NULL;
@@ -1675,7 +1675,7 @@ PyDict_GetItem(PyObject *op, PyObject *key)
16751675
if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
16761676
hash = PyObject_Hash(key);
16771677
if (hash == -1) {
1678-
PyErr_Clear();
1678+
PyErr_FormatUnraisable(warnmsg);
16791679
return NULL;
16801680
}
16811681
}
@@ -1696,12 +1696,24 @@ PyDict_GetItem(PyObject *op, PyObject *key)
16961696
ix = _Py_dict_lookup(mp, key, hash, &value);
16971697

16981698
/* Ignore any exception raised by the lookup */
1699+
PyObject *exc2 = _PyErr_Occurred(tstate);
1700+
if (exc2 && !PyErr_GivenExceptionMatches(exc2, PyExc_KeyError)) {
1701+
PyErr_FormatUnraisable(warnmsg);
1702+
}
16991703
_PyErr_SetRaisedException(tstate, exc);
17001704

17011705
assert(ix >= 0 || value == NULL);
17021706
return value; // borrowed reference
17031707
}
17041708

1709+
PyObject *
1710+
PyDict_GetItem(PyObject *op, PyObject *key)
1711+
{
1712+
return dict_getitem(op, key,
1713+
"Exception ignored in PyDict_GetItem(); consider using "
1714+
"PyDict_GetItemRef() or PyDict_GetItemWithError()");
1715+
}
1716+
17051717
Py_ssize_t
17061718
_PyDict_LookupIndex(PyDictObject *mp, PyObject *key)
17071719
{
@@ -3925,10 +3937,14 @@ PyDict_GetItemString(PyObject *v, const char *key)
39253937
PyObject *kv, *rv;
39263938
kv = PyUnicode_FromString(key);
39273939
if (kv == NULL) {
3928-
PyErr_Clear();
3940+
PyErr_FormatUnraisable(
3941+
"Exception ignored in PyDict_GetItemString(); consider using "
3942+
"PyDict_GetItemRefString()");
39293943
return NULL;
39303944
}
3931-
rv = PyDict_GetItem(v, kv);
3945+
rv = dict_getitem(v, kv,
3946+
"Exception ignored in PyDict_GetItemString(); consider using "
3947+
"PyDict_GetItemRefString()");
39323948
Py_DECREF(kv);
39333949
return rv; // borrowed reference
39343950
}

‎Objects/object.c

+8-2
Original file line numberDiff line numberDiff line change
@@ -1040,7 +1040,10 @@ PyObject_HasAttrString(PyObject *obj, const char *name)
10401040
{
10411041
int rc = PyObject_HasAttrStringWithError(obj, name);
10421042
if (rc < 0) {
1043-
PyErr_Clear();
1043+
PyErr_FormatUnraisable(
1044+
"Exception ignored in PyObject_HasAttrString(); consider using "
1045+
"PyObject_HasAttrStringWithError(), "
1046+
"PyObject_GetOptionalAttrString() or PyObject_GetAttrString()");
10441047
return 0;
10451048
}
10461049
return rc;
@@ -1275,7 +1278,10 @@ PyObject_HasAttr(PyObject *obj, PyObject *name)
12751278
{
12761279
int rc = PyObject_HasAttrWithError(obj, name);
12771280
if (rc < 0) {
1278-
PyErr_Clear();
1281+
PyErr_FormatUnraisable(
1282+
"Exception ignored in PyObject_HasAttr(); consider using "
1283+
"PyObject_HasAttrWithError(), "
1284+
"PyObject_GetOptionalAttr() or PyObject_GetAttr()");
12791285
return 0;
12801286
}
12811287
return rc;

‎Python/sysmodule.c

+3
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ PySys_GetObject(const char *name)
110110
PyObject *value = _PySys_GetObject(tstate->interp, name);
111111
/* XXX Suppress a new exception if it was raised and restore
112112
* the old one. */
113+
if (_PyErr_Occurred(tstate)) {
114+
PyErr_FormatUnraisable("Exception ignored in PySys_GetObject()");
115+
}
113116
_PyErr_SetRaisedException(tstate, exc);
114117
return value;
115118
}

0 commit comments

Comments
 (0)