[Python-checkins] bpo-34588: Fix an off-by-one error in traceback formatting. (GH-9077)

Mon Sep 10 12:00:13 EDT 2018

https://github.com/python/cpython/commit/49020174305ca3dc90a811b03a05f44873297c61
commit: 49020174305ca3dc90a811b03a05f44873297c61
branch: 3.7
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2018年09月10日T09:00:08-07:00
summary:
bpo-34588: Fix an off-by-one error in traceback formatting. (GH-9077)
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.
(cherry picked from commit d545869d084e70d4838310e79b52a25a72a1ca56)
Co-authored-by: Benjamin Peterson <benjamin at python.org>
files:
A Misc/NEWS.d/next/Core and Builtins/2018-09-05-22-56-52.bpo-34588.UIuPmL.rst
M Lib/test/test_traceback.py
M Lib/traceback.py
M Python/traceback.c

diff --git a/Lib/test/test_traceback.py b/Lib/test/test_traceback.py
index bffc03e663ff..8a3aa8a8648f 100644
--- a/Lib/test/test_traceback.py
+++ b/Lib/test/test_traceback.py
@@ -373,7 +373,7 @@ def g(count=10):
 ' 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 @@ def h(count=10):
 ' 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 @@ def h(count=10):
 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 afab0a4b91f1..4e7605d15fa4 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 @@ def format(self):
 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 @@ def format(self):
 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 000000000000..ec7a57f23ad2
--- /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 b00864b06e43..95bef64e734b 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;
