From 1630eaadd5954ed8330774a51c494706f319871b Mon Sep 17 00:00:00 2001 From: Siu Kwan Lam Date: Tue, 21 Jul 2020 08:13:24 -0500 Subject: [PATCH 1/4] Fixing up numba_do_raise --- numba/_helperlib.c | 217 +++++++++++++++++++++++++-------------------- 1 file changed, 119 insertions(+), 98 deletions(-) diff --git a/numba/_helperlib.c b/numba/_helperlib.c index ad698cdf5b2..2cbe01e63cd 100644 --- a/numba/_helperlib.c +++ b/numba/_helperlib.c @@ -825,86 +825,115 @@ static void traceback_add(const char *funcname, const char *filename, int lineno _PyErr_ChainExceptions(exc, val, tb); } -/* Logic for raising an arbitrary object. Adapted from CPython's ceval.c. - This *consumes* a reference count to its argument. */ -NUMBA_EXPORT_FUNC(int) -numba_do_raise(PyObject *exc_packed) -{ - PyObject *exc = NULL, *type = NULL, *value = NULL, *loc = NULL; + +static +void traceback_add_loc(PyObject *loc) { const char *function_name_str = NULL, *filename_str = NULL; PyObject *function_name = NULL, *filename = NULL, *lineno = NULL; Py_ssize_t pos; - /* We support the following forms of raise: - raise - raise - raise */ + /* instance is instantiated/internal exception is raised, if loc is present + * add a frame for it into the traceback */ + if(loc && loc != Py_None && PyTuple_Check(loc)) + { + pos = 0; + function_name = PyTuple_GET_ITEM(loc, pos); + function_name_str = PyString_AsString(function_name); + pos = 1; + filename = PyTuple_GET_ITEM(loc, pos); + filename_str = PyString_AsString(filename); + pos = 2; + lineno = PyTuple_GET_ITEM(loc, pos); + traceback_add(function_name_str, filename_str, \ + (int)PyLong_AsLong(lineno)); + } +} - /* could be a tuple from npm (some exc like thing, args, location) */ - if (PyTuple_CheckExact(exc_packed)) { - /* Unpack a (class/inst/tuple, arguments, location) tuple. */ - if (!PyArg_ParseTuple(exc_packed, "OOO", &exc, &value, &loc)) { - Py_DECREF(exc_packed); - goto raise_error_w_loc; - } +static +int process_packed_exc(PyObject *exc_packed) { + PyObject *exc = NULL, *type = NULL, *value = NULL, *loc = NULL; - if (exc == Py_None) { - /* Reraise */ - PyThreadState *tstate = PyThreadState_GET(); - PyObject *tb; + /* Unpack a (class/inst/tuple, arguments, location) tuple. */ + if (!PyArg_ParseTuple(exc_packed, "OOO", &exc, &value, &loc)) { + traceback_add_loc(loc); + return 0; + } + + if (exc == Py_None) { + /* Reraise */ + PyThreadState *tstate = PyThreadState_GET(); + PyObject *tb; #if (PY_MAJOR_VERSION >= 3) && (PY_MINOR_VERSION >= 7) - _PyErr_StackItem *tstate_exc = tstate->exc_info; + _PyErr_StackItem *tstate_exc = tstate->exc_info; #else - PyThreadState *tstate_exc = tstate; + PyThreadState *tstate_exc = tstate; #endif - Py_DECREF(exc_packed); - type = tstate_exc->exc_type; - value = tstate_exc->exc_value; - tb = tstate_exc->exc_traceback; - if (type == Py_None) { - PyErr_SetString(PyExc_RuntimeError, - "No active exception to reraise"); - return 0; - } - Py_XINCREF(type); - Py_XINCREF(value); - Py_XINCREF(tb); - PyErr_Restore(type, value, tb); - return 1; + type = tstate_exc->exc_type; + value = tstate_exc->exc_value; + tb = tstate_exc->exc_traceback; + if (type == Py_None) { + PyErr_SetString(PyExc_RuntimeError, + "No active exception to reraise"); + return 0; } - - /* the unpacked exc should be a class, value and loc are set from above - */ + Py_XINCREF(type); Py_XINCREF(value); - Py_XINCREF(loc); - if (PyExceptionClass_Check(exc)) { - /* It is a class, type used here just as a tmp var */ - type = PyObject_CallObject(exc, value); - if (type == NULL) - goto raise_error_w_loc; - if (!PyExceptionInstance_Check(type)) { - PyErr_SetString(PyExc_TypeError, - "exceptions must derive from BaseException"); - goto raise_error_w_loc; - } - /* all ok, set type to the exc */ - Py_DECREF(type); - type = exc; - } else { - /* this should be unreachable as typing should catch it */ - /* Not something you can raise. You get an exception - anyway, just not what you specified :-) */ - Py_DECREF(exc_packed); + Py_XINCREF(tb); + PyErr_Restore(type, value, tb); + return 1; + } + + /* the unpacked exc should be a class, value and loc are set from above + */ + if (PyExceptionClass_Check(exc)) { + /* It is a class, type used here just as a tmp var */ + type = PyObject_CallObject(exc, value); + if (type == NULL){ + traceback_add_loc(loc); + return 0; + } + if (!PyExceptionInstance_Check(type)) { PyErr_SetString(PyExc_TypeError, "exceptions must derive from BaseException"); - goto raise_error_w_loc; + traceback_add_loc(loc); + Py_DECREF(type); + return 0; } + /* all ok, set type to the exc */ + Py_DECREF(type); + type = exc; + PyErr_SetObject(type, value); + traceback_add_loc(loc); + return 1; + } else { + /* this should be unreachable as typing should catch it */ + /* Not something you can raise. You get an exception + anyway, just not what you specified :-) */ + PyErr_SetString(PyExc_TypeError, + "exceptions must derive from BaseException"); + traceback_add_loc(loc); + return 0; + } +} + +/* Logic for raising an arbitrary object. Adapted from CPython's ceval.c. + This *consumes* a reference count to its argument. */ +NUMBA_EXPORT_FUNC(int) +numba_do_raise(PyObject *exc_packed) +{ + PyObject *exc = NULL, *type = NULL, *value = NULL; + int status; - /* as this branch is exited: - * - type should be an exception class - * - value should be the args for the exception class instantiation - * - loc should be the location information (or None) - */ + /* We support the following forms of raise: + raise + raise + raise */ + + /* could be a tuple from npm (some exc like thing, args, location) */ + if (PyTuple_CheckExact(exc_packed)) { + status = process_packed_exc(exc_packed); + Py_DECREF(exc_packed); + return status; } else { /* could be a reraise or an exception from objmode */ exc = exc_packed; if (exc == Py_None) { @@ -936,18 +965,35 @@ numba_do_raise(PyObject *exc_packed) if (PyExceptionClass_Check(exc)) { type = exc; value = PyObject_CallObject(exc, value); - if (value == NULL) - goto raise_error; + if (value == NULL){ + + Py_XDECREF(value); + Py_XDECREF(type); + return 0; + } if (!PyExceptionInstance_Check(value)) { PyErr_SetString(PyExc_TypeError, "exceptions must derive from BaseException"); - goto raise_error; + + Py_XDECREF(value); + Py_XDECREF(type); + return 0; } + + Py_INCREF(type); + PyErr_SetObject(type, value); + Py_XDECREF(value); + Py_XDECREF(type); + return 1; } else if (PyExceptionInstance_Check(exc)) { value = exc; type = PyExceptionInstance_Class(exc); Py_INCREF(type); + PyErr_SetObject(type, value); + Py_XDECREF(value); + Py_XDECREF(type); + return 0; } else { /* Not something you can raise. You get an exception @@ -955,40 +1001,15 @@ numba_do_raise(PyObject *exc_packed) Py_DECREF(exc); // exc points to exc_packed PyErr_SetString(PyExc_TypeError, "exceptions must derive from BaseException"); - goto raise_error; - } - } - PyErr_SetObject(type, value); - -raise_error_w_loc: - /* instance is instantiated/internal exception is raised, if loc is present - * add a frame for it into the traceback */ - if(loc && loc != Py_None && PyTuple_Check(loc)) - { - pos = 0; - function_name = PyTuple_GET_ITEM(loc, pos); - function_name_str = PyString_AsString(function_name); - pos = 1; - filename = PyTuple_GET_ITEM(loc, pos); - filename_str = PyString_AsString(filename); - pos = 2; - lineno = PyTuple_GET_ITEM(loc, pos); - traceback_add(function_name_str, filename_str, \ - (int)PyLong_AsLong(lineno)); + Py_XDECREF(value); + Py_XDECREF(type); + return 0; + } } - - /* PyErr_SetObject incref's its arguments */ - Py_XDECREF(value); - Py_XDECREF(type); - return 0; - -raise_error: - Py_XDECREF(value); - Py_XDECREF(type); - return 0; } + NUMBA_EXPORT_FUNC(PyObject *) numba_unpickle(const char *data, int n) { From 8b3f34d936ba6c176284e4e7e1eb881ecda2b830 Mon Sep 17 00:00:00 2001 From: Siu Kwan Lam Date: Tue, 21 Jul 2020 16:13:13 -0500 Subject: [PATCH 2/4] Simplify do_raise logic --- numba/_helperlib.c | 205 +++++++++++++++++---------------------------- 1 file changed, 79 insertions(+), 126 deletions(-) diff --git a/numba/_helperlib.c b/numba/_helperlib.c index 2cbe01e63cd..d235f4ff811 100644 --- a/numba/_helperlib.c +++ b/numba/_helperlib.c @@ -850,68 +850,82 @@ void traceback_add_loc(PyObject *loc) { } static -int process_packed_exc(PyObject *exc_packed) { - PyObject *exc = NULL, *type = NULL, *value = NULL, *loc = NULL; - - /* Unpack a (class/inst/tuple, arguments, location) tuple. */ - if (!PyArg_ParseTuple(exc_packed, "OOO", &exc, &value, &loc)) { - traceback_add_loc(loc); - return 0; - } - - if (exc == Py_None) { - /* Reraise */ - PyThreadState *tstate = PyThreadState_GET(); - PyObject *tb; +int reraise_exc_is_none() { + /* Reraise */ + PyThreadState *tstate = PyThreadState_GET(); + PyObject *tb, *type, *value; #if (PY_MAJOR_VERSION >= 3) && (PY_MINOR_VERSION >= 7) - _PyErr_StackItem *tstate_exc = tstate->exc_info; + _PyErr_StackItem *tstate_exc = tstate->exc_info; #else - PyThreadState *tstate_exc = tstate; + PyThreadState *tstate_exc = tstate; #endif - type = tstate_exc->exc_type; - value = tstate_exc->exc_value; - tb = tstate_exc->exc_traceback; - if (type == Py_None) { - PyErr_SetString(PyExc_RuntimeError, - "No active exception to reraise"); - return 0; - } - Py_XINCREF(type); - Py_XINCREF(value); - Py_XINCREF(tb); - PyErr_Restore(type, value, tb); - return 1; + type = tstate_exc->exc_type; + value = tstate_exc->exc_value; + tb = tstate_exc->exc_traceback; + if (type == Py_None) { + PyErr_SetString(PyExc_RuntimeError, + "No active exception to reraise"); + return 0; } + /* incref needed because PyErr_Restore DOES NOT */ + Py_XINCREF(type); + Py_XINCREF(value); + Py_XINCREF(tb); + PyErr_Restore(type, value, tb); + return 1; +} - /* the unpacked exc should be a class, value and loc are set from above - */ - if (PyExceptionClass_Check(exc)) { - /* It is a class, type used here just as a tmp var */ - type = PyObject_CallObject(exc, value); - if (type == NULL){ - traceback_add_loc(loc); - return 0; - } - if (!PyExceptionInstance_Check(type)) { - PyErr_SetString(PyExc_TypeError, - "exceptions must derive from BaseException"); - traceback_add_loc(loc); - Py_DECREF(type); - return 0; - } - /* all ok, set type to the exc */ +/* + * PyExceptionClass_Check(exc) must be True + * value can be NULL + */ +static +int process_exception_class(PyObject *exc, PyObject *value) { + PyObject *type; + /* It is a class, type used here just as a tmp var */ + type = PyObject_CallObject(exc, value); + if (type == NULL){ + return 0; + } + if (!PyExceptionInstance_Check(type)) { + PyErr_SetString(PyExc_TypeError, + "exceptions must derive from BaseException"); Py_DECREF(type); - type = exc; - PyErr_SetObject(type, value); - traceback_add_loc(loc); - return 1; - } else { - /* this should be unreachable as typing should catch it */ + return 0; + } + /* all ok, set type to the exc */ + Py_DECREF(type); + type = exc; + PyErr_SetObject(type, value); + return 1; +} + +/* + * exc cannot be NULL + * value can be NULL + * loc can be NULL + */ +static +int process_raise(PyObject *exc, PyObject *value) { + /* exc is None */ + if (exc == Py_None) { + return reraise_exc_is_none(); + } + /* exc should be an exception class */ + else if (PyExceptionClass_Check(exc)) { + return process_exception_class(exc, value); + } + /* exc is an instance of an Exception */ + else if (PyExceptionInstance_Check(exc)) { + PyObject *type = PyExceptionInstance_Class(exc); + PyErr_SetObject(type, exc); + return 0; + } + else { /* Not something you can raise. You get an exception anyway, just not what you specified :-) */ PyErr_SetString(PyExc_TypeError, "exceptions must derive from BaseException"); - traceback_add_loc(loc); return 0; } } @@ -921,8 +935,8 @@ int process_packed_exc(PyObject *exc_packed) { NUMBA_EXPORT_FUNC(int) numba_do_raise(PyObject *exc_packed) { - PyObject *exc = NULL, *type = NULL, *value = NULL; int status; + PyObject *exc = NULL, *value = NULL, *loc = NULL; /* We support the following forms of raise: raise @@ -931,82 +945,21 @@ numba_do_raise(PyObject *exc_packed) /* could be a tuple from npm (some exc like thing, args, location) */ if (PyTuple_CheckExact(exc_packed)) { - status = process_packed_exc(exc_packed); - Py_DECREF(exc_packed); - return status; - } else { /* could be a reraise or an exception from objmode */ - exc = exc_packed; - if (exc == Py_None) { - /* Reraise */ - PyThreadState *tstate = PyThreadState_GET(); - PyObject *tb; -#if (PY_MAJOR_VERSION >= 3) && (PY_MINOR_VERSION >= 7) - _PyErr_StackItem *tstate_exc = tstate->exc_info; -#else - PyThreadState *tstate_exc = tstate; -#endif - Py_DECREF(exc); - type = tstate_exc->exc_type; - value = tstate_exc->exc_value; - tb = tstate_exc->exc_traceback; - if (type == Py_None) { - PyErr_SetString(PyExc_RuntimeError, - "No active exception to reraise"); - return 0; - } - Py_XINCREF(type); - Py_XINCREF(value); - Py_XINCREF(tb); - PyErr_Restore(type, value, tb); - return 1; - } - - /* exc should be an exception class or an instance of an exception */ - if (PyExceptionClass_Check(exc)) { - type = exc; - value = PyObject_CallObject(exc, value); - if (value == NULL){ - - Py_XDECREF(value); - Py_XDECREF(type); - return 0; - } - if (!PyExceptionInstance_Check(value)) { - PyErr_SetString(PyExc_TypeError, - "exceptions must derive from BaseException"); - - Py_XDECREF(value); - Py_XDECREF(type); - return 0; - } - - Py_INCREF(type); - PyErr_SetObject(type, value); - Py_XDECREF(value); - Py_XDECREF(type); - return 1; - } - else if (PyExceptionInstance_Check(exc)) { - value = exc; - type = PyExceptionInstance_Class(exc); - Py_INCREF(type); - PyErr_SetObject(type, value); - Py_XDECREF(value); - Py_XDECREF(type); - return 0; - } - else { - /* Not something you can raise. You get an exception - anyway, just not what you specified :-) */ - Py_DECREF(exc); // exc points to exc_packed - PyErr_SetString(PyExc_TypeError, - "exceptions must derive from BaseException"); - - Py_XDECREF(value); - Py_XDECREF(type); + /* Unpack a (class/inst/tuple, arguments, location) tuple. */ + if (!PyArg_ParseTuple(exc_packed, "OOO", &exc, &value, &loc)) { + traceback_add_loc(loc); return 0; } + } else { + /* could be a reraise or an exception from objmode */ + exc = exc_packed; + /* branch exit with value = NULL and loc = NULL */ } + /* value is either NULL or borrowed */ + status = process_raise(exc, value); + traceback_add_loc(loc); + Py_DECREF(exc_packed); + return status; } From 07415ebafbe9ae43e20e824762ce6e3941b4f21c Mon Sep 17 00:00:00 2001 From: Siu Kwan Lam Date: Thu, 30 Jul 2020 10:22:17 -0500 Subject: [PATCH 3/4] Add code comments --- numba/_helperlib.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/numba/_helperlib.c b/numba/_helperlib.c index d235f4ff811..6379e91bdd5 100644 --- a/numba/_helperlib.c +++ b/numba/_helperlib.c @@ -826,6 +826,10 @@ static void traceback_add(const char *funcname, const char *filename, int lineno } +/* + * Add traceback information to *loc* to the active exception. + * loc can be NULL, which causes this function to become a no-op. + */ static void traceback_add_loc(PyObject *loc) { const char *function_name_str = NULL, *filename_str = NULL; @@ -849,6 +853,10 @@ void traceback_add_loc(PyObject *loc) { } } +/** + * Reraise current active exception. + * Called internall by process_raise() when *exc* is None. + */ static int reraise_exc_is_none() { /* Reraise */ @@ -876,8 +884,10 @@ int reraise_exc_is_none() { } /* - * PyExceptionClass_Check(exc) must be True - * value can be NULL + * Set exception set given the Exception type and the constructor argument. + * Equivalent to ``raise exc(value)``. + * PyExceptionClass_Check(exc) must be True. + * value can be NULL. */ static int process_exception_class(PyObject *exc, PyObject *value) { @@ -901,9 +911,9 @@ int process_exception_class(PyObject *exc, PyObject *value) { } /* - * exc cannot be NULL - * value can be NULL - * loc can be NULL + * Internal routine to process exceptions. + * exc cannot be NULL. It can be a None, Exception type, or Exception instance + * value can be NULL for absent, or any PyObject valid for the exception. */ static int process_raise(PyObject *exc, PyObject *value) { From 19ce816b06e6437f16cb4bafdab00b3f30f57309 Mon Sep 17 00:00:00 2001 From: stuartarchibald Date: Mon, 3 Aug 2020 09:28:10 +0100 Subject: [PATCH 4/4] Apply suggestions from code review Fix typos. --- numba/_helperlib.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/numba/_helperlib.c b/numba/_helperlib.c index 6379e91bdd5..cfdb2b4fe87 100644 --- a/numba/_helperlib.c +++ b/numba/_helperlib.c @@ -854,8 +854,8 @@ void traceback_add_loc(PyObject *loc) { } /** - * Reraise current active exception. - * Called internall by process_raise() when *exc* is None. + * Re-raise the current active exception. + * Called internal by process_raise() when *exc* is None. */ static int reraise_exc_is_none() { @@ -884,7 +884,7 @@ int reraise_exc_is_none() { } /* - * Set exception set given the Exception type and the constructor argument. + * Set exception given the Exception type and the constructor argument. * Equivalent to ``raise exc(value)``. * PyExceptionClass_Check(exc) must be True. * value can be NULL. @@ -912,7 +912,7 @@ int process_exception_class(PyObject *exc, PyObject *value) { /* * Internal routine to process exceptions. - * exc cannot be NULL. It can be a None, Exception type, or Exception instance + * exc cannot be NULL. It can be a None, Exception type, or Exception instance. * value can be NULL for absent, or any PyObject valid for the exception. */ static