Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 25, 2021
2 changes: 2 additions & 0 deletions Include/internal/pycore_pyerrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 14 additions & 4 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -1101,7 +1101,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));
Expand Down Expand Up @@ -3717,7 +3717,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;
Expand Down Expand Up @@ -4177,7 +4179,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);
}
Expand Down Expand Up @@ -5896,7 +5904,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;
Expand Down
96 changes: 75 additions & 21 deletions Python/errors.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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;
}

Expand All @@ -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);

Expand Down
6 changes: 1 addition & 5 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 1 addition & 6 deletions Python/sysmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand Down