From 0fe104fce7da709edddb701baa2249e3275db1fd Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Fri, 10 Dec 2021 23:02:10 +0000 Subject: [PATCH] bpo-45635: refactor print_exception_recursive into smaller functions to standardize error handling (GH-30015) Co-authored-by: Erlend Egeberg Aasland --- Python/pythonrun.c | 366 +++++++++++++++++++++++++-------------------- 1 file changed, 202 insertions(+), 164 deletions(-) diff --git a/Python/pythonrun.c b/Python/pythonrun.c index 74b497c8f06..4fff088fc3e 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -956,7 +956,7 @@ print_exception_traceback(struct exception_print_context *ctx, PyObject *value) /* Prints the message line: module.qualname[: str(exc)] */ static int print_exception_message(struct exception_print_context *ctx, PyObject *type, - PyObject *value) + PyObject *value) { PyObject *f = ctx->file; @@ -1253,55 +1253,205 @@ print_chained(struct exception_print_context* ctx, PyObject *value, return 0; } -static void -print_exception_recursive(struct exception_print_context* ctx, PyObject *value) +/* Return true if value is in seen or there was a lookup error. + * Return false if lookup succeeded and the item was not found. + * We suppress errors because this makes us err on the side of + * under-printing which is better than over-printing irregular + * exceptions (e.g., unhashable ones). + */ +static bool +print_exception_seen_lookup(struct exception_print_context *ctx, + PyObject *value) { - int err = 0, res; - PyObject *cause, *context; + PyObject *check_id = PyLong_FromVoidPtr(value); + if (check_id == NULL) { + PyErr_Clear(); + return true; + } + int in_seen = PySet_Contains(ctx->seen, check_id); + Py_DECREF(check_id); + if (in_seen == -1) { + PyErr_Clear(); + return true; + } + + if (in_seen == 1) { + /* value is in seen */ + return true; + } + return false; +} + +static int +print_exception_cause_and_context(struct exception_print_context *ctx, + PyObject *value) +{ + PyObject *value_id = PyLong_FromVoidPtr(value); + if (value_id == NULL || PySet_Add(ctx->seen, value_id) == -1) { + PyErr_Clear(); + Py_XDECREF(value_id); + return 0; + } + Py_DECREF(value_id); + + if (!PyExceptionInstance_Check(value)) { + return 0; + } + + PyObject *cause = PyException_GetCause(value); + if (cause) { + int err = 0; + if (!print_exception_seen_lookup(ctx, cause)) { + err = print_chained(ctx, cause, cause_message, "cause"); + } + Py_DECREF(cause); + return err; + } + if (((PyBaseExceptionObject *)value)->suppress_context) { + return 0; + } + PyObject *context = PyException_GetContext(value); + if (context) { + int err = 0; + if (!print_exception_seen_lookup(ctx, context)) { + err = print_chained(ctx, context, context_message, "context"); + } + Py_DECREF(context); + return err; + } + return 0; +} + +static int +print_exception_group(struct exception_print_context *ctx, PyObject *value) +{ + PyObject *f = ctx->file; + + if (ctx->exception_group_depth > ctx->max_group_depth) { + /* depth exceeds limit */ + + if (write_indented_margin(ctx, f) < 0) { + return -1; + } + + PyObject *line = PyUnicode_FromFormat("... (max_group_depth is %d)\n", + ctx->max_group_depth); + if (line == NULL) { + return -1; + } + int err = PyFile_WriteObject(line, f, Py_PRINT_RAW); + Py_DECREF(line); + return err; + } + + if (ctx->exception_group_depth == 0) { + ctx->exception_group_depth += 1; + } + print_exception(ctx, value); + + PyObject *excs = ((PyBaseExceptionGroupObject *)value)->excs; + assert(excs && PyTuple_Check(excs)); + Py_ssize_t num_excs = PyTuple_GET_SIZE(excs); + assert(num_excs > 0); + Py_ssize_t n; + if (num_excs <= ctx->max_group_width) { + n = num_excs; + } + else { + n = ctx->max_group_width + 1; + } + + ctx->need_close = false; + for (Py_ssize_t i = 0; i < n; i++) { + bool last_exc = (i == n - 1); + if (last_exc) { + // The closing frame may be added in a recursive call + ctx->need_close = true; + } + + if (_Py_WriteIndent(EXC_INDENT(ctx), f) < 0) { + return -1; + } + bool truncated = (i >= ctx->max_group_width); + PyObject *line; + if (!truncated) { + line = PyUnicode_FromFormat( + "%s+---------------- %zd ----------------\n", + (i == 0) ? "+-" : " ", i + 1); + } + else { + line = PyUnicode_FromFormat( + "%s+---------------- ... ----------------\n", + (i == 0) ? "+-" : " "); + } + if (line == NULL) { + return -1; + } + int err = PyFile_WriteObject(line, f, Py_PRINT_RAW); + Py_DECREF(line); + if (err < 0) { + return -1; + } + + ctx->exception_group_depth += 1; + PyObject *exc = PyTuple_GET_ITEM(excs, i); + + if (!truncated) { + if (Py_EnterRecursiveCall(" in print_exception_recursive") != 0) { + return -1; + } + print_exception_recursive(ctx, exc); + Py_LeaveRecursiveCall(); + } + else { + Py_ssize_t excs_remaining = num_excs - ctx->max_group_width; + + if (write_indented_margin(ctx, f) < 0) { + return -1; + } + + PyObject *line = PyUnicode_FromFormat( + "and %zd more exception%s\n", + excs_remaining, excs_remaining > 1 ? "s" : ""); + + if (line == NULL) { + return -1; + } + + int err = PyFile_WriteObject(line, f, Py_PRINT_RAW); + Py_DECREF(line); + if (err < 0) { + return -1; + } + } + + if (last_exc && ctx->need_close) { + if (_Py_WriteIndent(EXC_INDENT(ctx), f) < 0) { + return -1; + } + if (PyFile_WriteString( + "+------------------------------------\n", f) < 0) { + return -1; + } + ctx->need_close = false; + } + ctx->exception_group_depth -= 1; + } + + if (ctx->exception_group_depth == 1) { + ctx->exception_group_depth -= 1; + } + return 0; +} + +static void +print_exception_recursive(struct exception_print_context *ctx, PyObject *value) +{ + int err = 0; if (ctx->seen != NULL) { /* Exception chaining */ - PyObject *value_id = PyLong_FromVoidPtr(value); - if (value_id == NULL || PySet_Add(ctx->seen, value_id) == -1) - PyErr_Clear(); - else if (PyExceptionInstance_Check(value)) { - PyObject *check_id = NULL; - - cause = PyException_GetCause(value); - context = PyException_GetContext(value); - if (cause) { - check_id = PyLong_FromVoidPtr(cause); - if (check_id == NULL) { - res = -1; - } else { - res = PySet_Contains(ctx->seen, check_id); - Py_DECREF(check_id); - } - if (res == -1) - PyErr_Clear(); - if (res == 0) { - err = print_chained(ctx, cause, cause_message, "cause"); - } - } - else if (context && - !((PyBaseExceptionObject *)value)->suppress_context) { - check_id = PyLong_FromVoidPtr(context); - if (check_id == NULL) { - res = -1; - } else { - res = PySet_Contains(ctx->seen, check_id); - Py_DECREF(check_id); - } - if (res == -1) - PyErr_Clear(); - if (res == 0) { - err = print_chained(ctx, context, context_message, "context"); - } - } - Py_XDECREF(context); - Py_XDECREF(cause); - } - Py_XDECREF(value_id); + err = print_exception_cause_and_context(ctx, value); } if (err) { /* don't do anything else */ @@ -1309,127 +1459,15 @@ print_exception_recursive(struct exception_print_context* ctx, PyObject *value) else if (!_PyBaseExceptionGroup_Check(value)) { print_exception(ctx, value); } - else if (ctx->exception_group_depth > ctx->max_group_depth) { - /* exception group but depth exceeds limit */ - - PyObject *line = PyUnicode_FromFormat( - "... (max_group_depth is %d)\n", ctx->max_group_depth); - - if (line) { - PyObject *f = ctx->file; - if (err == 0) { - err = write_indented_margin(ctx, f); - } - if (err == 0) { - err = PyFile_WriteObject(line, f, Py_PRINT_RAW); - } - Py_DECREF(line); - } - else { - err = -1; - } - } else { - /* format exception group */ - - if (ctx->exception_group_depth == 0) { - ctx->exception_group_depth += 1; - } - print_exception(ctx, value); - - PyObject *excs = ((PyBaseExceptionGroupObject *)value)->excs; - assert(excs && PyTuple_Check(excs)); - Py_ssize_t num_excs = PyTuple_GET_SIZE(excs); - assert(num_excs > 0); - Py_ssize_t n; - if (num_excs <= ctx->max_group_width) { - n = num_excs; + int prev_depth = ctx->exception_group_depth; + err = print_exception_group(ctx, value); + if (err < 0) { + /* restore the depth as long as we're ignoring errors */ + ctx->exception_group_depth = prev_depth; } else { - n = ctx->max_group_width + 1; - } - - PyObject *f = ctx->file; - - ctx->need_close = false; - for (Py_ssize_t i = 0; i < n; i++) { - int last_exc = (i == n - 1); - if (last_exc) { - // The closing frame may be added in a recursive call - ctx->need_close = true; - } - PyObject *line; - bool truncated = (i >= ctx->max_group_width); - if (!truncated) { - line = PyUnicode_FromFormat( - "%s+---------------- %zd ----------------\n", - (i == 0) ? "+-" : " ", i + 1); - } - else { - line = PyUnicode_FromFormat( - "%s+---------------- ... ----------------\n", - (i == 0) ? "+-" : " "); - } - - if (line) { - if (err == 0) { - err = _Py_WriteIndent(EXC_INDENT(ctx), f); - } - if (err == 0) { - err = PyFile_WriteObject(line, f, Py_PRINT_RAW); - } - Py_DECREF(line); - } - else { - err = -1; - } - - if (err == 0) { - ctx->exception_group_depth += 1; - PyObject *exc = PyTuple_GET_ITEM(excs, i); - - if (!truncated) { - if (!Py_EnterRecursiveCall(" in print_exception_recursive")) { - print_exception_recursive(ctx, exc); - Py_LeaveRecursiveCall(); - } - else { - err = -1; - } - } - else { - Py_ssize_t excs_remaining = num_excs - ctx->max_group_width; - PyObject *line = PyUnicode_FromFormat( - "and %zd more exception%s\n", - excs_remaining, excs_remaining > 1 ? "s" : ""); - - if (line) { - if (err == 0) { - err = write_indented_margin(ctx, f); - } - if (err == 0) { - err = PyFile_WriteObject(line, f, Py_PRINT_RAW); - } - Py_DECREF(line); - } - else { - err = -1; - } - } - - if (err == 0 && last_exc && ctx->need_close) { - err = _Py_WriteIndent(EXC_INDENT(ctx), f); - if (err == 0) { - err = PyFile_WriteString( - "+------------------------------------\n", f); - } - ctx->need_close = false; - } - ctx->exception_group_depth -= 1; - } - } - if (ctx->exception_group_depth == 1) { - ctx->exception_group_depth -= 1; + assert(prev_depth == ctx->exception_group_depth); } } if (err != 0)