Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-106672: C API: Report indiscriminately ignored errors #106674

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 79 additions & 10 deletions Lib/test/test_capi/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
import _testinternalcapi


NULL = None

def decode_stderr(err):
return err.decode('utf-8', 'replace').replace('\r', '')

Expand Down Expand Up @@ -338,16 +340,83 @@ def items(self):
self.assertRaises(TypeError, _testcapi.get_mapping_items, bad_mapping)

def test_mapping_has_key(self):
dct = {'a': 1}
self.assertTrue(_testcapi.mapping_has_key(dct, 'a'))
self.assertFalse(_testcapi.mapping_has_key(dct, 'b'))

class SubDict(dict):
pass

dct2 = SubDict({'a': 1})
self.assertTrue(_testcapi.mapping_has_key(dct2, 'a'))
self.assertFalse(_testcapi.mapping_has_key(dct2, 'b'))
has_key = _testcapi.mapping_has_key
dct = {'a': 1, '\U0001f40d': 2}
self.assertTrue(has_key(dct, 'a'))
self.assertFalse(has_key(dct, 'b'))
self.assertTrue(has_key(dct, '\U0001f40d'))

class M:
def __getitem__(self, key):
return dct[key]

dct2 = M()
self.assertTrue(has_key(dct2, 'a'))
self.assertFalse(has_key(dct2, 'b'))

with support.catch_unraisable_exception() as cm:
self.assertFalse(has_key(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(has_key({}, []))
self.assertEqual(cm.unraisable.exc_type, TypeError)
self.assertEqual(str(cm.unraisable.exc_value),
"unhashable type: 'list'")

self.assertTrue(has_key(['a', 'b'], 1))
with support.catch_unraisable_exception() as cm:
self.assertFalse(has_key([], 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(has_key([], 'a'))
self.assertEqual(cm.unraisable.exc_type, TypeError)
self.assertEqual(str(cm.unraisable.exc_value),
'list indices must be integers or slices, not str')

def test_mapping_has_key_string(self):
has_key_string = _testcapi.mapping_has_key_string
dct = {'a': 1, '\U0001f40d': 2}
self.assertTrue(has_key_string(dct, b'a'))
self.assertFalse(has_key_string(dct, b'b'))
self.assertTrue(has_key_string(dct, '\U0001f40d'.encode()))

class M:
def __getitem__(self, key):
return dct[key]

dct2 = M()
self.assertTrue(has_key_string(dct2, b'a'))
self.assertFalse(has_key_string(dct2, b'b'))

with support.catch_unraisable_exception() as cm:
self.assertFalse(has_key_string(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(has_key_string({}, 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(has_key_string({}, 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(has_key_string([], 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')

def test_sequence_set_slice(self):
# Correct case:
Expand Down
Original file line number Diff line number Diff line change
@@ -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`.
serhiy-storchaka marked this conversation as resolved.
Show resolved Hide resolved
34 changes: 11 additions & 23 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2078,37 +2078,25 @@ get_mapping_items(PyObject* self, PyObject *obj)
}

static PyObject *
test_mapping_has_key_string(PyObject *self, PyObject *Py_UNUSED(args))
mapping_has_key(PyObject* self, PyObject *args)
{
PyObject *context = PyDict_New();
PyObject *val = PyLong_FromLong(1);

// Since this uses `const char*` it is easier to test this in C:
PyDict_SetItemString(context, "a", val);
if (!PyMapping_HasKeyString(context, "a")) {
PyErr_SetString(PyExc_RuntimeError,
"Existing mapping key does not exist");
return NULL;
}
if (PyMapping_HasKeyString(context, "b")) {
PyErr_SetString(PyExc_RuntimeError,
"Missing mapping key exists");
PyObject *context, *key;
if (!PyArg_ParseTuple(args, "OO", &context, &key)) {
return NULL;
}

Py_DECREF(val);
Py_DECREF(context);
Py_RETURN_NONE;
return PyLong_FromLong(PyMapping_HasKey(context, key));
}

static PyObject *
mapping_has_key(PyObject* self, PyObject *args)
mapping_has_key_string(PyObject* self, PyObject *args)
{
PyObject *context, *key;
if (!PyArg_ParseTuple(args, "OO", &context, &key)) {
PyObject *context;
const char *key;
Py_ssize_t size;
if (!PyArg_ParseTuple(args, "Oz#", &context, &key, &size)) {
return NULL;
}
return PyLong_FromLong(PyMapping_HasKey(context, key));
return PyLong_FromLong(PyMapping_HasKeyString(context, key));
}

static PyObject *
Expand Down Expand Up @@ -3548,8 +3536,8 @@ static PyMethodDef TestMethods[] = {
{"get_mapping_keys", get_mapping_keys, METH_O},
{"get_mapping_values", get_mapping_values, METH_O},
{"get_mapping_items", get_mapping_items, METH_O},
{"test_mapping_has_key_string", test_mapping_has_key_string, METH_NOARGS},
{"mapping_has_key", mapping_has_key, METH_VARARGS},
{"mapping_has_key_string", mapping_has_key_string, METH_VARARGS},
{"sequence_set_slice", sequence_set_slice, METH_VARARGS},
{"sequence_del_slice", sequence_del_slice, METH_VARARGS},
{"test_pythread_tss_key_state", test_pythread_tss_key_state, METH_VARARGS},
Expand Down
42 changes: 24 additions & 18 deletions Objects/abstract.c
Original file line number Diff line number Diff line change
Expand Up @@ -2426,31 +2426,37 @@ PyMapping_SetItemString(PyObject *o, const char *key, PyObject *value)
}

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 *dummy;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename to 'item' or 'value'.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

int rc = PyMapping_GetOptionalItemString(obj, key, &dummy);
if (rc < 0) {
_PyErr_WriteUnraisableMsg("on testing a mapping key", obj);
return 0;
}
PyErr_Clear();
return 0;
if (rc == 0 && PyErr_Occurred()) {
_PyErr_WriteUnraisableMsg("before testing a mapping key", obj);
return 0;
}
Py_XDECREF(dummy);
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 *dummy;
int rc = PyMapping_GetOptionalItem(obj, key, &dummy);
if (rc < 0) {
_PyErr_WriteUnraisableMsg("on testing a mapping key", obj);
return 0;
}
PyErr_Clear();
return 0;
if (rc == 0 && PyErr_Occurred()) {
_PyErr_WriteUnraisableMsg("before testing a mapping key", obj);
return 0;
}
Py_XDECREF(dummy);
return rc;
}

/* This function is quite similar to PySequence_Fast(), but specialized to be
Expand Down
8 changes: 6 additions & 2 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1673,7 +1673,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_WriteUnraisableMsg("on getting a dict key", op);
return NULL;
}
}
Expand All @@ -1694,6 +1694,10 @@ 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_WriteUnraisableMsg("on getting a dict key", op);
}
_PyErr_SetRaisedException(tstate, exc);


Expand Down Expand Up @@ -3889,7 +3893,7 @@ PyDict_GetItemString(PyObject *v, const char *key)
PyObject *kv, *rv;
kv = PyUnicode_FromString(key);
if (kv == NULL) {
PyErr_Clear();
_PyErr_WriteUnraisableMsg("on getting a dict key", v);
return NULL;
}
rv = PyDict_GetItem(v, kv);
Expand Down
40 changes: 14 additions & 26 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -904,26 +904,16 @@ PyObject_GetAttrString(PyObject *v, const char *name)
}

int
PyObject_HasAttrString(PyObject *v, const char *name)
PyObject_HasAttrString(PyObject *obj, const char *name)
{
if (Py_TYPE(v)->tp_getattr != NULL) {
PyObject *res = (*Py_TYPE(v)->tp_getattr)(v, (char*)name);
if (res != NULL) {
Py_DECREF(res);
return 1;
}
PyErr_Clear();
return 0;
}

PyObject *attr_name = PyUnicode_FromString(name);
if (attr_name == NULL) {
PyErr_Clear();
PyObject *dummy;
int rc = PyObject_GetOptionalAttrString(obj, name, &dummy);
if (rc < 0) {
_PyErr_WriteUnraisableMsg("on testing an object attribute", obj);
return 0;
}
int ok = PyObject_HasAttr(v, attr_name);
Py_DECREF(attr_name);
return ok;
Py_XDECREF(dummy);
return rc;
}

int
Expand Down Expand Up @@ -1142,18 +1132,16 @@ PyObject_GetOptionalAttrString(PyObject *obj, const char *name, PyObject **resul
}

int
PyObject_HasAttr(PyObject *v, PyObject *name)
PyObject_HasAttr(PyObject *obj, PyObject *name)
{
PyObject *res;
if (PyObject_GetOptionalAttr(v, name, &res) < 0) {
PyErr_Clear();
PyObject *dummy;
int rc = PyObject_GetOptionalAttr(obj, name, &dummy);
if (rc < 0) {
_PyErr_WriteUnraisableMsg("on testing an object attribute", obj);
return 0;
}
if (res == NULL) {
return 0;
}
Py_DECREF(res);
return 1;
Py_XDECREF(dummy);
return rc;
}

int
Expand Down
3 changes: 3 additions & 0 deletions Python/sysmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,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_WriteUnraisableMsg("on getting the sys module attribute", NULL);
}
_PyErr_SetRaisedException(tstate, exc);
return value;
}
Expand Down