From 59b5886c9a5954a67df071aa231ad0af01d94a12 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 30 May 2025 10:09:02 -0700 Subject: [PATCH 1/7] Fix handling of `FORMAT_SIMPLE` when optimizing `LOAD_FAST` `FORMAT_SIMPLE` leaves its operand on the stack if its a unicode object. Since we cannot determine that at analysis (compile) time, treat it conservatively and assume it always leaves the operand on the stack. --- Include/internal/pycore_magic_number.h | 1 + Lib/test/test_peepholer.py | 31 ++++++++++++++++++++++++++ Python/flowgraph.c | 1 + 3 files changed, 33 insertions(+) diff --git a/Include/internal/pycore_magic_number.h b/Include/internal/pycore_magic_number.h index cd1fc873623ed1..bc63549eaa6f9a 100644 --- a/Include/internal/pycore_magic_number.h +++ b/Include/internal/pycore_magic_number.h @@ -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 (Treat FORMAT_SIMPLE conservatively when optimizing LOAD_FAST) Python 3.16 will start with 3700 diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index f33de3d420ca34..95d12d9ee0d0b0 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -2614,6 +2614,29 @@ 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_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 +2653,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/Python/flowgraph.c b/Python/flowgraph.c index 67ccf350b72ed6..02a4038ffef391 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -2870,6 +2870,7 @@ 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: From 220bbf929312ff12b01bd964e1d9a06f6f952f18 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 30 May 2025 16:06:28 -0700 Subject: [PATCH 2/7] Handle SET_FUNCTION_ATTRIBUTE correctly It leaves the function on the stack --- Lib/test/test_peepholer.py | 26 ++++++++++++++++++++++++++ Python/flowgraph.c | 10 ++++++++++ 2 files changed, 36 insertions(+) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index 95d12d9ee0d0b0..0f32605ce136c5 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -2637,6 +2637,32 @@ def test_format_simple(self): ] 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_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. diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 02a4038ffef391..3838e0b92f1761 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -2944,6 +2944,16 @@ optimize_load_fast(cfg_builder *g) break; } + case SET_FUNCTION_ATTRIBUTE: { + assert(_PyOpcode_num_popped(opcode, oparg) == 2); + assert(_PyOpcode_num_pushed(opcode, oparg) == 1); + ref func = ref_stack_pop(&refs); + // Pop attr + ref_stack_pop(&refs); + PUSH_REF(func.instr, func.local); + break; + } + // Opcodes that consume all of their inputs default: { int num_popped = _PyOpcode_num_popped(opcode, oparg); From 58db93d560d23d1e774cde4b4aaa96696dcb5df4 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 30 May 2025 17:24:22 -0700 Subject: [PATCH 3/7] Handle `GET_YIELD_FROM_ITER` and `END_SEND` correctly `GET_YIELD_FROM_ITER` doesn't consume its operand if its a coroutine or generator, so treat it conservatively. `END_SEND` leaves the TOS. --- Lib/test/test_peepholer.py | 18 ++++++++++++++++++ Python/flowgraph.c | 21 +++++++++++---------- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index 0f32605ce136c5..d5d11131b54aae 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -2663,6 +2663,24 @@ def test_set_function_attribute(self): ] 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_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. diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 3838e0b92f1761..f4094463d5df3c 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -2874,6 +2874,7 @@ optimize_load_fast(cfg_builder *g) case GET_ANEXT: case GET_ITER: case GET_LEN: + case GET_YIELD_FROM_ITER: case IMPORT_FROM: case MATCH_KEYS: case MATCH_MAPPING: @@ -2908,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); @@ -2944,16 +2955,6 @@ optimize_load_fast(cfg_builder *g) break; } - case SET_FUNCTION_ATTRIBUTE: { - assert(_PyOpcode_num_popped(opcode, oparg) == 2); - assert(_PyOpcode_num_pushed(opcode, oparg) == 1); - ref func = ref_stack_pop(&refs); - // Pop attr - ref_stack_pop(&refs); - PUSH_REF(func.instr, func.local); - break; - } - // Opcodes that consume all of their inputs default: { int num_popped = _PyOpcode_num_popped(opcode, oparg); From 3e6aa3fcfba6a8dfd863b9243a97cb8ae40f9817 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 30 May 2025 17:32:26 -0700 Subject: [PATCH 4/7] Handle `PUSH_EXC_INFO` correctly It leaves the TOS in place. --- Lib/test/test_peepholer.py | 7 +++++++ Python/flowgraph.c | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/Lib/test/test_peepholer.py b/Lib/test/test_peepholer.py index d5d11131b54aae..d990d0c84cb292 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -2681,6 +2681,13 @@ def test_get_yield_from_iter(self): ] 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_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. diff --git a/Python/flowgraph.c b/Python/flowgraph.c index f4094463d5df3c..050232e86035fa 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -2948,6 +2948,13 @@ optimize_load_fast(cfg_builder *g) break; } + 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); From a60551b585a65c680a3b08c55f3bdbbd7c96430c Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 30 May 2025 18:07:11 -0700 Subject: [PATCH 5/7] Fix handling of `LOAD_SPECIAL` Load special may leave `self` on TOS. Treat it conservatively and assume it always does. --- Lib/test/test_dis.py | 2 +- Lib/test/test_peepholer.py | 10 ++++++++++ Python/flowgraph.c | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index ec930a728aa5b3..355990ed58ee09 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 d990d0c84cb292..ef596630b930f7 100644 --- a/Lib/test/test_peepholer.py +++ b/Lib/test/test_peepholer.py @@ -2688,6 +2688,16 @@ def test_push_exc_info(self): ] 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. diff --git a/Python/flowgraph.c b/Python/flowgraph.c index 050232e86035fa..2adc8c84d83974 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -2948,6 +2948,7 @@ 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); From 0383f88a2890c9c5dc7b75d978868944e8b96337 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 30 May 2025 18:10:01 -0700 Subject: [PATCH 6/7] Add blurb --- .../2025-05-30-18-09-54.gh-issue-134889.Ic9UM-.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-05-30-18-09-54.gh-issue-134889.Ic9UM-.rst 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``. From 84ad79670cb2884f751a0f3ae993cdf6add3e669 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Fri, 30 May 2025 18:16:51 -0700 Subject: [PATCH 7/7] Actually bump the magic number, don't just write about it --- Include/internal/pycore_magic_number.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_magic_number.h b/Include/internal/pycore_magic_number.h index bc63549eaa6f9a..347d9762f26bff 100644 --- a/Include/internal/pycore_magic_number.h +++ b/Include/internal/pycore_magic_number.h @@ -280,7 +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 (Treat FORMAT_SIMPLE conservatively when optimizing LOAD_FAST) + 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 @@ -294,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 \