bpo-45711: use exc_value instead of exc_type to determine if exc_info is valid. Add more assertions. (GH-29627)

Author: iritkatriel

Include/internal/pycore_pyerrors.h

@@ -28,6 +28,8 @@ static inline void _PyErr_ClearExcState(_PyErr_StackItem *exc_state)
     Py_XDECREF(tb);
 }
 
+PyAPI_FUNC(PyObject*) _PyErr_StackItemToExcInfoTuple(
+    _PyErr_StackItem *err_info);
 
 PyAPI_FUNC(void) _PyErr_Fetch(
     PyThreadState *tstate,

Python/ceval.c

@@ -1102,7 +1102,7 @@ static void
 _assert_exception_type_is_redundant(PyObject* type, PyObject* val)
 {
     if (type == NULL || type == Py_None) {
-        assert(val == NULL || val == Py_None);
+        assert(val == type);
     }
     else {
         assert(PyExceptionInstance_Check(val));
@@ -3738,7 +3738,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
 
         TARGET(JUMP_IF_NOT_EXC_MATCH) {
             PyObject *right = POP();
-            PyObject *left = TOP();
+            ASSERT_EXC_TYPE_IS_REDUNDANT(TOP(), SECOND());
+            PyObject *left = SECOND();
+            assert(PyExceptionInstance_Check(left));
             if (check_except_type_valid(tstate, right) < 0) {
                  Py_DECREF(right);
                  goto error;
@@ -4198,7 +4200,13 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
             ASSERT_EXC_TYPE_IS_REDUNDANT(type, value);
             _PyErr_StackItem *exc_info = tstate->exc_info;
             SET_THIRD(exc_info->exc_traceback);
-            SET_SECOND(exc_info->exc_value);
+            if (exc_info->exc_value != NULL) {
+                SET_SECOND(exc_info->exc_value);
+            }
+            else {
+                Py_INCREF(Py_None);
+                SET_SECOND(Py_None);
+            }
             if (exc_info->exc_type != NULL) {
                 SET_TOP(exc_info->exc_type);
             }
@@ -5916,7 +5924,9 @@ do_raise(PyThreadState *tstate, PyObject *exc, PyObject *cause)
         type = exc_info->exc_type;
         value = exc_info->exc_value;
         tb = exc_info->exc_traceback;
-        if (Py_IsNone(type) || type == NULL) {
+        assert(((Py_IsNone(value) || value == NULL)) ==
+               ((Py_IsNone(type) || type == NULL)));
+        if (Py_IsNone(value) || value == NULL) {
             _PyErr_SetString(tstate, PyExc_RuntimeError,
                              "No active exception to reraise");
             return 0;

Python/errors.c

@@ -79,11 +79,16 @@ _PyErr_StackItem *
 _PyErr_GetTopmostException(PyThreadState *tstate)
 {
     _PyErr_StackItem *exc_info = tstate->exc_info;
-    while ((exc_info->exc_type == NULL || exc_info->exc_type == Py_None) &&
+    assert(exc_info);
+
+    while ((exc_info->exc_value == NULL || exc_info->exc_value == Py_None) &&
            exc_info->previous_item != NULL)
     {
+        assert(exc_info->exc_type == NULL || exc_info->exc_type == Py_None);
         exc_info = exc_info->previous_item;
     }
+    assert(exc_info->previous_item == NULL ||
+           (exc_info->exc_type != NULL && exc_info->exc_type != Py_None));
     return exc_info;
 }
 
@@ -471,10 +476,20 @@ _PyErr_GetExcInfo(PyThreadState *tstate,
                   PyObject **p_type, PyObject **p_value, PyObject **p_traceback)
 {
     _PyErr_StackItem *exc_info = _PyErr_GetTopmostException(tstate);
-    *p_type = exc_info->exc_type;
+
     *p_value = exc_info->exc_value;
     *p_traceback = exc_info->exc_traceback;
 
+    if (*p_value == NULL || *p_value == Py_None) {
+        assert(exc_info->exc_type == NULL || exc_info->exc_type == Py_None);
+        *p_type = Py_None;
+    }
+    else {
+        assert(PyExceptionInstance_Check(*p_value));
+        assert(exc_info->exc_type == PyExceptionInstance_Class(*p_value));
+        *p_type = PyExceptionInstance_Class(*p_value);
+    }
+
     Py_XINCREF(*p_type);
     Py_XINCREF(*p_value);
     Py_XINCREF(*p_traceback);
@@ -507,42 +522,66 @@ PyErr_SetExcInfo(PyObject *p_type, PyObject *p_value, PyObject *p_traceback)
     Py_XDECREF(oldtraceback);
 }
 
+
+PyObject*
+_PyErr_StackItemToExcInfoTuple(_PyErr_StackItem *err_info)
+{
+    PyObject *exc_value = err_info->exc_value;
+    if (exc_value == NULL) {
+        exc_value = Py_None;
+    }
+
+    assert(exc_value == Py_None || PyExceptionInstance_Check(exc_value));
+
+    PyObject *exc_type = PyExceptionInstance_Check(exc_value) ?
+               PyExceptionInstance_Class(exc_value) :
+               Py_None;
+
+    return Py_BuildValue(
+        "(OOO)",
+        exc_type,
+        exc_value,
+        err_info->exc_traceback != NULL ?
+            err_info->exc_traceback : Py_None);
+}
+
+
 /* Like PyErr_Restore(), but if an exception is already set,
    set the context associated with it.
 
    The caller is responsible for ensuring that this call won't create
    any cycles in the exception context chain. */
 void
-_PyErr_ChainExceptions(PyObject *exc, PyObject *val, PyObject *tb)
+_PyErr_ChainExceptions(PyObject *typ, PyObject *val, PyObject *tb)
 {
-    if (exc == NULL)
+    if (typ == NULL)
         return;
 
     PyThreadState *tstate = _PyThreadState_GET();
 
-    if (!PyExceptionClass_Check(exc)) {
+    if (!PyExceptionClass_Check(typ)) {
         _PyErr_Format(tstate, PyExc_SystemError,
                       "_PyErr_ChainExceptions: "
                       "exception %R is not a BaseException subclass",
-                      exc);
+                      typ);
         return;
     }
 
     if (_PyErr_Occurred(tstate)) {
-        PyObject *exc2, *val2, *tb2;
-        _PyErr_Fetch(tstate, &exc2, &val2, &tb2);
-        _PyErr_NormalizeException(tstate, &exc, &val, &tb);
+        PyObject *typ2, *val2, *tb2;
+        _PyErr_Fetch(tstate, &typ2, &val2, &tb2);
+        _PyErr_NormalizeException(tstate, &typ, &val, &tb);
         if (tb != NULL) {
             PyException_SetTraceback(val, tb);
             Py_DECREF(tb);
         }
-        Py_DECREF(exc);
-        _PyErr_NormalizeException(tstate, &exc2, &val2, &tb2);
+        Py_DECREF(typ);
+        _PyErr_NormalizeException(tstate, &typ2, &val2, &tb2);
         PyException_SetContext(val2, val);
-        _PyErr_Restore(tstate, exc2, val2, tb2);
+        _PyErr_Restore(tstate, typ2, val2, tb2);
     }
     else {
-        _PyErr_Restore(tstate, exc, val, tb);
+        _PyErr_Restore(tstate, typ, val, tb);
     }
 }
 
@@ -567,7 +606,11 @@ _PyErr_ChainStackItem(_PyErr_StackItem *exc_info)
     } else {
         exc_info_given = 1;
     }
-    if (exc_info->exc_type == NULL || exc_info->exc_type == Py_None) {
+
+    assert( (exc_info->exc_type == NULL || exc_info->exc_type == Py_None) ==
+            (exc_info->exc_value == NULL || exc_info->exc_value == Py_None) );
+
+    if (exc_info->exc_value == NULL || exc_info->exc_value == Py_None) {
         return;
     }
 
@@ -579,21 +622,32 @@ _PyErr_ChainStackItem(_PyErr_StackItem *exc_info)
         tstate->exc_info = exc_info;
     }
 
-    PyObject *exc, *val, *tb;
-    _PyErr_Fetch(tstate, &exc, &val, &tb);
+    PyObject *typ, *val, *tb;
+    _PyErr_Fetch(tstate, &typ, &val, &tb);
 
-    PyObject *exc2, *val2, *tb2;
-    exc2 = exc_info->exc_type;
+    PyObject *typ2, *val2, *tb2;
+    typ2 = exc_info->exc_type;
     val2 = exc_info->exc_value;
     tb2 = exc_info->exc_traceback;
-    _PyErr_NormalizeException(tstate, &exc2, &val2, &tb2);
+#ifdef Py_DEBUG
+    PyObject *typ2_before = typ2;
+    PyObject *val2_before = val2;
+    PyObject *tb2_before = tb2;
+#endif
+    _PyErr_NormalizeException(tstate, &typ2, &val2, &tb2);
+#ifdef Py_DEBUG
+    /* exc_info should already be normalized */
+    assert(typ2 == typ2_before);
+    assert(val2 == val2_before);
+    assert(tb2 == tb2_before);
+#endif
     if (tb2 != NULL) {
         PyException_SetTraceback(val2, tb2);
     }
 
     /* _PyErr_SetObject sets the context from PyThreadState. */
-    _PyErr_SetObject(tstate, exc, val);
-    Py_DECREF(exc);  // since _PyErr_Occurred was true
+    _PyErr_SetObject(tstate, typ, val);
+    Py_DECREF(typ);  // since _PyErr_Occurred was true
     Py_XDECREF(val);
     Py_XDECREF(tb);
 

Python/pystate.c

@@ -1341,11 +1341,7 @@ _PyThread_CurrentExceptions(void)
             if (id == NULL) {
                 goto fail;
             }
-            PyObject *exc_info = PyTuple_Pack(
-                3,
-                err_info->exc_type != NULL ? err_info->exc_type : Py_None,
-                err_info->exc_value != NULL ? err_info->exc_value : Py_None,
-                err_info->exc_traceback != NULL ? err_info->exc_traceback : Py_None);
+            PyObject *exc_info = _PyErr_StackItemToExcInfoTuple(err_info);
             if (exc_info == NULL) {
                 Py_DECREF(id);
                 goto fail;

Python/sysmodule.c

@@ -785,12 +785,7 @@ sys_exc_info_impl(PyObject *module)
 /*[clinic end generated code: output=3afd0940cf3a4d30 input=b5c5bf077788a3e5]*/
 {
     _PyErr_StackItem *err_info = _PyErr_GetTopmostException(_PyThreadState_GET());
-    return Py_BuildValue(
-        "(OOO)",
-        err_info->exc_type != NULL ? err_info->exc_type : Py_None,
-        err_info->exc_value != NULL ? err_info->exc_value : Py_None,
-        err_info->exc_traceback != NULL ?
-            err_info->exc_traceback : Py_None);
+    return _PyErr_StackItemToExcInfoTuple(err_info);
 }