Skip to content
Merged
Prev Previous commit
Next Next commit
Remove redundant type and traceback fields for the current exception.
  • Loading branch information
markshannon committed Feb 6, 2023
commit 14f21ff5f85f22def0b3a166457b8718cededd0b
4 changes: 1 addition & 3 deletions Include/cpython/pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,9 +166,7 @@ struct _ts {
PyObject *c_traceobj;

/* The exception currently being raised */
PyObject *curexc_type;
PyObject *curexc_value;
PyObject *curexc_traceback;
PyObject *current_exception;

/* Pointer to the top of the exception stack for the exceptions
* we may be currently handling. (See _PyErr_StackItem above.)
Expand Down
5 changes: 4 additions & 1 deletion Include/internal/pycore_pyerrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ extern void _PyErr_FiniTypes(PyInterpreterState *);
static inline PyObject* _PyErr_Occurred(PyThreadState *tstate)
{
assert(tstate != NULL);
return tstate->curexc_type;
if (tstate->current_exception == NULL) {
return NULL;
}
return (PyObject *)Py_TYPE(tstate->current_exception);
}

static inline void _PyErr_ClearExcState(_PyErr_StackItem *exc_state)
Expand Down
12 changes: 6 additions & 6 deletions Objects/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -2416,10 +2416,10 @@ _Py_Dealloc(PyObject *op)
destructor dealloc = type->tp_dealloc;
#ifdef Py_DEBUG
PyThreadState *tstate = _PyThreadState_GET();
PyObject *old_exc_type = tstate != NULL ? tstate->curexc_type : NULL;
PyObject *old_exc = tstate != NULL ? tstate->current_exception : NULL;
// Keep the old exception type alive to prevent undefined behavior
// on (tstate->curexc_type != old_exc_type) below
Py_XINCREF(old_exc_type);
Py_XINCREF(old_exc);
// Make sure that type->tp_name remains valid
Py_INCREF(type);
#endif
Expand All @@ -2432,12 +2432,12 @@ _Py_Dealloc(PyObject *op)
#ifdef Py_DEBUG
// gh-89373: The tp_dealloc function must leave the current exception
// unchanged.
if (tstate != NULL && tstate->curexc_type != old_exc_type) {
if (tstate != NULL && tstate->current_exception != old_exc) {
const char *err;
if (old_exc_type == NULL) {
if (old_exc == NULL) {
err = "Deallocator of type '%s' raised an exception";
}
else if (tstate->curexc_type == NULL) {
else if (tstate->current_exception == NULL) {
err = "Deallocator of type '%s' cleared the current exception";
}
else {
Expand All @@ -2448,7 +2448,7 @@ _Py_Dealloc(PyObject *op)
}
_Py_FatalErrorFormat(__func__, err, type->tp_name);
}
Py_XDECREF(old_exc_type);
Py_XDECREF(old_exc);
Py_DECREF(type);
#endif
}
Expand Down
15 changes: 6 additions & 9 deletions Parser/pegen.c
Original file line number Diff line number Diff line change
Expand Up @@ -643,13 +643,10 @@ _PyPegen_number_token(Parser *p)
PyThreadState *tstate = _PyThreadState_GET();
// The only way a ValueError should happen in _this_ code is via
// PyLong_FromString hitting a length limit.
if (tstate->curexc_type == PyExc_ValueError &&
tstate->curexc_value != NULL) {
PyObject *type, *value, *tb;
// This acts as PyErr_Clear() as we're replacing curexc.
PyErr_Fetch(&type, &value, &tb);
Py_XDECREF(tb);
Py_DECREF(type);
if (tstate->current_exception != NULL &&
Py_TYPE(tstate->current_exception) == (PyTypeObject *)PyExc_ValueError
) {
PyObject *exc = PyErr_Fetch1();
/* Intentionally omitting columns to avoid a wall of 1000s of '^'s
* on the error message. Nobody is going to overlook their huge
* numeric literal once given the line. */
Expand All @@ -659,8 +656,8 @@ _PyPegen_number_token(Parser *p)
t->end_lineno, -1 /* end_col_offset */,
"%S - Consider hexadecimal for huge integer literals "
"to avoid decimal conversion limits.",
value);
Py_DECREF(value);
exc);
Py_DECREF(exc);
}
return NULL;
}
Expand Down
84 changes: 30 additions & 54 deletions Python/errors.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,69 +28,44 @@ _PyErr_FormatV(PyThreadState *tstate, PyObject *exception,
const char *format, va_list vargs);

void
ASSERT_EXCEPTIONS_NORMALIZED(PyThreadState *tstate)
ASSERT_EXCEPTION_NORMALIZED(PyObject *type, PyObject *value,
PyObject *traceback)
{

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's currently a requirement to pass in only normalised exceptions. This could break C extensions.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add some extra logic. It will slow things down a bit, but safety first.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you could make _PyErr_Restore to expectonly normalised exceptions, but normalise them in PyErr_Restore?

/* Exceptions are normalized if all NULL,
* or if curexc_type = Py_TYPE(curexc_value) and
* curexc_traceback = curexc_value->traceback
* and both type and traceback are valid */
if (tstate->curexc_value == NULL) {
assert(tstate->curexc_type == NULL);
assert(tstate->curexc_traceback == NULL);
if (value == NULL) {
assert(type == NULL);
assert(traceback == NULL);
}
else {
assert(((PyBaseExceptionObject *)tstate->curexc_value)->traceback != Py_None);
assert(tstate->curexc_traceback != Py_None);
assert(PyExceptionClass_Check(tstate->curexc_type));
assert(tstate->curexc_type == (PyObject *)Py_TYPE(tstate->curexc_value));
assert(
tstate->curexc_traceback ==
((PyBaseExceptionObject *)tstate->curexc_value)->traceback
assert(PyExceptionClass_Check(type));
assert(type == (PyObject *)Py_TYPE(value));
assert(((PyBaseExceptionObject *)value)->traceback != Py_None);
assert(traceback == ((PyBaseExceptionObject *)value)->traceback ||
(traceback == Py_None &&
((PyBaseExceptionObject *)value)->traceback == NULL)
);
}
}

void
_PyErr_Restore1(PyThreadState *tstate, PyObject *exc)
{
if (exc == NULL) {
_PyErr_Restore(tstate, NULL, NULL, NULL);
}
else {
_PyErr_Restore(tstate,
Py_NewRef(Py_TYPE(exc)),
exc,
Py_XNewRef(((PyBaseExceptionObject *)exc)->traceback)
);
}
PyObject *old_exc = tstate->current_exception;
tstate->current_exception = exc;
Py_XDECREF(old_exc);
}

void
_PyErr_Restore(PyThreadState *tstate, PyObject *type, PyObject *value,
PyObject *traceback)
{
PyObject *oldtype, *oldvalue, *oldtraceback;

if (traceback != NULL && !PyTraceBack_Check(traceback)) {
/* XXX Should never happen -- fatal error instead? */
/* Well, it could be None. */
Py_SETREF(traceback, NULL);
}

/* Save these in locals to safeguard against recursive
invocation through Py_XDECREF */
oldtype = tstate->curexc_type;
oldvalue = tstate->curexc_value;
oldtraceback = tstate->curexc_traceback;

tstate->curexc_type = type;
tstate->curexc_value = value;
tstate->curexc_traceback = traceback;

Py_XDECREF(oldtype);
Py_XDECREF(oldvalue);
Py_XDECREF(oldtraceback);
ASSERT_EXCEPTIONS_NORMALIZED(tstate);
ASSERT_EXCEPTION_NORMALIZED(type, value, traceback);
_PyErr_Restore1(tstate, value);
Py_XDECREF(type);
Py_XDECREF(traceback);
}

void
Expand Down Expand Up @@ -453,10 +428,8 @@ PyErr_NormalizeException(PyObject **exc, PyObject **val, PyObject **tb)

PyObject *
_PyErr_Fetch1(PyThreadState *tstate) {
PyObject *exc = tstate->curexc_value;
tstate->curexc_value = NULL;
Py_CLEAR(tstate->curexc_type);
Py_CLEAR(tstate->curexc_traceback);
PyObject *exc = tstate->current_exception;
tstate->current_exception = NULL;
return exc;
}

Expand All @@ -471,13 +444,16 @@ void
_PyErr_Fetch(PyThreadState *tstate, PyObject **p_type, PyObject **p_value,
PyObject **p_traceback)
{
*p_type = tstate->curexc_type;
*p_value = tstate->curexc_value;
*p_traceback = tstate->curexc_traceback;

tstate->curexc_type = NULL;
tstate->curexc_value = NULL;
tstate->curexc_traceback = NULL;
PyObject *exc = _PyErr_Fetch1(tstate);
*p_value = exc;
if (exc == NULL) {
*p_type = NULL;
*p_traceback = NULL;
}
else {
*p_type = Py_NewRef(Py_TYPE(exc));
*p_traceback = Py_XNewRef(((PyBaseExceptionObject *)exc)->traceback);
}
}


Expand Down
4 changes: 1 addition & 3 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -1379,9 +1379,7 @@ PyThreadState_Clear(PyThreadState *tstate)
Py_CLEAR(tstate->dict);
Py_CLEAR(tstate->async_exc);

Py_CLEAR(tstate->curexc_type);
Py_CLEAR(tstate->curexc_value);
Py_CLEAR(tstate->curexc_traceback);
Py_CLEAR(tstate->current_exception);

Py_CLEAR(tstate->exc_state.exc_value);

Expand Down