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

This commit is contained in:
Brandt Bucher 2022-11-08 07:50:46 -08:00 committed by GitHub
parent e56e33d271
commit c7065ce019
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 131 additions and 58 deletions

View file

@ -815,12 +815,44 @@ 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
L = 7
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
sys.settrace(trace)
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
@ -832,15 +864,56 @@ 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")
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()

View file

@ -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.

View file

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