[Python-checkins] bpo-29587: _PyErr_ChainExceptions() checks exception (GH-19902)

[Victor Stinner]

https://github.com/python/cpython/commit/b0be6b3b94fbdf31b796adc19dc86a04a52b03e1
commit: b0be6b3b94fbdf31b796adc19dc86a04a52b03e1
branch: master
author: Victor Stinner <vstinner at python.org>
committer: GitHub <noreply at github.com>
date: 2020-05-05T17:07:41+02:00
summary:

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

files:
M Lib/test/test_generators.py
M Objects/exceptions.c
M Objects/genobject.c
M Python/errors.c

diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py
index 5824ecd7c37e8..e047801199680 100644
--- a/Lib/test/test_generators.py
+++ b/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()
diff --git a/Objects/exceptions.c b/Objects/exceptions.c
index ca917b436c4bb..db5e3da12b00f 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 b27fa929a2625..5b253edfdcd0f 100644
--- a/Objects/genobject.c
+++ b/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);
 
diff --git a/Python/errors.c b/Python/errors.c
index 9e53d050416ff..f856a798eed1e 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);