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

Author: vstinner

Lib/test/test_generators.py

@@ -342,9 +342,6 @@ def g():
  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()

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,14 +356,15 @@ 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
 
 const char *
 PyExceptionClass_Name(PyObject *ob)
 {
+ assert(PyExceptionClass_Check(ob));
  return ((PyTypeObject*)ob)->tp_name;
 }
 

Objects/genobject.c

@@ -512,15 +512,15 @@ _gen_throw(PyGenObject *gen, int close_on_genexit,
  }
 
  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);
 

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);