From 8964e0685159055c5cf92920878e914fb14384ab Mon Sep 17 00:00:00 2001 From: mpage Date: Wed, 4 Jun 2025 16:07:58 -0700 Subject: [PATCH] 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. (cherry picked from commit 6b77af257c25d31f1f137e477cb23e63692ddf29) --- Include/internal/pycore_magic_number.h | 3 +- Lib/test/test_dis.py | 2 +- Lib/test/test_peepholer.py | 92 +++++++++++++++++++ ...-05-30-18-09-54.gh-issue-134889.Ic9UM-.rst | 2 + Python/flowgraph.c | 20 ++++ 5 files changed, 117 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-05-30-18-09-54.gh-issue-134889.Ic9UM-.rst diff --git a/Include/internal/pycore_magic_number.h b/Include/internal/pycore_magic_number.h index 1cc897c5a69469..95754025a4aa05 100644 --- a/Include/internal/pycore_magic_number.h +++ b/Include/internal/pycore_magic_number.h @@ -277,6 +277,7 @@ Known values: Python 3.14a7 3622 (Store annotations in different class dict keys) Python 3.14a7 3623 (Add BUILD_INTERPOLATION & BUILD_TEMPLATE opcodes) Python 3.14b1 3624 (Don't optimize LOAD_FAST when local is killed by DELETE_FAST) + Python 3.14b3 3625 (Fix handling of opcodes that may leave operands on the stack when optimizing LOAD_FAST) Python 3.15 will start with 3650 @@ -289,7 +290,7 @@ PC/launcher.c must also be updated. */ -#define PYC_MAGIC_NUMBER 3624 +#define PYC_MAGIC_NUMBER 3625 /* This is equivalent to converting PYC_MAGIC_NUMBER to 2 bytes (little-endian) and then appending b'\r\n'. */ #define PYC_MAGIC_NUMBER_TOKEN \ diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index ae68c1dd75c641..dbf90472ca34b6 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -606,7 +606,7 @@ async def _asyncwith(c): POP_TOP L1: RESUME 0 -%4d LOAD_FAST_BORROW 0 (c) +%4d LOAD_FAST 0 (c) COPY 1 LOAD_SPECIAL 3 (__aexit__) SWAP 2 diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index 0a9ba578673b39..a4868deadc8882 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -2614,6 +2614,90 @@ def test_send(self): ] 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 @@ def create_obj(): 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__": diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-05-30-18-09-54.gh-issue-134889.Ic9UM-.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-30-18-09-54.gh-issue-134889.Ic9UM-.rst new file mode 100644 index 00000000000000..3b86134bf16800 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-30-18-09-54.gh-issue-134889.Ic9UM-.rst @@ -0,0 +1,2 @@ +Fix handling of a few opcodes that leave operands on the stack when +optimizing ``LOAD_FAST``. diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 78ef02a911a72b..cee3e9bd6355a6 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -2862,8 +2862,10 @@ 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_LEN: + case GET_YIELD_FROM_ITER: case IMPORT_FROM: case MATCH_KEYS: case MATCH_MAPPING: @@ -2898,6 +2900,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); @@ -2927,6 +2939,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);