From 02bf1849e85d0715de30e1d0ecd759532b1c11b5 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Wed, 5 Sep 2018 22:57:02 -0700 Subject: [PATCH] bpo-34588: Fix an off-by-one error in traceback formatting. The recursive frame pruning code always undercounted the number of elided frames by one. That is, in the "[Previous line repeated N more times]" message, N would always be one too few. Near the recursive pruning cutoff, one frame could be silently dropped. That situation is demonstrated in the OP of the bug report. The fix is to start the identical frame counter at 1. --- Lib/test/test_traceback.py | 61 ++++++++++++++++++- Lib/traceback.py | 28 ++++++--- .../2018-09-05-22-56-52.bpo-34588.UIuPmL.rst | 2 + Python/traceback.c | 30 ++++----- 4 files changed, 95 insertions(+), 26 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-09-05-22-56-52.bpo-34588.UIuPmL.rst diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index bffc03e663..8a3aa8a864 100644 --- a/Lib/test/test_traceback.py +++ b/Lib/test/test_traceback.py @@ -373,7 +373,7 @@ class TracebackFormatTests(unittest.TestCase): ' return g(count-1)\n' f' File "{__file__}", line {lineno_g+2}, in g\n' ' return g(count-1)\n' - ' [Previous line repeated 6 more times]\n' + ' [Previous line repeated 7 more times]\n' f' File "{__file__}", line {lineno_g+3}, in g\n' ' raise ValueError\n' 'ValueError\n' @@ -412,7 +412,7 @@ class TracebackFormatTests(unittest.TestCase): ' return h(count-1)\n' f' File "{__file__}", line {lineno_h+2}, in h\n' ' return h(count-1)\n' - ' [Previous line repeated 6 more times]\n' + ' [Previous line repeated 7 more times]\n' f' File "{__file__}", line {lineno_h+3}, in h\n' ' g()\n' ) @@ -420,6 +420,63 @@ class TracebackFormatTests(unittest.TestCase): actual = stderr_h.getvalue().splitlines() self.assertEqual(actual, expected) + # Check the boundary conditions. First, test just below the cutoff. + with captured_output("stderr") as stderr_g: + try: + g(traceback._RECURSIVE_CUTOFF) + except ValueError as exc: + render_exc() + else: + self.fail("no error raised") + result_g = ( + f' File "{__file__}", line {lineno_g+2}, in g\n' + ' return g(count-1)\n' + f' File "{__file__}", line {lineno_g+2}, in g\n' + ' return g(count-1)\n' + f' File "{__file__}", line {lineno_g+2}, in g\n' + ' return g(count-1)\n' + f' File "{__file__}", line {lineno_g+3}, in g\n' + ' raise ValueError\n' + 'ValueError\n' + ) + tb_line = ( + 'Traceback (most recent call last):\n' + f' File "{__file__}", line {lineno_g+71}, in _check_recursive_traceback_display\n' + ' g(traceback._RECURSIVE_CUTOFF)\n' + ) + expected = (tb_line + result_g).splitlines() + actual = stderr_g.getvalue().splitlines() + self.assertEqual(actual, expected) + + # Second, test just above the cutoff. + with captured_output("stderr") as stderr_g: + try: + g(traceback._RECURSIVE_CUTOFF + 1) + except ValueError as exc: + render_exc() + else: + self.fail("no error raised") + result_g = ( + f' File "{__file__}", line {lineno_g+2}, in g\n' + ' return g(count-1)\n' + f' File "{__file__}", line {lineno_g+2}, in g\n' + ' return g(count-1)\n' + f' File "{__file__}", line {lineno_g+2}, in g\n' + ' return g(count-1)\n' + ' [Previous line repeated 1 more time]\n' + f' File "{__file__}", line {lineno_g+3}, in g\n' + ' raise ValueError\n' + 'ValueError\n' + ) + tb_line = ( + 'Traceback (most recent call last):\n' + f' File "{__file__}", line {lineno_g+99}, in _check_recursive_traceback_display\n' + ' g(traceback._RECURSIVE_CUTOFF + 1)\n' + ) + expected = (tb_line + result_g).splitlines() + actual = stderr_g.getvalue().splitlines() + self.assertEqual(actual, expected) + def test_recursive_traceback_python(self): self._check_recursive_traceback_display(traceback.print_exc) diff --git a/Lib/traceback.py b/Lib/traceback.py index afab0a4b91..9f8aee1a99 100644 --- a/Lib/traceback.py +++ b/Lib/traceback.py @@ -310,6 +310,8 @@ def walk_tb(tb): tb = tb.tb_next +_RECURSIVE_CUTOFF = 3 # Also hardcoded in traceback.c. + class StackSummary(list): """A stack of frames.""" @@ -398,18 +400,21 @@ class StackSummary(list): last_name = None count = 0 for frame in self: - if (last_file is not None and last_file == frame.filename and - last_line is not None and last_line == frame.lineno and - last_name is not None and last_name == frame.name): - count += 1 - else: - if count > 3: - result.append(f' [Previous line repeated {count-3} more times]\n') + if (last_file is None or last_file != frame.filename or + last_line is None or last_line != frame.lineno or + last_name is None or last_name != frame.name): + if count > _RECURSIVE_CUTOFF: + count -= _RECURSIVE_CUTOFF + result.append( + f' [Previous line repeated {count} more ' + f'time{"s" if count > 1 else ""}]\n' + ) last_file = frame.filename last_line = frame.lineno last_name = frame.name count = 0 - if count >= 3: + count += 1 + if count > _RECURSIVE_CUTOFF: continue row = [] row.append(' File "{}", line {}, in {}\n'.format( @@ -420,8 +425,11 @@ class StackSummary(list): for name, value in sorted(frame.locals.items()): row.append(' {name} = {value}\n'.format(name=name, value=value)) result.append(''.join(row)) - if count > 3: - result.append(f' [Previous line repeated {count-3} more times]\n') + if count > _RECURSIVE_CUTOFF: + count -= _RECURSIVE_CUTOFF + result.append( + f' [Previous line repeated {count} more time{"s" if count > 1 else ""}]\n' + ) return result diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-09-05-22-56-52.bpo-34588.UIuPmL.rst b/Misc/NEWS.d/next/Core and Builtins/2018-09-05-22-56-52.bpo-34588.UIuPmL.rst new file mode 100644 index 0000000000..ec7a57f23a --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-09-05-22-56-52.bpo-34588.UIuPmL.rst @@ -0,0 +1,2 @@ +Fix an off-by-one in the recursive call pruning feature of traceback +formatting. diff --git a/Python/traceback.c b/Python/traceback.c index 21fb034160..5b5c715a0f 100644 --- a/Python/traceback.c +++ b/Python/traceback.c @@ -511,16 +511,21 @@ tb_displayline(PyObject *f, PyObject *filename, int lineno, PyObject *name) return err; } +static const int TB_RECURSIVE_CUTOFF = 3; // Also hardcoded in traceback.py. + static int tb_print_line_repeated(PyObject *f, long cnt) { - int err; + cnt -= TB_RECURSIVE_CUTOFF; PyObject *line = PyUnicode_FromFormat( - " [Previous line repeated %ld more times]\n", cnt-3); + (cnt > 1) + ? " [Previous line repeated %ld more times]\n" + : " [Previous line repeated %ld more time]\n", + cnt); if (line == NULL) { return -1; } - err = PyFile_WriteObject(line, f, Py_PRINT_RAW); + int err = PyFile_WriteObject(line, f, Py_PRINT_RAW); Py_DECREF(line); return err; } @@ -544,15 +549,11 @@ tb_printinternal(PyTracebackObject *tb, PyObject *f, long limit) tb = tb->tb_next; } while (tb != NULL && err == 0) { - if (last_file != NULL && - tb->tb_frame->f_code->co_filename == last_file && - last_line != -1 && tb->tb_lineno == last_line && - last_name != NULL && tb->tb_frame->f_code->co_name == last_name) - { - cnt++; - } - else { - if (cnt > 3) { + if (last_file == NULL || + tb->tb_frame->f_code->co_filename != last_file || + last_line == -1 || tb->tb_lineno != last_line || + last_name == NULL || tb->tb_frame->f_code->co_name != last_name) { + if (cnt > TB_RECURSIVE_CUTOFF) { err = tb_print_line_repeated(f, cnt); } last_file = tb->tb_frame->f_code->co_filename; @@ -560,7 +561,8 @@ tb_printinternal(PyTracebackObject *tb, PyObject *f, long limit) last_name = tb->tb_frame->f_code->co_name; cnt = 0; } - if (err == 0 && cnt < 3) { + cnt++; + if (err == 0 && cnt <= TB_RECURSIVE_CUTOFF) { err = tb_displayline(f, tb->tb_frame->f_code->co_filename, tb->tb_lineno, @@ -571,7 +573,7 @@ tb_printinternal(PyTracebackObject *tb, PyObject *f, long limit) } tb = tb->tb_next; } - if (err == 0 && cnt > 3) { + if (err == 0 && cnt > TB_RECURSIVE_CUTOFF) { err = tb_print_line_repeated(f, cnt); } return err; -- 2.40.0