Skip to content

closes bpo-34588: Fix an off-by-one error in traceback formatting. #9077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 10, 2018
Merged

closes bpo-34588: Fix an off-by-one error in traceback formatting. #9077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 59 additions & 2 deletions Lib/test/test_traceback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -412,14 +412,71 @@ 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'
)
expected = (result_h + result_g).splitlines()
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)

Expand Down
29 changes: 19 additions & 10 deletions Lib/traceback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down Expand Up @@ -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(
Expand All @@ -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


Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2018-09-05-22-56-52.bpo-34588.UIuPmL.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix an off-by-one in the recursive call pruning feature of traceback
formatting.
30 changes: 16 additions & 14 deletions Python/traceback.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -544,23 +549,20 @@ 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;
last_line = tb->tb_lineno;
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,
Expand All @@ -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;
Expand Down