Skip to content

Commit 59936ae

Browse files
benjaminpbenjaminp
committed
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.
1 parent 0afada1 commit 59936ae

4 files changed

Lines changed: 96 additions & 26 deletions

File tree

‎Lib/test/test_traceback.py‎

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ def g(count=10):
373373
' return g(count-1)\n'
374374
f' File "{__file__}", line {lineno_g+2}, in g\n'
375375
' return g(count-1)\n'
376-
' [Previous line repeated 6 more times]\n'
376+
' [Previous line repeated 7 more times]\n'
377377
f' File "{__file__}", line {lineno_g+3}, in g\n'
378378
' raise ValueError\n'
379379
'ValueError\n'
@@ -412,14 +412,71 @@ def h(count=10):
412412
' return h(count-1)\n'
413413
f' File "{__file__}", line {lineno_h+2}, in h\n'
414414
' return h(count-1)\n'
415-
' [Previous line repeated 6 more times]\n'
415+
' [Previous line repeated 7 more times]\n'
416416
f' File "{__file__}", line {lineno_h+3}, in h\n'
417417
' g()\n'
418418
)
419419
expected = (result_h + result_g).splitlines()
420420
actual = stderr_h.getvalue().splitlines()
421421
self.assertEqual(actual, expected)
422422

423+
# Check the boundary conditions. First, test just below the cutoff.
424+
with captured_output("stderr") as stderr_g:
425+
try:
426+
g(traceback._RECURSIVE_CUTOFF)
427+
except ValueError as exc:
428+
render_exc()
429+
else:
430+
self.fail("no error raised")
431+
result_g = (
432+
f' File "{__file__}", line {lineno_g+2}, in g\n'
433+
' return g(count-1)\n'
434+
f' File "{__file__}", line {lineno_g+2}, in g\n'
435+
' return g(count-1)\n'
436+
f' File "{__file__}", line {lineno_g+2}, in g\n'
437+
' return g(count-1)\n'
438+
f' File "{__file__}", line {lineno_g+3}, in g\n'
439+
' raise ValueError\n'
440+
'ValueError\n'
441+
)
442+
tb_line = (
443+
'Traceback (most recent call last):\n'
444+
f' File "{__file__}", line {lineno_g+71}, in _check_recursive_traceback_display\n'
445+
' g(traceback._RECURSIVE_CUTOFF)\n'
446+
)
447+
expected = (tb_line + result_g).splitlines()
448+
actual = stderr_g.getvalue().splitlines()
449+
self.assertEqual(actual, expected)
450+
451+
# Second, test just above the cutoff.
452+
with captured_output("stderr") as stderr_g:
453+
try:
454+
g(traceback._RECURSIVE_CUTOFF + 1)
455+
except ValueError as exc:
456+
render_exc()
457+
else:
458+
self.fail("no error raised")
459+
result_g = (
460+
f' File "{__file__}", line {lineno_g+2}, in g\n'
461+
' return g(count-1)\n'
462+
f' File "{__file__}", line {lineno_g+2}, in g\n'
463+
' return g(count-1)\n'
464+
f' File "{__file__}", line {lineno_g+2}, in g\n'
465+
' return g(count-1)\n'
466+
' [Previous line repeated 1 more time]\n'
467+
f' File "{__file__}", line {lineno_g+3}, in g\n'
468+
' raise ValueError\n'
469+
'ValueError\n'
470+
)
471+
tb_line = (
472+
'Traceback (most recent call last):\n'
473+
f' File "{__file__}", line {lineno_g+99}, in _check_recursive_traceback_display\n'
474+
' g(traceback._RECURSIVE_CUTOFF + 1)\n'
475+
)
476+
expected = (tb_line + result_g).splitlines()
477+
actual = stderr_g.getvalue().splitlines()
478+
self.assertEqual(actual, expected)
479+
423480
def test_recursive_traceback_python(self):
424481
self._check_recursive_traceback_display(traceback.print_exc)
425482

‎Lib/traceback.py‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,8 @@ def walk_tb(tb):
310310
tb = tb.tb_next
311311

312312

313+
_RECURSIVE_CUTOFF = 3 # Also hardcoded in traceback.c.
314+
313315
class StackSummary(list):
314316
"""A stack of frames."""
315317

@@ -398,18 +400,21 @@ def format(self):
398400
last_name = None
399401
count = 0
400402
for frame in self:
401-
if (last_file is not None and last_file == frame.filename and
402-
last_line is not None and last_line == frame.lineno and
403-
last_name is not None and last_name == frame.name):
404-
count += 1
405-
else:
406-
if count > 3:
407-
result.append(f' [Previous line repeated {count-3} more times]\n')
403+
if (last_file is None or last_file != frame.filename or
404+
last_line is None or last_line != frame.lineno or
405+
last_name is None or last_name != frame.name):
406+
if count > _RECURSIVE_CUTOFF:
407+
count -= _RECURSIVE_CUTOFF
408+
result.append(
409+
f' [Previous line repeated {count} more '
410+
f'time{"s" if count > 1 else ""}]\n'
411+
)
408412
last_file = frame.filename
409413
last_line = frame.lineno
410414
last_name = frame.name
411415
count = 0
412-
if count >= 3:
416+
count += 1
417+
if count > _RECURSIVE_CUTOFF:
413418
continue
414419
row = []
415420
row.append(' File "{}", line {}, in {}\n'.format(
@@ -420,8 +425,12 @@ def format(self):
420425
for name, value in sorted(frame.locals.items()):
421426
row.append(' {name} = {value}\n'.format(name=name, value=value))
422427
result.append(''.join(row))
423-
if count > 3:
424-
result.append(f' [Previous line repeated {count-3} more times]\n')
428+
if count > _RECURSIVE_CUTOFF:
429+
count -= _RECURSIVE_CUTOFF
430+
result.append(
431+
f' [Previous line repeated {count} more '
432+
f'time{"s" if count > 1 else ""}]\n'
433+
)
425434
return result
426435

427436

‎Misc/NEWS.d/next/Core and Builtins/2018-09-05-22-56-52.bpo-34588.UIuPmL.rst‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix an off-by-one in the recursive call pruning feature of traceback
2+
formatting.

‎Python/traceback.c‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -511,16 +511,21 @@ tb_displayline(PyObject *f, PyObject *filename, int lineno, PyObject *name)
511511
return err;
512512
}
513513

514+
static const int TB_RECURSIVE_CUTOFF = 3; // Also hardcoded in traceback.py.
515+
514516
static int
515517
tb_print_line_repeated(PyObject *f, long cnt)
516518
{
517-
int err;
519+
cnt -= TB_RECURSIVE_CUTOFF;
518520
PyObject *line = PyUnicode_FromFormat(
519-
" [Previous line repeated %ld more times]\n", cnt-3);
521+
(cnt > 1)
522+
? " [Previous line repeated %ld more times]\n"
523+
: " [Previous line repeated %ld more time]\n",
524+
cnt);
520525
if (line == NULL) {
521526
return -1;
522527
}
523-
err = PyFile_WriteObject(line, f, Py_PRINT_RAW);
528+
int err = PyFile_WriteObject(line, f, Py_PRINT_RAW);
524529
Py_DECREF(line);
525530
return err;
526531
}
@@ -544,23 +549,20 @@ tb_printinternal(PyTracebackObject *tb, PyObject *f, long limit)
544549
tb = tb->tb_next;
545550
}
546551
while (tb != NULL && err == 0) {
547-
if (last_file != NULL &&
548-
tb->tb_frame->f_code->co_filename == last_file &&
549-
last_line != -1 && tb->tb_lineno == last_line &&
550-
last_name != NULL && tb->tb_frame->f_code->co_name == last_name)
551-
{
552-
cnt++;
553-
}
554-
else {
555-
if (cnt > 3) {
552+
if (last_file == NULL ||
553+
tb->tb_frame->f_code->co_filename != last_file ||
554+
last_line == -1 || tb->tb_lineno != last_line ||
555+
last_name == NULL || tb->tb_frame->f_code->co_name != last_name) {
556+
if (cnt > TB_RECURSIVE_CUTOFF) {
556557
err = tb_print_line_repeated(f, cnt);
557558
}
558559
last_file = tb->tb_frame->f_code->co_filename;
559560
last_line = tb->tb_lineno;
560561
last_name = tb->tb_frame->f_code->co_name;
561562
cnt = 0;
562563
}
563-
if (err == 0 && cnt < 3) {
564+
cnt++;
565+
if (err == 0 && cnt <= TB_RECURSIVE_CUTOFF) {
564566
err = tb_displayline(f,
565567
tb->tb_frame->f_code->co_filename,
566568
tb->tb_lineno,
@@ -571,7 +573,7 @@ tb_printinternal(PyTracebackObject *tb, PyObject *f, long limit)
571573
}
572574
tb = tb->tb_next;
573575
}
574-
if (err == 0 && cnt > 3) {
576+
if (err == 0 && cnt > TB_RECURSIVE_CUTOFF) {
575577
err = tb_print_line_repeated(f, cnt);
576578
}
577579
return err;

0 commit comments

Comments
 (0)