gh-134889: Fix handling of a few opcodes when optimizing LOAD_FAST (#134958)

We were incorrectly handling a few opcodes that leave their operands on the stack. Treat all of these conservatively; assume that they always leave operands on the stack.
This commit is contained in:
mpage 2025-06-04 16:07:58 -07:00 committed by GitHub
parent e598eecf4c
commit 6b77af257c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 117 additions and 2 deletions

View file

@ -280,6 +280,7 @@ Known values:
Python 3.15a0 3650 (Initial version)
Python 3.15a1 3651 (Simplify LOAD_CONST)
Python 3.15a1 3652 (Virtual iterators)
Python 3.15a1 3653 (Fix handling of opcodes that may leave operands on the stack when optimizing LOAD_FAST)
Python 3.16 will start with 3700
@ -293,7 +294,7 @@ PC/launcher.c must also be updated.
*/
#define PYC_MAGIC_NUMBER 3652
#define PYC_MAGIC_NUMBER 3653
/* This is equivalent to converting PYC_MAGIC_NUMBER to 2 bytes
(little-endian) and then appending b'\r\n'. */
#define PYC_MAGIC_NUMBER_TOKEN \

View file

@ -606,7 +606,7 @@ dis_asyncwith = """\
POP_TOP
L1: RESUME 0
%4d LOAD_FAST_BORROW 0 (c)
%4d LOAD_FAST 0 (c)
COPY 1
LOAD_SPECIAL 3 (__aexit__)
SWAP 2

View file

@ -2614,6 +2614,90 @@ class OptimizeLoadFastTestCase(DirectCfgOptimizerTests):
]
self.cfg_optimization_test(insts, expected, consts=[None])
def test_format_simple(self):
# FORMAT_SIMPLE will leave its operand on the stack if it's a unicode
# object. We treat it conservatively and assume that it always leaves
# its operand on the stack.
insts = [
("LOAD_FAST", 0, 1),
("FORMAT_SIMPLE", None, 2),
("STORE_FAST", 1, 3),
]
self.check(insts, insts)
insts = [
("LOAD_FAST", 0, 1),
("FORMAT_SIMPLE", None, 2),
("POP_TOP", None, 3),
]
expected = [
("LOAD_FAST_BORROW", 0, 1),
("FORMAT_SIMPLE", None, 2),
("POP_TOP", None, 3),
]
self.check(insts, expected)
def test_set_function_attribute(self):
# SET_FUNCTION_ATTRIBUTE leaves the function on the stack
insts = [
("LOAD_CONST", 0, 1),
("LOAD_FAST", 0, 2),
("SET_FUNCTION_ATTRIBUTE", 2, 3),
("STORE_FAST", 1, 4),
("LOAD_CONST", 0, 5),
("RETURN_VALUE", None, 6)
]
self.cfg_optimization_test(insts, insts, consts=[None])
insts = [
("LOAD_CONST", 0, 1),
("LOAD_FAST", 0, 2),
("SET_FUNCTION_ATTRIBUTE", 2, 3),
("RETURN_VALUE", None, 4)
]
expected = [
("LOAD_CONST", 0, 1),
("LOAD_FAST_BORROW", 0, 2),
("SET_FUNCTION_ATTRIBUTE", 2, 3),
("RETURN_VALUE", None, 4)
]
self.cfg_optimization_test(insts, expected, consts=[None])
def test_get_yield_from_iter(self):
# GET_YIELD_FROM_ITER may leave its operand on the stack
insts = [
("LOAD_FAST", 0, 1),
("GET_YIELD_FROM_ITER", None, 2),
("LOAD_CONST", 0, 3),
send := self.Label(),
("SEND", end := self.Label(), 5),
("YIELD_VALUE", 1, 6),
("RESUME", 2, 7),
("JUMP", send, 8),
end,
("END_SEND", None, 9),
("LOAD_CONST", 0, 10),
("RETURN_VALUE", None, 11),
]
self.cfg_optimization_test(insts, insts, consts=[None])
def test_push_exc_info(self):
insts = [
("LOAD_FAST", 0, 1),
("PUSH_EXC_INFO", None, 2),
]
self.check(insts, insts)
def test_load_special(self):
# LOAD_SPECIAL may leave self on the stack
insts = [
("LOAD_FAST", 0, 1),
("LOAD_SPECIAL", 0, 2),
("STORE_FAST", 1, 3),
]
self.check(insts, insts)
def test_del_in_finally(self):
# This loads `obj` onto the stack, executes `del obj`, then returns the
# `obj` from the stack. See gh-133371 for more details.
@ -2630,6 +2714,14 @@ class OptimizeLoadFastTestCase(DirectCfgOptimizerTests):
gc.collect()
self.assertEqual(obj, [42])
def test_format_simple_unicode(self):
# Repro from gh-134889
def f():
var = f"{1}"
var = f"{var}"
return var
self.assertEqual(f(), "1")
if __name__ == "__main__":

View file

@ -0,0 +1,2 @@
Fix handling of a few opcodes that leave operands on the stack when
optimizing ``LOAD_FAST``.

View file

@ -2870,9 +2870,11 @@ optimize_load_fast(cfg_builder *g)
// how many inputs should be left on the stack.
// Opcodes that consume no inputs
case FORMAT_SIMPLE:
case GET_ANEXT:
case GET_ITER:
case GET_LEN:
case GET_YIELD_FROM_ITER:
case IMPORT_FROM:
case MATCH_KEYS:
case MATCH_MAPPING:
@ -2907,6 +2909,16 @@ optimize_load_fast(cfg_builder *g)
break;
}
case END_SEND:
case SET_FUNCTION_ATTRIBUTE: {
assert(_PyOpcode_num_popped(opcode, oparg) == 2);
assert(_PyOpcode_num_pushed(opcode, oparg) == 1);
ref tos = ref_stack_pop(&refs);
ref_stack_pop(&refs);
PUSH_REF(tos.instr, tos.local);
break;
}
// Opcodes that consume some inputs and push new values
case CHECK_EXC_MATCH: {
ref_stack_pop(&refs);
@ -2936,6 +2948,14 @@ optimize_load_fast(cfg_builder *g)
break;
}
case LOAD_SPECIAL:
case PUSH_EXC_INFO: {
ref tos = ref_stack_pop(&refs);
PUSH_REF(i, NOT_LOCAL);
PUSH_REF(tos.instr, tos.local);
break;
}
case SEND: {
load_fast_push_block(&sp, instr->i_target, refs.size);
ref_stack_pop(&refs);