From c62f8ae1955a294db4e54247fe6df7cf100e65c6 Mon Sep 17 00:00:00 2001 From: Carey Metcalfe Date: Mon, 6 Nov 2023 11:24:54 -0500 Subject: [PATCH 1/2] Use NULL in the exception stack to indicate an exception was handled Previously, both `NULL` and `Py_None` would be used interchangeably to indicate that an exception is no longer being handled. By ensuring that only `NULL` is used, this opens up the possibility to use `Py_None` to indicate a cleared exception. The difference here would be that clearing would indicate that no exception is currently being handled vs. handling would indicate that the next exception in the stack is currently being handled. This functionality will be used to patch up some edge cases in how the exception context interacts with exceptions thrown into coroutines. This is implemented in this commit by changing code that could add `Py_None` to the exception stack to indicate that an exception is no longer being handled to add `NULL` instead. An assert was also added to ensure that `Py_None` is no longer added to the exception stack. See gh-111676 for context. --- .../2023-12-19-22-03-43.gh-issue-111375.M9vuA6.rst | 2 ++ Objects/frameobject.c | 2 +- Python/bytecodes.c | 2 +- Python/errors.c | 3 ++- Python/executor_cases.c.h | 2 +- Python/generated_cases.c.h | 2 +- 6 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-12-19-22-03-43.gh-issue-111375.M9vuA6.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-12-19-22-03-43.gh-issue-111375.M9vuA6.rst b/Misc/NEWS.d/next/Core and Builtins/2023-12-19-22-03-43.gh-issue-111375.M9vuA6.rst new file mode 100644 index 00000000000000..fbb517173451f8 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-12-19-22-03-43.gh-issue-111375.M9vuA6.rst @@ -0,0 +1,2 @@ +Only use ``NULL`` in the exception stack to indicate an exception was +handled. Patch by Carey Metcalfe. diff --git a/Objects/frameobject.c b/Objects/frameobject.c index be330a775872c2..cafe4ef6141d9a 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -811,7 +811,7 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore PyObject *exc = _PyFrame_StackPop(f->f_frame); assert(PyExceptionInstance_Check(exc) || exc == Py_None); PyThreadState *tstate = _PyThreadState_GET(); - Py_XSETREF(tstate->exc_info->exc_value, exc); + Py_XSETREF(tstate->exc_info->exc_value, exc == Py_None ? NULL : exc); } else { PyObject *v = _PyFrame_StackPop(f->f_frame); diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 19e2268046fcdc..962624514a84bc 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1100,7 +1100,7 @@ dummy_func( inst(POP_EXCEPT, (exc_value -- )) { _PyErr_StackItem *exc_info = tstate->exc_info; - Py_XSETREF(exc_info->exc_value, exc_value); + Py_XSETREF(exc_info->exc_value, exc_value == Py_None ? NULL : exc_value); } inst(RERAISE, (values[oparg], exc -- values[oparg])) { diff --git a/Python/errors.c b/Python/errors.c index ed5eec5c261970..85290a486c56dc 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -126,6 +126,7 @@ _PyErr_GetTopmostException(PyThreadState *tstate) { exc_info = exc_info->previous_item; } + assert(!Py_IsNone(exc_info->exc_value)); return exc_info; } @@ -592,7 +593,7 @@ PyErr_GetHandledException(void) void _PyErr_SetHandledException(PyThreadState *tstate, PyObject *exc) { - Py_XSETREF(tstate->exc_info->exc_value, Py_XNewRef(exc)); + Py_XSETREF(tstate->exc_info->exc_value, Py_XNewRef(exc == Py_None ? NULL : exc)); } void diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 7cc29c8e644d8d..69adb20ed46595 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -806,7 +806,7 @@ PyObject *exc_value; exc_value = stack_pointer[-1]; _PyErr_StackItem *exc_info = tstate->exc_info; - Py_XSETREF(exc_info->exc_value, exc_value); + Py_XSETREF(exc_info->exc_value, exc_value == Py_None ? NULL : exc_value); stack_pointer += -1; break; } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index a274427a699a43..73b784577ce9ac 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -4554,7 +4554,7 @@ PyObject *exc_value; exc_value = stack_pointer[-1]; _PyErr_StackItem *exc_info = tstate->exc_info; - Py_XSETREF(exc_info->exc_value, exc_value); + Py_XSETREF(exc_info->exc_value, exc_value == Py_None ? NULL : exc_value); stack_pointer += -1; DISPATCH(); } From 4ac01c0b8c999b58b799d8ccfc659eb196bae2e3 Mon Sep 17 00:00:00 2001 From: Carey Metcalfe Date: Wed, 20 Dec 2023 08:08:55 -0500 Subject: [PATCH 2/2] Don't skip over Py_None when getting the topmost exception --- Python/errors.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Python/errors.c b/Python/errors.c index 85290a486c56dc..e5f176a5dd208e 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -121,8 +121,7 @@ _PyErr_GetTopmostException(PyThreadState *tstate) _PyErr_StackItem *exc_info = tstate->exc_info; assert(exc_info); - while ((exc_info->exc_value == NULL || exc_info->exc_value == Py_None) && - exc_info->previous_item != NULL) + while (exc_info->exc_value == NULL && exc_info->previous_item != NULL) { exc_info = exc_info->previous_item; }