bpo-42823: Fix frame lineno when frame.f_trace is set (pythonGH-24099)

* Add test for frame.f_lineno with/without tracing.

* Make sure that frame.f_lineno is correct regardless of whether frame.f_trace is set.

* Update importlib

* Add NEWS

Include/cpython/frameobject.h

@@ -42,12 +42,7 @@ struct _frame {
     PyObject *f_gen;
 
     int f_lasti;                /* Last instruction if called */
-    /* Call PyFrame_GetLineNumber() instead of reading this field
-       directly.  As of 2.3 f_lineno is only valid when tracing is
-       active (i.e. when f_trace is set).  At other times we use
-       PyCode_Addr2Line to calculate the line from the current
-       bytecode index. */
-    int f_lineno;               /* Current line number */
+    int f_lineno;               /* Current line number. Only valid if non-zero */
     int f_iblock;               /* index in f_blockstack */
     PyFrameState f_state;       /* What state the frame is in */
     PyTryBlock f_blockstack[CO_MAXBLOCKS]; /* for try and loop blocks */

Lib/test/test_frame.py

@@ -1,4 +1,5 @@
 import re
+import sys
 import types
 import unittest
 import weakref
@@ -94,6 +95,26 @@ def g():
         f.clear()
         self.assertTrue(endly)
 
+    def test_lineno_with_tracing(self):
+        def record_line():
+            f = sys._getframe(1)
+            lines.append(f.f_lineno-f.f_code.co_firstlineno)
+
+        def test(trace):
+            record_line()
+            if trace:
+                sys._getframe(0).f_trace = True
+            record_line()
+            record_line()
+
+        expected_lines = [1, 4, 5]
+        lines = []
+        test(False)
+        self.assertEqual(lines, expected_lines)
+        lines = []
+        test(True)
+        self.assertEqual(lines, expected_lines)
+
     @support.cpython_only
     def test_clear_refcycles(self):
         # .clear() doesn't leave any refcycle behind

Misc/NEWS.d/next/Core and Builtins/2021-01-04-18-17-07.bpo-42823.dcSynu.rst

@@ -0,0 +1 @@
+frame.f_lineno is correct even if frame.f_trace is set to True

Objects/codeobject.c

@@ -1261,7 +1261,8 @@ PyLineTable_InitAddressRange(char *linetable, int firstlineno, PyCodeAddressRang
     range->lo_next = linetable;
     range->ar_start = -1;
     range->ar_end = 0;
-    range->ar_computed_line = range->ar_line = firstlineno;
+    range->ar_computed_line = firstlineno;
+    range->ar_line = -1;
 }
 
 int

Objects/frameobject.c

@@ -44,7 +44,7 @@ int
 PyFrame_GetLineNumber(PyFrameObject *f)
 {
     assert(f != NULL);
-    if (f->f_trace) {
+    if (f->f_lineno != 0) {
         return f->f_lineno;
     }
     else {
@@ -476,8 +476,8 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
         start_block_stack = pop_block(start_block_stack);
     }
 
-    /* Finally set the new f_lineno and f_lasti and return OK. */
-    f->f_lineno = new_lineno;
+    /* Finally set the new f_lasti and return OK. */
+    f->f_lineno = 0;
     f->f_lasti = best_addr;
     return 0;
 }
@@ -498,11 +498,9 @@ frame_gettrace(PyFrameObject *f, void *closure)
 static int
 frame_settrace(PyFrameObject *f, PyObject* v, void *closure)
 {
-    /* We rely on f_lineno being accurate when f_trace is set. */
-    f->f_lineno = PyFrame_GetLineNumber(f);
-
-    if (v == Py_None)
+    if (v == Py_None) {
         v = NULL;
+    }
     Py_XINCREF(v);
     Py_XSETREF(f->f_trace, v);
 

Python/ceval.c

@@ -4993,27 +4993,28 @@ maybe_call_line_trace(Py_tracefunc func, PyObject *obj,
                       PyCodeAddressRange *bounds, int *instr_prev)
 {
     int result = 0;
-    int line = frame->f_lineno;
 
-    /* If the last instruction executed isn't in the current
-       instruction window, reset the window.
-    */
-    line = _PyCode_CheckLineNumber(frame->f_lasti, bounds);
     /* If the last instruction falls at the start of a line or if it
        represents a jump backwards, update the frame's line number and
        then call the trace function if we're tracing source lines.
     */
-    if ((line != frame->f_lineno || frame->f_lasti < *instr_prev)) {
-        if (line != -1) {
+    int lastline = bounds->ar_line;
+    int line = _PyCode_CheckLineNumber(frame->f_lasti, bounds);
+    if (line != -1 && frame->f_trace_lines) {
+        /* Trace backward edges or first instruction of a new line */
+        if (frame->f_lasti < *instr_prev ||
+            (line != lastline && frame->f_lasti == bounds->ar_start))
+        {
             frame->f_lineno = line;
-            if (frame->f_trace_lines) {
-                result = call_trace(func, obj, tstate, frame, PyTrace_LINE, Py_None);
-            }
+            result = call_trace(func, obj, tstate, frame, PyTrace_LINE, Py_None);
+            frame->f_lineno = 0;
         }
     }
     /* Always emit an opcode event if we're tracing all opcodes. */
     if (frame->f_trace_opcodes) {
+        frame->f_lineno = _PyCode_CheckLineNumber(frame->f_lasti, bounds);
         result = call_trace(func, obj, tstate, frame, PyTrace_OPCODE, Py_None);
+        frame->f_lineno = 0;
     }
     *instr_prev = frame->f_lasti;
     return result;

Python/compile.c

@@ -6558,6 +6558,12 @@ ensure_exits_have_lineno(struct compiler *c)
     if (is_exit_without_lineno(entry)) {
         entry->b_instr[0].i_lineno = c->u->u_firstlineno;
     }
+    /* Eliminate empty blocks */
+    for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) {
+        while (b->b_next && b->b_next->b_iused == 0) {
+            b->b_next = b->b_next->b_next;
+        }
+    }
     /* Any remaining reachable exit blocks without line number can only be reached by
      * fall through, and thus can only have a single predecessor */
     for (basicblock *b = c->u->u_blocks; b != NULL; b = b->b_list) {