diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py index bffc03e663f..8a3aa8a8648 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 afab0a4b91f..4e7605d15fa 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,12 @@ 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 ' + f'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 00000000000..ec7a57f23ad --- /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 b00864b06e4..95bef64e734 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;