GH-93143: Don't turn LOAD_FAST into LOAD_FAST_CHECK

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);