GH-93143: Don't turn LOAD_FAST into LOAD_FAST_CHECK (GH-99075)

Author: brandtbucher

Lib/test/test_peepholer.py

@@ -815,15 +815,15 @@ def f():
         self.assertInBytecode(f, 'LOAD_FAST_CHECK', "a73")
         self.assertInBytecode(f, 'LOAD_FAST', "a73")
 
-    def test_setting_lineno_adds_check(self):
-        code = textwrap.dedent("""\
+    def test_setting_lineno_no_undefined(self):
+        code = textwrap.dedent(f"""\
             def f():
-                x = 2
-                L = 3
-                L = 4
+                x = y = 2
+                if not x:
+                    return 4
                 for i in range(55):
                     x + 6
-                del x
+                L = 7
                 L = 8
                 L = 9
                 L = 10
@@ -832,15 +832,88 @@ def f():
         exec(code, ns)
         f = ns['f']
         self.assertInBytecode(f, "LOAD_FAST")
+        self.assertNotInBytecode(f, "LOAD_FAST_CHECK")
+        co_code = f.__code__.co_code
         def trace(frame, event, arg):
             if event == 'line' and frame.f_lineno == 9:
-                frame.f_lineno = 2
+                frame.f_lineno = 3
                 sys.settrace(None)
                 return None
             return trace
         sys.settrace(trace)
-        f()
-        self.assertNotInBytecode(f, "LOAD_FAST")
+        result = f()
+        self.assertIsNone(result)
+        self.assertInBytecode(f, "LOAD_FAST")
+        self.assertNotInBytecode(f, "LOAD_FAST_CHECK")
+        self.assertEqual(f.__code__.co_code, co_code)
+
+    def test_setting_lineno_one_undefined(self):
+        code = textwrap.dedent(f"""\
+            def f():
+                x = y = 2
+                if not x:
+                    return 4
+                for i in range(55):
+                    x + 6
+                del x
+                L = 8
+                L = 9
+                L = 10
+        """)
+        ns = {}
+        exec(code, ns)
+        f = ns['f']
+        self.assertInBytecode(f, "LOAD_FAST")
+        self.assertNotInBytecode(f, "LOAD_FAST_CHECK")
+        co_code = f.__code__.co_code
+        def trace(frame, event, arg):
+            if event == 'line' and frame.f_lineno == 9:
+                frame.f_lineno = 3
+                sys.settrace(None)
+                return None
+            return trace
+        e = r"assigning None to 1 unbound local"
+        with self.assertWarnsRegex(RuntimeWarning, e):
+            sys.settrace(trace)
+            result = f()
+        self.assertEqual(result, 4)
+        self.assertInBytecode(f, "LOAD_FAST")
+        self.assertNotInBytecode(f, "LOAD_FAST_CHECK")
+        self.assertEqual(f.__code__.co_code, co_code)
+
+    def test_setting_lineno_two_undefined(self):
+        code = textwrap.dedent(f"""\
+            def f():
+                x = y = 2
+                if not x:
+                    return 4
+                for i in range(55):
+                    x + 6
+                del x, y
+                L = 8
+                L = 9
+                L = 10
+        """)
+        ns = {}
+        exec(code, ns)
+        f = ns['f']
+        self.assertInBytecode(f, "LOAD_FAST")
+        self.assertNotInBytecode(f, "LOAD_FAST_CHECK")
+        co_code = f.__code__.co_code
+        def trace(frame, event, arg):
+            if event == 'line' and frame.f_lineno == 9:
+                frame.f_lineno = 3
+                sys.settrace(None)
+                return None
+            return trace
+        e = r"assigning None to 2 unbound locals"
+        with self.assertWarnsRegex(RuntimeWarning, e):
+            sys.settrace(trace)
+            result = f()
+        self.assertEqual(result, 4)
+        self.assertInBytecode(f, "LOAD_FAST")
+        self.assertNotInBytecode(f, "LOAD_FAST_CHECK")
+        self.assertEqual(f.__code__.co_code, co_code)
 
     def make_function_with_no_checks(self):
         code = textwrap.dedent("""\
@@ -860,18 +933,22 @@ def f():
         self.assertNotInBytecode(f, "LOAD_FAST_CHECK")
         return f
 
-    def test_deleting_local_adds_check(self):
+    def test_deleting_local_warns_and_assigns_none(self):
         f = self.make_function_with_no_checks()
+        co_code = f.__code__.co_code
         def trace(frame, event, arg):
             if event == 'line' and frame.f_lineno == 4:
                 del frame.f_locals["x"]
                 sys.settrace(None)
                 return None
             return trace
-        sys.settrace(trace)
-        f()
-        self.assertNotInBytecode(f, "LOAD_FAST")
-        self.assertInBytecode(f, "LOAD_FAST_CHECK")
+        e = r"assigning None to unbound local 'x'"
+        with self.assertWarnsRegex(RuntimeWarning, e):
+            sys.settrace(trace)
+            f()
+        self.assertInBytecode(f, "LOAD_FAST")
+        self.assertNotInBytecode(f, "LOAD_FAST_CHECK")
+        self.assertEqual(f.__code__.co_code, co_code)
 
     def test_modifying_local_does_not_add_check(self):
         f = self.make_function_with_no_checks()

Misc/NEWS.d/next/Core and Builtins/2022-10-19-23-54-43.gh-issue-93143.1wCYub.rst

@@ -0,0 +1,4 @@
+Rather than changing :attr:`~types.CodeType.co_code`, the interpreter will
+now display a :exc:`RuntimeWarning` and assign :const:`None` to any fast
+locals that are left unbound after jumps or :keyword:`del`
+statements executed while tracing.

Objects/frameobject.c

@@ -620,42 +620,6 @@ _PyFrame_GetState(PyFrameObject *frame)
     Py_UNREACHABLE();
 }
 
-static void
-add_load_fast_null_checks(PyCodeObject *co)
-{
-    int changed = 0;
-    _Py_CODEUNIT *instructions = _PyCode_CODE(co);
-    for (Py_ssize_t i = 0; i < Py_SIZE(co); i++) {
-        int opcode = _Py_OPCODE(instructions[i]);
-        switch (opcode) {
-            case LOAD_FAST:
-            case LOAD_FAST__LOAD_FAST:
-            case LOAD_FAST__LOAD_CONST:
-                changed = 1;
-                _Py_SET_OPCODE(instructions[i], LOAD_FAST_CHECK);
-                break;
-            case LOAD_CONST__LOAD_FAST:
-                changed = 1;
-                _Py_SET_OPCODE(instructions[i], LOAD_CONST);
-                break;
-            case STORE_FAST__LOAD_FAST:
-                changed = 1;
-                _Py_SET_OPCODE(instructions[i], STORE_FAST);
-                break;
-        }
-        i += _PyOpcode_Caches[_PyOpcode_Deopt[opcode]];
-    }
-    if (changed && co->_co_cached != NULL) {
-        // invalidate cached co_code object
-        Py_CLEAR(co->_co_cached->_co_code);
-        Py_CLEAR(co->_co_cached->_co_cellvars);
-        Py_CLEAR(co->_co_cached->_co_freevars);
-        Py_CLEAR(co->_co_cached->_co_varnames);
-        PyMem_Free(co->_co_cached);
-        co->_co_cached = NULL;
-    }
-}
-
 /* Setter for f_lineno - you can set f_lineno from within a trace function in
  * order to jump to a given line of code, subject to some restrictions.  Most
  * lines are OK to jump to because they don't make any assumptions about the
@@ -745,8 +709,6 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
         return -1;
     }
 
-    add_load_fast_null_checks(f->f_frame->f_code);
-
     /* PyCode_NewWithPosOnlyArgs limits co_code to be under INT_MAX so this
      * should never overflow. */
     int len = (int)Py_SIZE(f->f_frame->f_code);
@@ -805,6 +767,31 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
         PyErr_SetString(PyExc_ValueError, msg);
         return -1;
     }
+    // Populate any NULL locals that the compiler might have "proven" to exist
+    // in the new location. Rather than crashing or changing co_code, just bind
+    // None instead:
+    int unbound = 0;
+    for (int i = 0; i < f->f_frame->f_code->co_nlocalsplus; i++) {
+        // Counting every unbound local is overly-cautious, but a full flow
+        // analysis (like we do in the compiler) is probably too expensive:
+        unbound += f->f_frame->localsplus[i] == NULL;
+    }
+    if (unbound) {
+        const char *e = "assigning None to %d unbound local%s";
+        const char *s = (unbound == 1) ? "" : "s";
+        if (PyErr_WarnFormat(PyExc_RuntimeWarning, 0, e, unbound, s)) {
+            return -1;
+        }
+        // Do this in a second pass to avoid writing a bunch of Nones when
+        // warnings are being treated as errors and the previous bit raises:
+        for (int i = 0; i < f->f_frame->f_code->co_nlocalsplus; i++) {
+            if (f->f_frame->localsplus[i] == NULL) {
+                f->f_frame->localsplus[i] = Py_NewRef(Py_None);
+                unbound--;
+            }
+        }
+        assert(unbound == 0);
+    }
     if (state == FRAME_SUSPENDED) {
         /* Account for value popped by yield */
         start_stack = pop_value(start_stack);
@@ -1269,7 +1256,6 @@ _PyFrame_LocalsToFast(_PyInterpreterFrame *frame, int clear)
     }
     fast = _PyFrame_GetLocalsArray(frame);
     co = frame->f_code;
-    bool added_null_checks = false;
 
     PyErr_Fetch(&error_type, &error_value, &error_traceback);
     for (int i = 0; i < co->co_nlocalsplus; i++) {
@@ -1289,10 +1275,6 @@ _PyFrame_LocalsToFast(_PyInterpreterFrame *frame, int clear)
             }
         }
         PyObject *oldvalue = fast[i];
-        if (!added_null_checks && oldvalue != NULL && value == NULL) {
-            add_load_fast_null_checks(co);
-            added_null_checks = true;
-        }
         PyObject *cell = NULL;
         if (kind == CO_FAST_FREE) {
             // The cell was set when the frame was created from
@@ -1319,7 +1301,17 @@ _PyFrame_LocalsToFast(_PyInterpreterFrame *frame, int clear)
             }
         }
         else if (value != oldvalue) {
-            Py_XINCREF(value);
+            if (value == NULL) {
+                // Probably can't delete this, since the compiler's flow
+                // analysis may have already "proven" that it exists here:
+                const char *e = "assigning None to unbound local %R";
+                if (PyErr_WarnFormat(PyExc_RuntimeWarning, 0, e, name)) {
+                    // It's okay if frame_obj is NULL, just try anyways:
+                    PyErr_WriteUnraisable((PyObject *)frame->frame_obj);
+                }
+                value = Py_NewRef(Py_None);
+            }
+            Py_INCREF(value);
             Py_XSETREF(fast[i], value);
         }
         Py_XDECREF(value);