From ff03c61a841cb55e8dbfe768c0d4f2558c59e61e Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Mon, 13 Mar 2023 23:29:44 +0000 Subject: [PATCH 1/6] PyErr_SetObject adds note to exception raised on normalization error --- Include/cpython/pyerrors.h | 4 ++++ Lib/test/test_capi/test_exceptions.py | 20 +++++++++++++++++ Modules/_testcapi/exceptions.c | 21 ++++++++++++++++++ Objects/exceptions.c | 6 +++++ Python/errors.c | 32 +++++++++++++++++++++++++++ 5 files changed, 83 insertions(+) diff --git a/Include/cpython/pyerrors.h b/Include/cpython/pyerrors.h index 0d9cc9922f7368..db27956d723cd8 100644 --- a/Include/cpython/pyerrors.h +++ b/Include/cpython/pyerrors.h @@ -112,6 +112,10 @@ PyAPI_FUNC(PyObject *) _PyErr_FormatFromCause( /* In exceptions.c */ +PyAPI_FUNC(PyObject*) _PyException_AddNote( + PyBaseExceptionObject *exc, + PyObject *note); + /* Helper that attempts to replace the current exception with one of the * same type but with a prefix added to the exception text. The resulting * exception description looks like: diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index 55f131699a2567..47af5d9d01b8c0 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -169,5 +169,25 @@ class Broken(Exception, metaclass=Meta): with self.assertRaises(ZeroDivisionError) as e: _testcapi.exc_set_object(Broken, Broken()) + def test_set_object_and_fetch(self): + class Broken(Exception): + def __init__(self, *arg): + raise ValueError("Broken __init__") + + exc = _testcapi.exc_set_object_fetch(Broken, ('abcd')) + self.assertIsInstance(exc, ValueError) + self.assertEqual(exc.__notes__[0], + "Normalization failed: type=Broken args='abcd'") + + class BadArg: + def __repr__(self): + raise TypeError('Broken arg type') + + exc = _testcapi.exc_set_object_fetch(Broken, (BadArg())) + self.assertIsInstance(exc, ValueError) + self.assertEqual(exc.__notes__[0], + 'Normalization failed: type=Broken args=') + + if __name__ == "__main__": unittest.main() diff --git a/Modules/_testcapi/exceptions.c b/Modules/_testcapi/exceptions.c index a0575213987ffc..c64b823663c32f 100644 --- a/Modules/_testcapi/exceptions.c +++ b/Modules/_testcapi/exceptions.c @@ -92,6 +92,26 @@ exc_set_object(PyObject *self, PyObject *args) return NULL; } +static PyObject * +exc_set_object_fetch(PyObject *self, PyObject *args) +{ + PyObject *exc; + PyObject *obj; + PyObject *type; + PyObject *value; + PyObject *tb; + + if (!PyArg_ParseTuple(args, "OO:exc_set_object", &exc, &obj)) { + return NULL; + } + + PyErr_SetObject(exc, obj); + PyErr_Fetch(&type, &value, &tb); + Py_XDECREF(type); + Py_XDECREF(tb); + return value; +} + static PyObject * raise_exception(PyObject *self, PyObject *args) { @@ -262,6 +282,7 @@ static PyMethodDef test_methods[] = { {"make_exception_with_doc", _PyCFunction_CAST(make_exception_with_doc), METH_VARARGS | METH_KEYWORDS}, {"exc_set_object", exc_set_object, METH_VARARGS}, + {"exc_set_object_fetch", exc_set_object_fetch, METH_VARARGS}, {"raise_exception", raise_exception, METH_VARARGS}, {"raise_memoryerror", raise_memoryerror, METH_NOARGS}, {"set_exc_info", test_set_exc_info, METH_VARARGS}, diff --git a/Objects/exceptions.c b/Objects/exceptions.c index c6fb6a3f19b2d0..96acfce1cbde58 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -3749,6 +3749,12 @@ _PyExc_Fini(PyInterpreterState *interp) _PyExc_FiniTypes(interp); } +PyObject * +_PyException_AddNote(PyBaseExceptionObject *exc, PyObject *note) +{ + return BaseException_add_note((PyObject *)exc, note); +} + /* Helper to do the equivalent of "raise X from Y" in C, but always using * the current exception rather than passing one in. * diff --git a/Python/errors.c b/Python/errors.c index bbf6d397ce8097..d33adf45ac1cf9 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -135,6 +135,28 @@ _PyErr_GetTopmostException(PyThreadState *tstate) return exc_info; } +PyObject * +get_normalization_failure_note(PyThreadState *tstate, PyObject *exception, PyObject *value) +{ + PyObject *args = PyObject_Repr(value); + if (args == NULL) { + _PyErr_Clear(tstate); + args = PyUnicode_FromFormat(""); + } + PyObject *note; + const char *tpname = ((PyTypeObject*)exception)->tp_name; + if (args == NULL) { + _PyErr_Clear(tstate); + note = PyUnicode_FromFormat("Normalization failed: type=%s", tpname); + } + else { + note = PyUnicode_FromFormat("Normalization failed: type=%s args=%S", + tpname, args); + Py_DECREF(args); + } + return note; +} + void _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) { @@ -169,6 +191,16 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) fixed_value = _PyErr_CreateException(exception, value); Py_XDECREF(value); if (fixed_value == NULL) { + PyObject *exc = _PyErr_GetRaisedException(tstate); + assert(PyExceptionInstance_Check(exc)); + + PyObject *note = get_normalization_failure_note(tstate, exception, value); + if (note != NULL) { + PyObject *res = _PyException_AddNote((PyBaseExceptionObject*)exc, note); + Py_DECREF(note); + Py_XDECREF(res); + } + _PyErr_SetRaisedException(tstate, exc); return; } From 31e6415b21b394b1d87b1e5d44105a7955195eb3 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 14 Mar 2023 00:11:49 +0000 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2023-03-14-00-11-46.gh-issue-102594.BjU-m2.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-03-14-00-11-46.gh-issue-102594.BjU-m2.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-03-14-00-11-46.gh-issue-102594.BjU-m2.rst b/Misc/NEWS.d/next/Core and Builtins/2023-03-14-00-11-46.gh-issue-102594.BjU-m2.rst new file mode 100644 index 00000000000000..0b95b5ec98e811 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-03-14-00-11-46.gh-issue-102594.BjU-m2.rst @@ -0,0 +1 @@ +Add note to exception raised in ``PyErr_SetObject`` when normalization fails. From 827a48c9fcc58eb694ebc3f187c222aa73e2651c Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 14 Mar 2023 10:15:39 +0000 Subject: [PATCH 3/6] add missing 'static' --- Python/errors.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/errors.c b/Python/errors.c index d33adf45ac1cf9..7dbd48bd71ca00 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -135,7 +135,7 @@ _PyErr_GetTopmostException(PyThreadState *tstate) return exc_info; } -PyObject * +static PyObject * get_normalization_failure_note(PyThreadState *tstate, PyObject *exception, PyObject *value) { PyObject *args = PyObject_Repr(value); From 4333933ee1ce0235bac7ff6dffda504cb69f3570 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 14 Mar 2023 19:49:52 +0000 Subject: [PATCH 4/6] apply offline comments from Guido --- Include/cpython/pyerrors.h | 4 ++-- Lib/test/test_capi/test_exceptions.py | 4 ++-- Objects/exceptions.c | 15 ++++++++++++--- Python/errors.c | 11 +++++------ 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/Include/cpython/pyerrors.h b/Include/cpython/pyerrors.h index db27956d723cd8..d0300f6ee56a25 100644 --- a/Include/cpython/pyerrors.h +++ b/Include/cpython/pyerrors.h @@ -112,8 +112,8 @@ PyAPI_FUNC(PyObject *) _PyErr_FormatFromCause( /* In exceptions.c */ -PyAPI_FUNC(PyObject*) _PyException_AddNote( - PyBaseExceptionObject *exc, +PyAPI_FUNC(int) _PyException_AddNote( + PyObject *exc, PyObject *note); /* Helper that attempts to replace the current exception with one of the diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index 47af5d9d01b8c0..81319a39b0a79a 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -174,7 +174,7 @@ class Broken(Exception): def __init__(self, *arg): raise ValueError("Broken __init__") - exc = _testcapi.exc_set_object_fetch(Broken, ('abcd')) + exc = _testcapi.exc_set_object_fetch(Broken, 'abcd') self.assertIsInstance(exc, ValueError) self.assertEqual(exc.__notes__[0], "Normalization failed: type=Broken args='abcd'") @@ -183,7 +183,7 @@ class BadArg: def __repr__(self): raise TypeError('Broken arg type') - exc = _testcapi.exc_set_object_fetch(Broken, (BadArg())) + exc = _testcapi.exc_set_object_fetch(Broken, BadArg()) self.assertIsInstance(exc, ValueError) self.assertEqual(exc.__notes__[0], 'Normalization failed: type=Broken args=') diff --git a/Objects/exceptions.c b/Objects/exceptions.c index 96acfce1cbde58..d69f7400ca6042 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -3749,10 +3749,19 @@ _PyExc_Fini(PyInterpreterState *interp) _PyExc_FiniTypes(interp); } -PyObject * -_PyException_AddNote(PyBaseExceptionObject *exc, PyObject *note) +int +_PyException_AddNote(PyObject *exc, PyObject *note) { - return BaseException_add_note((PyObject *)exc, note); + if (!PyExceptionInstance_Check(exc)) { + PyErr_Format(PyExc_TypeError, + "exc must be an exception, not '%s'", + Py_TYPE(exc)->tp_name); + return -1; + } + PyObject *r = BaseException_add_note(exc, note); + int res = r == NULL ? -1 : 0; + Py_XDECREF(r); + return res; } /* Helper to do the equivalent of "raise X from Y" in C, but always using diff --git a/Python/errors.c b/Python/errors.c index 7dbd48bd71ca00..b664ca49e3baab 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -182,28 +182,27 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) Py_XINCREF(value); if (!is_subclass) { /* We must normalize the value right now */ - PyObject *fixed_value; /* Issue #23571: functions must not be called with an exception set */ _PyErr_Clear(tstate); - fixed_value = _PyErr_CreateException(exception, value); - Py_XDECREF(value); + PyObject *fixed_value = _PyErr_CreateException(exception, value); if (fixed_value == NULL) { PyObject *exc = _PyErr_GetRaisedException(tstate); assert(PyExceptionInstance_Check(exc)); PyObject *note = get_normalization_failure_note(tstate, exception, value); + Py_XDECREF(value); if (note != NULL) { - PyObject *res = _PyException_AddNote((PyBaseExceptionObject*)exc, note); + /* ignore errors in _PyException_AddNote - they will be overwritten below */ + _PyException_AddNote(exc, note); Py_DECREF(note); - Py_XDECREF(res); } _PyErr_SetRaisedException(tstate, exc); return; } - + Py_XDECREF(value); value = fixed_value; } From 03dbc6bd670b2d4307a7756ab4dbe790b4bd19d1 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 14 Mar 2023 21:59:58 +0000 Subject: [PATCH 5/6] use Py_XSETREF --- Python/errors.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Python/errors.c b/Python/errors.c index b664ca49e3baab..bdcbac317eb9ee 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -202,8 +202,7 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) _PyErr_SetRaisedException(tstate, exc); return; } - Py_XDECREF(value); - value = fixed_value; + Py_XSETREF(value, fixed_value); } exc_value = _PyErr_GetTopmostException(tstate)->exc_value; From a9d1068c12fee3e39603f5c1688c8c64f500d8ff Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 15 Mar 2023 10:17:45 +0000 Subject: [PATCH 6/6] fix whitespace --- Lib/test/test_capi/test_exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index 81319a39b0a79a..b1c1a61e20685e 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -172,7 +172,7 @@ class Broken(Exception, metaclass=Meta): def test_set_object_and_fetch(self): class Broken(Exception): def __init__(self, *arg): - raise ValueError("Broken __init__") + raise ValueError("Broken __init__") exc = _testcapi.exc_set_object_fetch(Broken, 'abcd') self.assertIsInstance(exc, ValueError)