From 077747d86858ecf548d15177b5c63290e489dda3 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 7 Mar 2023 16:38:29 +0000 Subject: [PATCH 1/8] add test for PyErr_SetObject --- Lib/test/test_capi/test_exceptions.py | 19 +++++++++++++++++++ Modules/_testcapi/exceptions.c | 15 +++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index b543a1a565a56f..c9a51896537847 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -140,6 +140,25 @@ def test_err_restore(self): self.assertEqual(1, v.args[0]) self.assertIs(tb, v.__traceback__.tb_next) + def test_set_object(self): + + # new exception as obj is not an exception + with self.assertRaises(ValueError) as e: + _testcapi.exc_set_object(ValueError, 42) + self.assertEqual(e.exception.args, (42,)) + + # wraps the exception because unrelated types + with self.assertRaises(ValueError) as e: + _testcapi.exc_set_object(ValueError, TypeError(1,2,3)) + wrapped = e.exception.args[0] + self.assertIsInstance(wrapped, TypeError) + self.assertEqual(wrapped.args, (1, 2, 3)) + + # is superclass, so does not wrap + with self.assertRaises(PermissionError) as e: + _testcapi.exc_set_object(OSError, PermissionError(24)) + self.assertEqual(e.exception.args, (24,)) + if __name__ == "__main__": unittest.main() diff --git a/Modules/_testcapi/exceptions.c b/Modules/_testcapi/exceptions.c index 43b88ccf261d98..a0575213987ffc 100644 --- a/Modules/_testcapi/exceptions.c +++ b/Modules/_testcapi/exceptions.c @@ -78,6 +78,20 @@ make_exception_with_doc(PyObject *self, PyObject *args, PyObject *kwargs) return PyErr_NewExceptionWithDoc(name, doc, base, dict); } +static PyObject * +exc_set_object(PyObject *self, PyObject *args) +{ + PyObject *exc; + PyObject *obj; + + if (!PyArg_ParseTuple(args, "OO:exc_set_object", &exc, &obj)) { + return NULL; + } + + PyErr_SetObject(exc, obj); + return NULL; +} + static PyObject * raise_exception(PyObject *self, PyObject *args) { @@ -247,6 +261,7 @@ static PyMethodDef test_methods[] = { PyDoc_STR("fatal_error(message, release_gil=False): call Py_FatalError(message)")}, {"make_exception_with_doc", _PyCFunction_CAST(make_exception_with_doc), METH_VARARGS | METH_KEYWORDS}, + {"exc_set_object", exc_set_object, METH_VARARGS}, {"raise_exception", raise_exception, METH_VARARGS}, {"raise_memoryerror", raise_memoryerror, METH_NOARGS}, {"set_exc_info", test_set_exc_info, METH_VARARGS}, From feef425173732849be82809e8d5f62eda74e9919 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 7 Mar 2023 16:40:36 +0000 Subject: [PATCH 2/8] fix normalization in PyErr_SetObject --- Python/errors.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Python/errors.c b/Python/errors.c index f573bed3d63ef0..4039e0b09b2a88 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -151,7 +151,14 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) } Py_XINCREF(value); /* Normalize the exception */ - if (value == NULL || (PyObject *)Py_TYPE(value) != exception) { + int is_subclass = 0; + if (value != NULL) { + is_subclass = PyObject_IsSubclass((PyObject*)Py_TYPE(value), exception); + if (is_subclass < 0) { + return; + } + } + if (value == NULL || !is_subclass) { /* We must normalize the value right now */ PyObject *fixed_value; @@ -208,7 +215,7 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) } if (value != NULL && PyExceptionInstance_Check(value)) tb = PyException_GetTraceback(value); - _PyErr_Restore(tstate, Py_XNewRef(exception), value, tb); + _PyErr_Restore(tstate, Py_NewRef(Py_TYPE(value)), value, tb); } void From c211d860b0f6dd8912ff69cf99f53d7b62427be6 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 7 Mar 2023 16:56:40 +0000 Subject: [PATCH 3/8] add news --- .../2023-03-07-16-56-28.gh-issue-102502.gTXrcD.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-03-07-16-56-28.gh-issue-102502.gTXrcD.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-03-07-16-56-28.gh-issue-102502.gTXrcD.rst b/Misc/NEWS.d/next/Core and Builtins/2023-03-07-16-56-28.gh-issue-102502.gTXrcD.rst new file mode 100644 index 00000000000000..4c4e88ca4e7c3c --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-03-07-16-56-28.gh-issue-102502.gTXrcD.rst @@ -0,0 +1 @@ +Fix regression in semantics of normalisation in ``PyErr_SetObject``. From cea7ff94cb42d3859780d70f0bfeddc4bd193e6f Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 7 Mar 2023 18:11:11 +0000 Subject: [PATCH 4/8] rename news file --- ....gTXrcD.rst => 2023-03-07-16-56-28.gh-issue-102493.gTXrcD.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Misc/NEWS.d/next/Core and Builtins/{2023-03-07-16-56-28.gh-issue-102502.gTXrcD.rst => 2023-03-07-16-56-28.gh-issue-102493.gTXrcD.rst} (100%) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-03-07-16-56-28.gh-issue-102502.gTXrcD.rst b/Misc/NEWS.d/next/Core and Builtins/2023-03-07-16-56-28.gh-issue-102493.gTXrcD.rst similarity index 100% rename from Misc/NEWS.d/next/Core and Builtins/2023-03-07-16-56-28.gh-issue-102502.gTXrcD.rst rename to Misc/NEWS.d/next/Core and Builtins/2023-03-07-16-56-28.gh-issue-102493.gTXrcD.rst From 61c661558e13b0505c67e71d2b5889cd77a69566 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 7 Mar 2023 18:56:44 +0000 Subject: [PATCH 5/8] assert not null --- Python/errors.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/errors.c b/Python/errors.c index 4039e0b09b2a88..b5ca63bdb5a584 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -213,7 +213,8 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) Py_DECREF(exc_value); } } - if (value != NULL && PyExceptionInstance_Check(value)) + assert(value != NULL); + if (PyExceptionInstance_Check(value)) tb = PyException_GetTraceback(value); _PyErr_Restore(tstate, Py_NewRef(Py_TYPE(value)), value, tb); } From a9094f9e6de93d4239a69cbb86fa26559ee52515 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 7 Mar 2023 19:03:13 +0000 Subject: [PATCH 6/8] skip issubclass when not an exception --- Python/errors.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/errors.c b/Python/errors.c index b5ca63bdb5a584..f1119ba618cf15 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -152,13 +152,13 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) Py_XINCREF(value); /* Normalize the exception */ int is_subclass = 0; - if (value != NULL) { + if (value != NULL && PyExceptionInstance_Check(value)) { is_subclass = PyObject_IsSubclass((PyObject*)Py_TYPE(value), exception); if (is_subclass < 0) { return; } } - if (value == NULL || !is_subclass) { + if (!is_subclass) { /* We must normalize the value right now */ PyObject *fixed_value; From 6d7bccd0b6d219edc35c6457ad443e58d880a926 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Tue, 7 Mar 2023 19:05:02 +0000 Subject: [PATCH 7/8] whitespace Co-authored-by: Jelle Zijlstra --- Python/errors.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/errors.c b/Python/errors.c index f1119ba618cf15..0e12c05257b23a 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -153,7 +153,7 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) /* Normalize the exception */ int is_subclass = 0; if (value != NULL && PyExceptionInstance_Check(value)) { - is_subclass = PyObject_IsSubclass((PyObject*)Py_TYPE(value), exception); + is_subclass = PyObject_IsSubclass((PyObject *)Py_TYPE(value), exception); if (is_subclass < 0) { return; } From fd0181213a05ff99dc8c6e61d052a461860e7555 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 7 Mar 2023 20:46:07 +0000 Subject: [PATCH 8/8] fix refleak when is_subclass fails --- Lib/test/test_capi/test_exceptions.py | 9 +++++++++ Python/errors.c | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index c9a51896537847..55f131699a2567 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -159,6 +159,15 @@ def test_set_object(self): _testcapi.exc_set_object(OSError, PermissionError(24)) self.assertEqual(e.exception.args, (24,)) + class Meta(type): + def __subclasscheck__(cls, sub): + 1/0 + + class Broken(Exception, metaclass=Meta): + pass + + with self.assertRaises(ZeroDivisionError) as e: + _testcapi.exc_set_object(Broken, Broken()) if __name__ == "__main__": unittest.main() diff --git a/Python/errors.c b/Python/errors.c index 0e12c05257b23a..bbf6d397ce8097 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -149,7 +149,6 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) exception); return; } - Py_XINCREF(value); /* Normalize the exception */ int is_subclass = 0; if (value != NULL && PyExceptionInstance_Check(value)) { @@ -158,6 +157,7 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) return; } } + Py_XINCREF(value); if (!is_subclass) { /* We must normalize the value right now */ PyObject *fixed_value;