From b0be6b3b94fbdf31b796adc19dc86a04a52b03e1 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 5 May 2020 17:07:41 +0200 Subject: [PATCH] bpo-29587: _PyErr_ChainExceptions() checks exception (GH-19902) _PyErr_ChainExceptions() now ensures that the first parameter is an exception type, as done by _PyErr_SetObject(). * The following function now check PyExceptionInstance_Check() in an assertion using a new _PyBaseExceptionObject_cast() helper function: * PyException_GetTraceback(), PyException_SetTraceback() * PyException_GetCause(), PyException_SetCause() * PyException_GetContext(), PyException_SetContext() * PyExceptionClass_Name() now checks PyExceptionClass_Check() with an assertion. * Remove XXX comment and add gi_exc_state variable to _gen_throw(). * Remove comment from test_generators --- Lib/test/test_generators.py | 3 --- Objects/exceptions.c | 36 +++++++++++++++++++++++++----------- Objects/genobject.c | 18 +++++++++--------- Python/errors.c | 12 +++++++++++- 4 files changed, 45 insertions(+), 24 deletions(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 5824ecd7c37..e0478011996 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -342,9 +342,6 @@ class GeneratorThrowTest(unittest.TestCase): try: yield except Exception: - # Without the `gi_exc_state.exc_type != Py_None` in - # _gen_throw(), this line was causing a crash ("Segmentation - # fault (core dumped)") on e.g. Fedora 32. raise RuntimeError gen = g() diff --git a/Objects/exceptions.c b/Objects/exceptions.c index ca917b436c4..db5e3da12b0 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -304,22 +304,33 @@ static PyGetSetDef BaseException_getset[] = { }; +static inline PyBaseExceptionObject* +_PyBaseExceptionObject_cast(PyObject *exc) +{ + assert(PyExceptionInstance_Check(exc)); + return (PyBaseExceptionObject *)exc; +} + + PyObject * -PyException_GetTraceback(PyObject *self) { - PyBaseExceptionObject *base_self = (PyBaseExceptionObject *)self; +PyException_GetTraceback(PyObject *self) +{ + PyBaseExceptionObject *base_self = _PyBaseExceptionObject_cast(self); Py_XINCREF(base_self->traceback); return base_self->traceback; } int -PyException_SetTraceback(PyObject *self, PyObject *tb) { - return BaseException_set_tb((PyBaseExceptionObject *)self, tb, NULL); +PyException_SetTraceback(PyObject *self, PyObject *tb) +{ + return BaseException_set_tb(_PyBaseExceptionObject_cast(self), tb, NULL); } PyObject * -PyException_GetCause(PyObject *self) { - PyObject *cause = ((PyBaseExceptionObject *)self)->cause; +PyException_GetCause(PyObject *self) +{ + PyObject *cause = _PyBaseExceptionObject_cast(self)->cause; Py_XINCREF(cause); return cause; } @@ -328,13 +339,15 @@ PyException_GetCause(PyObject *self) { void PyException_SetCause(PyObject *self, PyObject *cause) { - ((PyBaseExceptionObject *)self)->suppress_context = 1; - Py_XSETREF(((PyBaseExceptionObject *)self)->cause, cause); + PyBaseExceptionObject *base_self = _PyBaseExceptionObject_cast(self); + base_self->suppress_context = 1; + Py_XSETREF(base_self->cause, cause); } PyObject * -PyException_GetContext(PyObject *self) { - PyObject *context = ((PyBaseExceptionObject *)self)->context; +PyException_GetContext(PyObject *self) +{ + PyObject *context = _PyBaseExceptionObject_cast(self)->context; Py_XINCREF(context); return context; } @@ -343,7 +356,7 @@ PyException_GetContext(PyObject *self) { void PyException_SetContext(PyObject *self, PyObject *context) { - Py_XSETREF(((PyBaseExceptionObject *)self)->context, context); + Py_XSETREF(_PyBaseExceptionObject_cast(self)->context, context); } #undef PyExceptionClass_Name @@ -351,6 +364,7 @@ PyException_SetContext(PyObject *self, PyObject *context) const char * PyExceptionClass_Name(PyObject *ob) { + assert(PyExceptionClass_Check(ob)); return ((PyTypeObject*)ob)->tp_name; } diff --git a/Objects/genobject.c b/Objects/genobject.c index b27fa929a26..5b253edfdcd 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -512,15 +512,15 @@ throw_here: } PyErr_Restore(typ, val, tb); - /* XXX It seems like we shouldn't have to check not equal to Py_None - here because exc_type should only ever be a class. But not including - this check was causing crashes on certain tests e.g. on Fedora. */ - if (gen->gi_exc_state.exc_type && gen->gi_exc_state.exc_type != Py_None) { - Py_INCREF(gen->gi_exc_state.exc_type); - Py_XINCREF(gen->gi_exc_state.exc_value); - Py_XINCREF(gen->gi_exc_state.exc_traceback); - _PyErr_ChainExceptions(gen->gi_exc_state.exc_type, - gen->gi_exc_state.exc_value, gen->gi_exc_state.exc_traceback); + + _PyErr_StackItem *gi_exc_state = &gen->gi_exc_state; + if (gi_exc_state->exc_type != NULL && gi_exc_state->exc_type != Py_None) { + Py_INCREF(gi_exc_state->exc_type); + Py_XINCREF(gi_exc_state->exc_value); + Py_XINCREF(gi_exc_state->exc_traceback); + _PyErr_ChainExceptions(gi_exc_state->exc_type, + gi_exc_state->exc_value, + gi_exc_state->exc_traceback); } return gen_send_ex(gen, Py_None, 1, 0); diff --git a/Python/errors.c b/Python/errors.c index 9e53d050416..f856a798eed 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -107,7 +107,8 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) if (exception != NULL && !PyExceptionClass_Check(exception)) { _PyErr_Format(tstate, PyExc_SystemError, - "exception %R not a BaseException subclass", + "_PyErr_SetObject: " + "exception %R is not a BaseException subclass", exception); return; } @@ -484,6 +485,15 @@ _PyErr_ChainExceptions(PyObject *exc, PyObject *val, PyObject *tb) return; PyThreadState *tstate = _PyThreadState_GET(); + + if (!PyExceptionClass_Check(exc)) { + _PyErr_Format(tstate, PyExc_SystemError, + "_PyErr_ChainExceptions: " + "exception %R is not a BaseException subclass", + exc); + return; + } + if (_PyErr_Occurred(tstate)) { PyObject *exc2, *val2, *tb2; _PyErr_Fetch(tstate, &exc2, &val2, &tb2);