bpo-24565: f->f_lineno is now -1 when tracing is not set

When tracing is set, f->f_lineno is valid for all the frames of the stack except upon starting or resuming a frame where it is -1 until the first call to maybe_call_line_trace(). Fix issues 7238, 16482, 17277 and 17697.
xdegaye · Mar 25, 2018 · Mar 25, 2018 · Mar 25, 2018 · 8a49be2477710a667957bdb250213db37b78f819 · xdegaye
commit 8a49be2477710a667957bdb250213db37b78f819
diff --git a/Include/frameobject.h b/Include/frameobject.h
@@ -35,10 +35,9 @@ typedef struct _frame {
 
     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. */
+       directly.  As of 2.3 f_lineno is valid when tracing is active.  At
+       other times we use PyCode_Addr2Line to calculate the line from the
+       current bytecode index and f->f_lineno is -1. */
     int f_lineno;               /* Current line number */
     int f_iblock;               /* index in f_blockstack */
     char f_executing;           /* whether the frame is still executing */

diff --git a/Lib/bdb.py b/Lib/bdb.py
@@ -341,10 +341,6 @@ def set_continue(self):
         if not self.breaks:
             # no breakpoints; run without debugger overhead
             sys.settrace(None)
-            frame = sys._getframe().f_back
-            while frame and frame is not self.botframe:
-                del frame.f_trace
-                frame = frame.f_back
 
     def set_quit(self):
         """Set quitting attribute to True.

diff --git a/Lib/test/test_bdb.py b/Lib/test/test_bdb.py
@@ -1139,6 +1139,41 @@ def main():
             with TracerRun(self) as tracer:
                 tracer.runcall(tfunc_import)
 
+    def test_botframe_lineno(self):
+        # Issue #17697.
+        # This test failed when the condition f->f_trace != NULL was used to
+        # mean that tracing is active and therefore f->f_lineno are valid. The
+        # implementation of Bdb.set_continue() did not delete the f_trace of
+        # self.botframe and in that case all the f.f_lineno in the func()
+        # frame after returning from subf() have the same value, i.e. the line
+        # number of the call to subf().
+        def subf():
+            lno = 2
+
+        def func(tracer):
+            tracer.set_trace()
+            f = sys._getframe()
+            tracer.botframe = f     # assume func() is a module
+            subf()
+            if f.f_lineno - f.f_code.co_firstlineno + 1 == 6:
+                tracer.set_trace()
+            lno = 8
+
+        self.expect_set = [
+            ('line', 3, 'func'),            ('step',),
+            ('line', 4, 'func'),            ('step',),
+            ('line', 5, 'func'),            ('step',),
+            ('call', 1, 'subf'),            ('step',),
+            ('line', 2, 'subf'),            ('continue',),
+            ('line', 8, 'func'),            ('step',),
+            ('return', 8, 'func'),          ('quit',),
+        ]
+        with TracerRun(self) as tracer:
+            try:
+                func(tracer)
+            except _bdb.BdbQuit:
+                pass
+
 def test_main():
     test.support.run_unittest(
         StateTestCase,

diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py
@@ -7,8 +7,16 @@
 import gc
 from functools import wraps
 import asyncio
+from itertools import islice
 
 
+def get_frame_lineno(caller_level=0):
+    f = sys._getframe().f_back
+    while caller_level and f:
+        caller_level -= 1
+        f = f.f_back
+    return (f.f_lineno - f.f_code.co_firstlineno) if f else None
+
 class tracecontext:
     """Context manager that traces its enter and exit."""
     def __init__(self, output, value):
@@ -443,6 +451,100 @@ def func():
             [(0, 'call'),
              (1, 'line')])
 
+    def test_18_generator(self):
+        # Issue 17277: the f_lineno attribute of a generator frame is correct
+        # after the tracer has been removed while the generator was suspended.
+        def func():
+            def gen():
+                yield 1
+                self.assertEqual(get_frame_lineno(), 2)
+                yield 3
+
+            it = gen()
+            next(it)
+            sys.settrace(None)
+            next(it)
+
+        func.events = [
+            (0, 'call'),
+            (1, 'line'),
+            (6, 'line'),
+            (7, 'line'),
+            (1, 'call'),
+            (2, 'line'),
+            (2, 'return'),
+            (8, 'line'),
+        ]
+        self.run_test(func)
+
+    def test_19_stack_frame(self):
+        # Issues 16482 and 17277: the f_lineno of a frame on the call stack is
+        # correct after the tracer has been removed.
+        def func():
+            def remove_tracer():
+                sys.settrace(None)
+
+            def f():
+                remove_tracer()
+                self.assertEqual(get_frame_lineno(), 2)
+
+            f()
+
+        func.events = [
+            (0, 'call'),
+            (1, 'line'),
+            (4, 'line'),
+            (8, 'line'),
+            (4, 'call'),
+            (5, 'line'),
+            (1, 'call'),
+            (2, 'line'),
+        ]
+        self.run_test(func)
+
+    def test_20_return_in_caller(self):
+        # Issue 24565: upon returning in the caller and when the next opcode
+        # in the caller is RETURN_VALUE, f_lineno is the line of the previous
+        # line trace event, which is not necessarily the line obtained from
+        # co_lnotab and f_lasti. See the discussion at the end of
+        # Objects/lnotab_notes.txt.
+        def func(tracer):
+            def set_trace():
+                sys._getframe().f_back.f_trace = tracer
+                sys.settrace(tracer)
+
+            def f(a):
+                while a:
+                    lineno = 7
+                    set_trace(); break
+                else:
+                    lineno = 10
+
+            f(True)
+
+        func.events = [(8, 'return')]
+        self.run_test2(func)
+
+    def test_21_tracer_removed_after_call(self):
+        # Issue 7238: remove the tracer after a call event.
+        def func():
+            def f():
+                sys.settrace(None)
+                self.assertEqual(get_frame_lineno(), 2)
+                self.assertEqual(get_frame_lineno(), 3)
+
+            f()
+            f()
+
+        func.events = [
+            (0, 'call'),
+            (1, 'line'),
+            (6, 'line'),
+            (1, 'call'),
+            (2, 'line'),
+        ]
+        self.run_test(func)
+
 
 class SkipLineEventsTraceTestCase(TraceTestCase):
     """Repeat the trace tests, but with per-line events skipped"""

diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-03-23-18-31-12.bpo-24565.8eJzyI.rst b/Misc/NEWS.d/next/Core and Builtins/2018-03-23-18-31-12.bpo-24565.8eJzyI.rst
@@ -0,0 +1,4 @@
+`f->f_lineno` is now `-1` when tracing is not set. When tracing is
+set, `f->f_lineno` is valid for all the frames on the stack except
+upon starting or resuming a frame where it is -1 until the first
+call to :c:func:`maybe_call_line_trace()`.
diff --git a/Objects/frameobject.c b/Objects/frameobject.c
@@ -33,7 +33,7 @@ frame_getlocals(PyFrameObject *f, void *closure)
 int
 PyFrame_GetLineNumber(PyFrameObject *f)
 {
-    if (f->f_trace)
+    if (f->f_lineno != -1)
         return f->f_lineno;
     else
         return PyCode_Addr2Line(f->f_code, f->f_lasti);
@@ -112,21 +112,19 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno)
         return -1;
     }
 
-    /* Upon the 'call' trace event of a new frame, f->f_lasti is -1 and
-     * f->f_trace is NULL, check first on the first condition.
-     * Forbidding jumps from the 'call' event of a new frame is a side effect
-     * of allowing to set f_lineno only from trace functions. */
+    /* Upon the 'call' trace event of a new frame, f->f_lasti is -1 */
     if (f->f_lasti == -1) {
         PyErr_Format(PyExc_ValueError,
                      "can't jump from the 'call' trace event of a new frame");
         return -1;
     }
 
-    /* You can only do this from within a trace function, not via
-     * _getframe or similar hackery. */
-    if (!f->f_trace) {
+    /* You can only do this from within a line trace function, not via
+     * _getframe or similar hackery or from the call trace function
+     * of a resumed generator. */
+    if (f->f_lineno == -1) {
         PyErr_Format(PyExc_ValueError,
-                     "f_lineno can only be set by a trace function");
+                     "f_lineno can only be set by a line trace function");
         return -1;
     }
 
@@ -332,9 +330,6 @@ 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)
         v = NULL;
     Py_XINCREF(v);
@@ -705,7 +700,7 @@ _PyFrame_New_NoTrack(PyThreadState *tstate, PyCodeObject *code,
     }
 
     f->f_lasti = -1;
-    f->f_lineno = code->co_firstlineno;
+    f->f_lineno = -1;
     f->f_iblock = 0;
     f->f_executing = 0;
     f->f_gen = NULL;

diff --git a/Python/ceval.c b/Python/ceval.c
@@ -3458,6 +3458,7 @@ _PyEval_EvalFrameDefault(PyFrameObject *f, int throwflag)
                                      tstate, f, PyTrace_RETURN, retval)) {
                 Py_CLEAR(retval);
             }
+            f->f_lineno = -1;
         }
         if (tstate->c_profilefunc) {
             if (call_trace_protected(tstate->c_profilefunc, tstate->c_profileobj,
@@ -4308,8 +4309,10 @@ PyEval_SetProfile(Py_tracefunc func, PyObject *arg)
 void
 PyEval_SetTrace(Py_tracefunc func, PyObject *arg)
 {
+    PyFrameObject *f;
     PyThreadState *tstate = PyThreadState_GET();
     PyObject *temp = tstate->c_traceobj;
+
     _Py_TracingPossible += (func != NULL) - (tstate->c_tracefunc != NULL);
     Py_XINCREF(arg);
     tstate->c_tracefunc = NULL;
@@ -4322,6 +4325,14 @@ PyEval_SetTrace(Py_tracefunc func, PyObject *arg)
     /* Flag that tracing or profiling is turned on */
     tstate->use_tracing = ((func != NULL)
                            || (tstate->c_profilefunc != NULL));
+
+    /* When tracing is not set, ensure that f_lineno is -1 for all the frames
+     * on the stack.
+     * When tracing is set, we rely on f_lineno being valid for all the frames
+     * on the stack for the case where the next opcode would be RETURN_VALUE,
+     * see the last paragraph of Objects/lnotab_notes.txt. */
+    for (f = tstate->frame; f != NULL; f = f->f_back)
+        f->f_lineno = func ? PyCode_Addr2Line(f->f_code, f->f_lasti) : -1;
 }
 
 void
@@ -5083,7 +5094,7 @@ static void
 maybe_dtrace_line(PyFrameObject *frame,
                   int *instr_lb, int *instr_ub, int *instr_prev)
 {
-    int line = frame->f_lineno;
+    int line;
     const char *co_filename, *co_name;
 
     /* If the last instruction executed isn't in the current
@@ -5095,12 +5106,14 @@ maybe_dtrace_line(PyFrameObject *frame,
                                        &bounds);
         *instr_lb = bounds.ap_lower;
         *instr_ub = bounds.ap_upper;
+    } else {
+        line = PyFrame_GetLineNumber(frame);
     }
+
     /* 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 call the trace function. */
     if (frame->f_lasti == *instr_lb || frame->f_lasti < *instr_prev) {
-        frame->f_lineno = line;
         co_filename = PyUnicode_AsUTF8(frame->f_code->co_filename);
         if (!co_filename)
             co_filename = "?";