From e6a91ae4464b4891c7414a66d3fec5c5f79a5484 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 4 Mar 2025 14:34:02 +0000 Subject: [PATCH 1/8] Fix branch monitoring for . Both branches in a pair now have a common source and are included in co_branches --- Include/internal/pycore_opcode_metadata.h | 4 +- Include/opcode_ids.h | 118 +++++++++++----------- Lib/_opcode_metadata.py | 118 +++++++++++----------- Lib/test/test_code.py | 9 ++ Lib/test/test_monitoring.py | 37 +++++-- Python/assemble.c | 10 +- Python/bytecodes.c | 8 +- Python/codegen.c | 9 +- Python/flowgraph.c | 2 +- Python/generated_cases.c.h | 9 +- Python/instruction_sequence.c | 1 + Python/instrumentation.c | 8 ++ Python/opcode_targets.h | 4 +- 13 files changed, 197 insertions(+), 140 deletions(-) diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index b6d85490eef1f3..2cd57140747dec 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -2084,7 +2084,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[266] = { [DELETE_SUBSCR] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [DICT_MERGE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [DICT_UPDATE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, - [END_ASYNC_FOR] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, + [END_ASYNC_FOR] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, [END_FOR] = { true, INSTR_FMT_IX, HAS_ESCAPES_FLAG | HAS_NO_SAVE_IP_FLAG }, [END_SEND] = { true, INSTR_FMT_IX, HAS_ESCAPES_FLAG | HAS_PURE_FLAG }, [ENTER_EXECUTOR] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, @@ -2108,7 +2108,7 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[266] = { [INSTRUMENTED_CALL] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_EVAL_BREAK_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, [INSTRUMENTED_CALL_FUNCTION_EX] = { true, INSTR_FMT_IX, HAS_EVAL_BREAK_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, [INSTRUMENTED_CALL_KW] = { true, INSTR_FMT_IBC00, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, - [INSTRUMENTED_END_ASYNC_FOR] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, + [INSTRUMENTED_END_ASYNC_FOR] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, [INSTRUMENTED_END_FOR] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG | HAS_NO_SAVE_IP_FLAG }, [INSTRUMENTED_END_SEND] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, [INSTRUMENTED_FOR_ITER] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_JUMP_FLAG | HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG }, diff --git a/Include/opcode_ids.h b/Include/opcode_ids.h index a634d5e5a229c8..51b549b108ab7a 100644 --- a/Include/opcode_ids.h +++ b/Include/opcode_ids.h @@ -18,65 +18,65 @@ extern "C" { #define CHECK_EXC_MATCH 5 #define CLEANUP_THROW 6 #define DELETE_SUBSCR 7 -#define END_ASYNC_FOR 8 -#define END_FOR 9 -#define END_SEND 10 -#define EXIT_INIT_CHECK 11 -#define FORMAT_SIMPLE 12 -#define FORMAT_WITH_SPEC 13 -#define GET_AITER 14 -#define GET_ANEXT 15 -#define GET_ITER 16 +#define END_FOR 8 +#define END_SEND 9 +#define EXIT_INIT_CHECK 10 +#define FORMAT_SIMPLE 11 +#define FORMAT_WITH_SPEC 12 +#define GET_AITER 13 +#define GET_ANEXT 14 +#define GET_ITER 15 +#define GET_LEN 16 #define RESERVED 17 -#define GET_LEN 18 -#define GET_YIELD_FROM_ITER 19 -#define INTERPRETER_EXIT 20 -#define LOAD_BUILD_CLASS 21 -#define LOAD_LOCALS 22 -#define MAKE_FUNCTION 23 -#define MATCH_KEYS 24 -#define MATCH_MAPPING 25 -#define MATCH_SEQUENCE 26 -#define NOP 27 -#define NOT_TAKEN 28 -#define POP_EXCEPT 29 -#define POP_ITER 30 -#define POP_TOP 31 -#define PUSH_EXC_INFO 32 -#define PUSH_NULL 33 -#define RETURN_GENERATOR 34 -#define RETURN_VALUE 35 -#define SETUP_ANNOTATIONS 36 -#define STORE_SLICE 37 -#define STORE_SUBSCR 38 -#define TO_BOOL 39 -#define UNARY_INVERT 40 -#define UNARY_NEGATIVE 41 -#define UNARY_NOT 42 -#define WITH_EXCEPT_START 43 -#define BINARY_OP 44 -#define BUILD_LIST 45 -#define BUILD_MAP 46 -#define BUILD_SET 47 -#define BUILD_SLICE 48 -#define BUILD_STRING 49 -#define BUILD_TUPLE 50 -#define CALL 51 -#define CALL_INTRINSIC_1 52 -#define CALL_INTRINSIC_2 53 -#define CALL_KW 54 -#define COMPARE_OP 55 -#define CONTAINS_OP 56 -#define CONVERT_VALUE 57 -#define COPY 58 -#define COPY_FREE_VARS 59 -#define DELETE_ATTR 60 -#define DELETE_DEREF 61 -#define DELETE_FAST 62 -#define DELETE_GLOBAL 63 -#define DELETE_NAME 64 -#define DICT_MERGE 65 -#define DICT_UPDATE 66 +#define GET_YIELD_FROM_ITER 18 +#define INTERPRETER_EXIT 19 +#define LOAD_BUILD_CLASS 20 +#define LOAD_LOCALS 21 +#define MAKE_FUNCTION 22 +#define MATCH_KEYS 23 +#define MATCH_MAPPING 24 +#define MATCH_SEQUENCE 25 +#define NOP 26 +#define NOT_TAKEN 27 +#define POP_EXCEPT 28 +#define POP_ITER 29 +#define POP_TOP 30 +#define PUSH_EXC_INFO 31 +#define PUSH_NULL 32 +#define RETURN_GENERATOR 33 +#define RETURN_VALUE 34 +#define SETUP_ANNOTATIONS 35 +#define STORE_SLICE 36 +#define STORE_SUBSCR 37 +#define TO_BOOL 38 +#define UNARY_INVERT 39 +#define UNARY_NEGATIVE 40 +#define UNARY_NOT 41 +#define WITH_EXCEPT_START 42 +#define BINARY_OP 43 +#define BUILD_LIST 44 +#define BUILD_MAP 45 +#define BUILD_SET 46 +#define BUILD_SLICE 47 +#define BUILD_STRING 48 +#define BUILD_TUPLE 49 +#define CALL 50 +#define CALL_INTRINSIC_1 51 +#define CALL_INTRINSIC_2 52 +#define CALL_KW 53 +#define COMPARE_OP 54 +#define CONTAINS_OP 55 +#define CONVERT_VALUE 56 +#define COPY 57 +#define COPY_FREE_VARS 58 +#define DELETE_ATTR 59 +#define DELETE_DEREF 60 +#define DELETE_FAST 61 +#define DELETE_GLOBAL 62 +#define DELETE_NAME 63 +#define DICT_MERGE 64 +#define DICT_UPDATE 65 +#define END_ASYNC_FOR 66 #define EXTENDED_ARG 67 #define FOR_ITER 68 #define GET_AWAITABLE 69 @@ -243,7 +243,7 @@ extern "C" { #define SETUP_WITH 264 #define STORE_FAST_MAYBE_NULL 265 -#define HAVE_ARGUMENT 43 +#define HAVE_ARGUMENT 42 #define MIN_SPECIALIZED_OPCODE 150 #define MIN_INSTRUMENTED_OPCODE 234 diff --git a/Lib/_opcode_metadata.py b/Lib/_opcode_metadata.py index 3dc69635cba39e..d6265f34a59a1a 100644 --- a/Lib/_opcode_metadata.py +++ b/Lib/_opcode_metadata.py @@ -220,64 +220,64 @@ 'CHECK_EXC_MATCH': 5, 'CLEANUP_THROW': 6, 'DELETE_SUBSCR': 7, - 'END_ASYNC_FOR': 8, - 'END_FOR': 9, - 'END_SEND': 10, - 'EXIT_INIT_CHECK': 11, - 'FORMAT_SIMPLE': 12, - 'FORMAT_WITH_SPEC': 13, - 'GET_AITER': 14, - 'GET_ANEXT': 15, - 'GET_ITER': 16, - 'GET_LEN': 18, - 'GET_YIELD_FROM_ITER': 19, - 'INTERPRETER_EXIT': 20, - 'LOAD_BUILD_CLASS': 21, - 'LOAD_LOCALS': 22, - 'MAKE_FUNCTION': 23, - 'MATCH_KEYS': 24, - 'MATCH_MAPPING': 25, - 'MATCH_SEQUENCE': 26, - 'NOP': 27, - 'NOT_TAKEN': 28, - 'POP_EXCEPT': 29, - 'POP_ITER': 30, - 'POP_TOP': 31, - 'PUSH_EXC_INFO': 32, - 'PUSH_NULL': 33, - 'RETURN_GENERATOR': 34, - 'RETURN_VALUE': 35, - 'SETUP_ANNOTATIONS': 36, - 'STORE_SLICE': 37, - 'STORE_SUBSCR': 38, - 'TO_BOOL': 39, - 'UNARY_INVERT': 40, - 'UNARY_NEGATIVE': 41, - 'UNARY_NOT': 42, - 'WITH_EXCEPT_START': 43, - 'BINARY_OP': 44, - 'BUILD_LIST': 45, - 'BUILD_MAP': 46, - 'BUILD_SET': 47, - 'BUILD_SLICE': 48, - 'BUILD_STRING': 49, - 'BUILD_TUPLE': 50, - 'CALL': 51, - 'CALL_INTRINSIC_1': 52, - 'CALL_INTRINSIC_2': 53, - 'CALL_KW': 54, - 'COMPARE_OP': 55, - 'CONTAINS_OP': 56, - 'CONVERT_VALUE': 57, - 'COPY': 58, - 'COPY_FREE_VARS': 59, - 'DELETE_ATTR': 60, - 'DELETE_DEREF': 61, - 'DELETE_FAST': 62, - 'DELETE_GLOBAL': 63, - 'DELETE_NAME': 64, - 'DICT_MERGE': 65, - 'DICT_UPDATE': 66, + 'END_FOR': 8, + 'END_SEND': 9, + 'EXIT_INIT_CHECK': 10, + 'FORMAT_SIMPLE': 11, + 'FORMAT_WITH_SPEC': 12, + 'GET_AITER': 13, + 'GET_ANEXT': 14, + 'GET_ITER': 15, + 'GET_LEN': 16, + 'GET_YIELD_FROM_ITER': 18, + 'INTERPRETER_EXIT': 19, + 'LOAD_BUILD_CLASS': 20, + 'LOAD_LOCALS': 21, + 'MAKE_FUNCTION': 22, + 'MATCH_KEYS': 23, + 'MATCH_MAPPING': 24, + 'MATCH_SEQUENCE': 25, + 'NOP': 26, + 'NOT_TAKEN': 27, + 'POP_EXCEPT': 28, + 'POP_ITER': 29, + 'POP_TOP': 30, + 'PUSH_EXC_INFO': 31, + 'PUSH_NULL': 32, + 'RETURN_GENERATOR': 33, + 'RETURN_VALUE': 34, + 'SETUP_ANNOTATIONS': 35, + 'STORE_SLICE': 36, + 'STORE_SUBSCR': 37, + 'TO_BOOL': 38, + 'UNARY_INVERT': 39, + 'UNARY_NEGATIVE': 40, + 'UNARY_NOT': 41, + 'WITH_EXCEPT_START': 42, + 'BINARY_OP': 43, + 'BUILD_LIST': 44, + 'BUILD_MAP': 45, + 'BUILD_SET': 46, + 'BUILD_SLICE': 47, + 'BUILD_STRING': 48, + 'BUILD_TUPLE': 49, + 'CALL': 50, + 'CALL_INTRINSIC_1': 51, + 'CALL_INTRINSIC_2': 52, + 'CALL_KW': 53, + 'COMPARE_OP': 54, + 'CONTAINS_OP': 55, + 'CONVERT_VALUE': 56, + 'COPY': 57, + 'COPY_FREE_VARS': 58, + 'DELETE_ATTR': 59, + 'DELETE_DEREF': 60, + 'DELETE_FAST': 61, + 'DELETE_GLOBAL': 62, + 'DELETE_NAME': 63, + 'DICT_MERGE': 64, + 'DICT_UPDATE': 65, + 'END_ASYNC_FOR': 66, 'EXTENDED_ARG': 67, 'FOR_ITER': 68, 'GET_AWAITABLE': 69, @@ -360,5 +360,5 @@ 'STORE_FAST_MAYBE_NULL': 265, } -HAVE_ARGUMENT = 43 +HAVE_ARGUMENT = 42 MIN_INSTRUMENTED_OPCODE = 234 diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index 69c1ee0690d269..5a94794dabd23d 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -936,6 +936,15 @@ def with_extended_args(x): get_line_branches(with_extended_args), [(1,2,8)]) + async def afunc(): + async for letter in async_iter1: + 2 + 3 + + self.assertEqual( + get_line_branches(afunc), + [(1,2,3)]) + if check_impl_detail(cpython=True) and ctypes is not None: py = ctypes.pythonapi freefunc = ctypes.CFUNCTYPE(None,ctypes.c_voidp) diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py index 305ba6fdeed96b..cb6a8cacab0e3c 100644 --- a/Lib/test/test_monitoring.py +++ b/Lib/test/test_monitoring.py @@ -1683,7 +1683,9 @@ async def foo(): class TestBranchConsistency(MonitoringTestBase, unittest.TestCase): - def check_branches(self, func, tool=TEST_TOOL, recorders=BRANCH_OFFSET_RECORDERS): + def check_branches(self, run_func, test_func=None, tool=TEST_TOOL, recorders=BRANCH_OFFSET_RECORDERS): + if test_func is None: + test_func = run_func try: self.assertEqual(sys.monitoring._all_events(), {}) event_list = [] @@ -1692,25 +1694,26 @@ def check_branches(self, func, tool=TEST_TOOL, recorders=BRANCH_OFFSET_RECORDERS ev = recorder.event_type sys.monitoring.register_callback(tool, ev, recorder(event_list)) all_events |= ev - sys.monitoring.set_local_events(tool, func.__code__, all_events) - func() - sys.monitoring.set_local_events(tool, func.__code__, 0) + sys.monitoring.set_local_events(tool, test_func.__code__, all_events) + run_func() + sys.monitoring.set_local_events(tool, test_func.__code__, 0) for recorder in recorders: sys.monitoring.register_callback(tool, recorder.event_type, None) lefts = set() rights = set() - for (src, left, right) in func.__code__.co_branches(): + for (src, left, right) in test_func.__code__.co_branches(): lefts.add((src, left)) rights.add((src, right)) + print(event_list) for event in event_list: - way, _, src, dest = event + way, name, src, dest = event if "left" in way: self.assertIn((src, dest), lefts) else: self.assertIn("right", way) self.assertIn((src, dest), rights) finally: - sys.monitoring.set_local_events(tool, func.__code__, 0) + sys.monitoring.set_local_events(tool, test_func.__code__, 0) for recorder in recorders: sys.monitoring.register_callback(tool, recorder.event_type, None) @@ -1762,6 +1765,26 @@ def foo(n=0): self.check_branches(foo) + def test_async_for(self): + + async def gen(): + yield 2 + yield 3 + + async def foo(): + async for y in gen(): + 2 + pass # line 3 + + def func(): + try: + foo().send(None) + except StopIteration: + pass + + print(list(foo.__code__.co_branches())) + self.check_branches(func, foo) + class TestLoadSuperAttr(CheckEvents): RECORDERS = CallRecorder, LineRecorder, CRaiseRecorder, CReturnRecorder diff --git a/Python/assemble.c b/Python/assemble.c index 070be1ca54e3ea..c9abcfcca58d20 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -632,6 +632,10 @@ makecode(_PyCompile_CodeUnitMetadata *umd, struct assembler *a, PyObject *const_ return co; } + +// The offset (in code units) of the END_SEND from the SEND in the `yield from` sequence. +#define END_SEND_OFFSET 5 + static int resolve_jump_offsets(instr_sequence *instrs) { @@ -670,7 +674,11 @@ resolve_jump_offsets(instr_sequence *instrs) if (OPCODE_HAS_JUMP(instr->i_opcode)) { instruction *target = &instrs->s_instrs[instr->i_target]; instr->i_oparg = target->i_offset; - if (instr->i_oparg < offset) { + if (instr->i_opcode == END_ASYNC_FOR) { + // Adjust the offset so that target is the END_SEND + instr->i_oparg = offset - instr->i_oparg - END_SEND_OFFSET; + } + else if (instr->i_oparg < offset) { assert(IS_BACKWARDS_JUMP_OPCODE(instr->i_opcode)); instr->i_oparg = offset - instr->i_oparg; } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 24aa7bbb87c193..6f4fc78b883370 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1340,6 +1340,8 @@ dummy_func( } tier1 op(_END_ASYNC_FOR, (awaitable_st, exc_st -- )) { + JUMPBY(0); // Pretend jump as we need source offset for monitoring + (void)oparg; PyObject *exc = PyStackRef_AsPyObjectBorrow(exc_st); assert(exc && PyExceptionInstance_Check(exc)); @@ -1355,12 +1357,12 @@ dummy_func( } } - tier1 op(_MONITOR_BRANCH_RIGHT, ( -- )) { - INSTRUMENTED_JUMP(prev_instr, this_instr+1, PY_MONITORING_EVENT_BRANCH_RIGHT); + tier1 op(_MONITOR_END_ASYNC_FOR, ( -- )) { + INSTRUMENTED_JUMP(next_instr-oparg, this_instr+1, PY_MONITORING_EVENT_BRANCH_RIGHT); } macro(INSTRUMENTED_END_ASYNC_FOR) = - _MONITOR_BRANCH_RIGHT + + _MONITOR_END_ASYNC_FOR + _END_ASYNC_FOR; macro(END_ASYNC_FOR) = _END_ASYNC_FOR; diff --git a/Python/codegen.c b/Python/codegen.c index 8f1a2983007ce4..7a3f787aec0d2d 100644 --- a/Python/codegen.c +++ b/Python/codegen.c @@ -2019,13 +2019,13 @@ codegen_for(compiler *c, stmt_ty s) return SUCCESS; } - static int codegen_async_for(compiler *c, stmt_ty s) { location loc = LOC(s); NEW_JUMP_TARGET_LABEL(c, start); + NEW_JUMP_TARGET_LABEL(c, send); NEW_JUMP_TARGET_LABEL(c, except); NEW_JUMP_TARGET_LABEL(c, end); @@ -2039,6 +2039,7 @@ codegen_async_for(compiler *c, stmt_ty s) ADDOP_JUMP(c, loc, SETUP_FINALLY, except); ADDOP(c, loc, GET_ANEXT); ADDOP_LOAD_CONST(c, loc, Py_None); + USE_LABEL(c, send); ADD_YIELD_FROM(c, loc, 1); ADDOP(c, loc, POP_BLOCK); /* for SETUP_FINALLY */ ADDOP(c, loc, NOT_TAKEN); @@ -2057,7 +2058,7 @@ codegen_async_for(compiler *c, stmt_ty s) /* Use same line number as the iterator, * as the END_ASYNC_FOR succeeds the `for`, not the body. */ loc = LOC(s->v.AsyncFor.iter); - ADDOP(c, loc, END_ASYNC_FOR); + ADDOP_JUMP(c, loc, END_ASYNC_FOR, send); /* `else` block */ VISIT_SEQ(c, stmt, s->v.AsyncFor.orelse); @@ -4252,6 +4253,7 @@ codegen_async_comprehension_generator(compiler *c, location loc, int iter_on_stack) { NEW_JUMP_TARGET_LABEL(c, start); + NEW_JUMP_TARGET_LABEL(c, send); NEW_JUMP_TARGET_LABEL(c, except); NEW_JUMP_TARGET_LABEL(c, if_cleanup); @@ -4279,6 +4281,7 @@ codegen_async_comprehension_generator(compiler *c, location loc, ADDOP_JUMP(c, loc, SETUP_FINALLY, except); ADDOP(c, loc, GET_ANEXT); ADDOP_LOAD_CONST(c, loc, Py_None); + USE_LABEL(c, send); ADD_YIELD_FROM(c, loc, 1); ADDOP(c, loc, POP_BLOCK); VISIT(c, expr, gen->target); @@ -4338,7 +4341,7 @@ codegen_async_comprehension_generator(compiler *c, location loc, USE_LABEL(c, except); - ADDOP(c, loc, END_ASYNC_FOR); + ADDOP_JUMP(c, loc, END_ASYNC_FOR, send); return SUCCESS; } diff --git a/Python/flowgraph.c b/Python/flowgraph.c index fb3c73a059a589..6ba60d4312e56c 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -849,7 +849,7 @@ calculate_stackdepth(cfg_builder *g) goto error; } maxdepth = Py_MAX(maxdepth, depth + effects.max); - if (HAS_TARGET(instr->i_opcode)) { + if (HAS_TARGET(instr->i_opcode) && instr->i_opcode != END_ASYNC_FOR) { if (get_stack_effects(instr->i_opcode, instr->i_oparg, 1, &effects) < 0) { PyErr_Format(PyExc_SystemError, "Invalid stack effect for opcode=%d, arg=%i", diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 8c3c0e3910b8d1..3561d893d78ec0 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5181,6 +5181,8 @@ _PyStackRef exc_st; exc_st = stack_pointer[-1]; awaitable_st = stack_pointer[-2]; + JUMPBY(0); // Pretend jump as we need source offset for monitoring + (void)oparg; PyObject *exc = PyStackRef_AsPyObjectBorrow(exc_st); assert(exc && PyExceptionInstance_Check(exc)); _PyFrame_SetStackPointer(frame, stack_pointer); @@ -6605,7 +6607,6 @@ int opcode = INSTRUMENTED_END_ASYNC_FOR; (void)(opcode); #endif - _Py_CODEUNIT* const prev_instr = frame->instr_ptr; _Py_CODEUNIT* const this_instr = next_instr; (void)this_instr; frame->instr_ptr = next_instr; @@ -6613,14 +6614,16 @@ INSTRUCTION_STATS(INSTRUMENTED_END_ASYNC_FOR); _PyStackRef awaitable_st; _PyStackRef exc_st; - // _MONITOR_BRANCH_RIGHT + // _MONITOR_END_ASYNC_FOR { - INSTRUMENTED_JUMP(prev_instr, this_instr+1, PY_MONITORING_EVENT_BRANCH_RIGHT); + INSTRUMENTED_JUMP(next_instr-oparg, this_instr+1, PY_MONITORING_EVENT_BRANCH_RIGHT); } // _END_ASYNC_FOR { exc_st = stack_pointer[-1]; awaitable_st = stack_pointer[-2]; + JUMPBY(0); // Pretend jump as we need source offset for monitoring + (void)oparg; PyObject *exc = PyStackRef_AsPyObjectBorrow(exc_st); assert(exc && PyExceptionInstance_Check(exc)); _PyFrame_SetStackPointer(frame, stack_pointer); diff --git a/Python/instruction_sequence.c b/Python/instruction_sequence.c index ed40c06715f1f3..bba701033e2891 100644 --- a/Python/instruction_sequence.c +++ b/Python/instruction_sequence.c @@ -29,6 +29,7 @@ typedef _Py_SourceLocation location; #define RETURN_IF_ERROR(X) \ if ((X) == -1) { \ + assert(PyErr_Occurred()); \ return ERROR; \ } diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 4e7ca808b3c3e6..f871d1949db322 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -3109,6 +3109,14 @@ branchesiter_next(branchesiterator *bi) int not_taken = next_offset + 1; bi->bi_offset = not_taken; return int_triple(offset*2, not_taken*2, (next_offset + oparg)*2); + case END_ASYNC_FOR: + oparg = (oparg << 8) | inst.op.arg; + int src_offset = next_offset - oparg; + bi->bi_offset = next_offset; + assert(_Py_GetBaseCodeUnit(bi->bi_code, src_offset).op.code == END_SEND); + assert(_Py_GetBaseCodeUnit(bi->bi_code, src_offset+1).op.code == NOT_TAKEN); + not_taken = src_offset + 2; + return int_triple(src_offset *2, not_taken*2, next_offset*2); default: oparg = 0; } diff --git a/Python/opcode_targets.h b/Python/opcode_targets.h index 0435d0841dbae1..6bf5214bb11776 100644 --- a/Python/opcode_targets.h +++ b/Python/opcode_targets.h @@ -8,7 +8,6 @@ static void *opcode_targets[256] = { &&TARGET_CHECK_EXC_MATCH, &&TARGET_CLEANUP_THROW, &&TARGET_DELETE_SUBSCR, - &&TARGET_END_ASYNC_FOR, &&TARGET_END_FOR, &&TARGET_END_SEND, &&TARGET_EXIT_INIT_CHECK, @@ -17,8 +16,8 @@ static void *opcode_targets[256] = { &&TARGET_GET_AITER, &&TARGET_GET_ANEXT, &&TARGET_GET_ITER, - &&TARGET_RESERVED, &&TARGET_GET_LEN, + &&TARGET_RESERVED, &&TARGET_GET_YIELD_FROM_ITER, &&TARGET_INTERPRETER_EXIT, &&TARGET_LOAD_BUILD_CLASS, @@ -67,6 +66,7 @@ static void *opcode_targets[256] = { &&TARGET_DELETE_NAME, &&TARGET_DICT_MERGE, &&TARGET_DICT_UPDATE, + &&TARGET_END_ASYNC_FOR, &&TARGET_EXTENDED_ARG, &&TARGET_FOR_ITER, &&TARGET_GET_AWAITABLE, From f8eb10a2160ebd847da76c051e2232e821387a97 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 4 Mar 2025 15:15:27 +0000 Subject: [PATCH 2/8] Bump magic and add news --- Include/internal/pycore_magic_number.h | 3 ++- .../2025-03-04-15-12-32.gh-issue-128534.3A0K3D.rst | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-03-04-15-12-32.gh-issue-128534.3A0K3D.rst diff --git a/Include/internal/pycore_magic_number.h b/Include/internal/pycore_magic_number.h index e97d066ebf8e5c..e8c33b7bb1609d 100644 --- a/Include/internal/pycore_magic_number.h +++ b/Include/internal/pycore_magic_number.h @@ -270,6 +270,7 @@ Known values: Python 3.14a5 3615 (CALL_FUNCTION_EX always take a kwargs argument) Python 3.14a5 3616 (Remove BINARY_SUBSCR and family. Make them BINARY_OPs) Python 3.14a6 3617 (Branch monitoring for async for loops) + Python 3.14a6 3618 (Add oparg to END_ASYNC_FOR) Python 3.15 will start with 3650 @@ -282,7 +283,7 @@ PC/launcher.c must also be updated. */ -#define PYC_MAGIC_NUMBER 3617 +#define PYC_MAGIC_NUMBER 3618 /* 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/Misc/NEWS.d/next/Core_and_Builtins/2025-03-04-15-12-32.gh-issue-128534.3A0K3D.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-03-04-15-12-32.gh-issue-128534.3A0K3D.rst new file mode 100644 index 00000000000000..130f55c8164c3e --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-03-04-15-12-32.gh-issue-128534.3A0K3D.rst @@ -0,0 +1,2 @@ +Ensure that both and left branches have same source for ``async for`` loops. +Add these branches to the ``co_branches()`` iterator. From 4c50ef319aac02a50939d9cc29963189b16114ef Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 4 Mar 2025 15:20:37 +0000 Subject: [PATCH 3/8] Tidy up debug code --- Lib/test/test_monitoring.py | 3 +-- Python/assemble.c | 2 +- Python/instruction_sequence.c | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py index cb6a8cacab0e3c..fc98b9eee383e4 100644 --- a/Lib/test/test_monitoring.py +++ b/Lib/test/test_monitoring.py @@ -1706,7 +1706,7 @@ def check_branches(self, run_func, test_func=None, tool=TEST_TOOL, recorders=BRA rights.add((src, right)) print(event_list) for event in event_list: - way, name, src, dest = event + way, _, src, dest = event if "left" in way: self.assertIn((src, dest), lefts) else: @@ -1782,7 +1782,6 @@ def func(): except StopIteration: pass - print(list(foo.__code__.co_branches())) self.check_branches(func, foo) diff --git a/Python/assemble.c b/Python/assemble.c index c9abcfcca58d20..1dbcfe51da6eec 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -675,7 +675,7 @@ resolve_jump_offsets(instr_sequence *instrs) instruction *target = &instrs->s_instrs[instr->i_target]; instr->i_oparg = target->i_offset; if (instr->i_opcode == END_ASYNC_FOR) { - // Adjust the offset so that target is the END_SEND + // Monitoring needs the target to be the END_SEND instr->i_oparg = offset - instr->i_oparg - END_SEND_OFFSET; } else if (instr->i_oparg < offset) { diff --git a/Python/instruction_sequence.c b/Python/instruction_sequence.c index bba701033e2891..ed40c06715f1f3 100644 --- a/Python/instruction_sequence.c +++ b/Python/instruction_sequence.c @@ -29,7 +29,6 @@ typedef _Py_SourceLocation location; #define RETURN_IF_ERROR(X) \ if ((X) == -1) { \ - assert(PyErr_Occurred()); \ return ERROR; \ } From 28991037ce2b23581dab8b81a37011a133bb8f67 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 4 Mar 2025 15:46:10 +0000 Subject: [PATCH 4/8] Fixup tests --- Lib/test/test_code.py | 2 +- Lib/test/test_dis.py | 11 +++++++++++ Programs/test_frozenmain.h | 22 +++++++++++----------- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index 5a94794dabd23d..9422392a154ec9 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -943,7 +943,7 @@ async def afunc(): self.assertEqual( get_line_branches(afunc), - [(1,2,3)]) + [(1,1,3)]) if check_impl_detail(cpython=True) and ctypes is not None: py = ctypes.pythonapi diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index 27350120d667c2..3910c7488e3e78 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -1256,6 +1256,17 @@ def test__try_compile_no_context_exc_on_error(self): except Exception as e: self.assertIsNone(e.__context__) + def test_async_for(self): + + async def afunc(): + async for letter in async_iter1: + l2 + l3 + + expected = "" + self.do_disassembly_test(afunc, expected) + + @staticmethod def code_quicken(f): _testinternalcapi = import_helper.import_module("_testinternalcapi") diff --git a/Programs/test_frozenmain.h b/Programs/test_frozenmain.h index 0fe8d3d3f7d8c6..58127656fd2fec 100644 --- a/Programs/test_frozenmain.h +++ b/Programs/test_frozenmain.h @@ -2,18 +2,18 @@ unsigned char M_test_frozenmain[] = { 227,0,0,0,0,0,0,0,0,0,0,0,0,9,0,0, 0,0,0,0,0,243,184,0,0,0,149,0,90,0,80,0, - 71,0,112,0,90,0,80,0,71,1,112,1,89,2,33,0, - 80,1,51,1,0,0,0,0,0,0,31,0,89,2,33,0, + 71,0,112,0,90,0,80,0,71,1,112,1,89,2,32,0, + 80,1,50,1,0,0,0,0,0,0,30,0,89,2,32,0, 80,2,89,0,78,6,0,0,0,0,0,0,0,0,0,0, - 0,0,0,0,0,0,0,0,51,2,0,0,0,0,0,0, - 31,0,89,1,78,8,0,0,0,0,0,0,0,0,0,0, - 0,0,0,0,0,0,0,0,33,0,51,0,0,0,0,0, - 0,0,80,3,44,26,0,0,0,0,0,0,0,0,0,0, - 112,5,80,4,16,0,68,24,0,0,112,6,89,2,33,0, - 80,5,89,6,12,0,80,6,89,5,89,6,44,26,0,0, - 0,0,0,0,0,0,0,0,12,0,49,4,51,1,0,0, - 0,0,0,0,31,0,73,26,0,0,9,0,30,0,80,0, - 35,0,41,7,78,122,18,70,114,111,122,101,110,32,72,101, + 0,0,0,0,0,0,0,0,50,2,0,0,0,0,0,0, + 30,0,89,1,78,8,0,0,0,0,0,0,0,0,0,0, + 0,0,0,0,0,0,0,0,32,0,50,0,0,0,0,0, + 0,0,80,3,43,26,0,0,0,0,0,0,0,0,0,0, + 112,5,80,4,15,0,68,24,0,0,112,6,89,2,32,0, + 80,5,89,6,11,0,80,6,89,5,89,6,43,26,0,0, + 0,0,0,0,0,0,0,0,11,0,48,4,50,1,0,0, + 0,0,0,0,30,0,73,26,0,0,8,0,29,0,80,0, + 34,0,41,7,78,122,18,70,114,111,122,101,110,32,72,101, 108,108,111,32,87,111,114,108,100,122,8,115,121,115,46,97, 114,103,118,218,6,99,111,110,102,105,103,41,5,218,12,112, 114,111,103,114,97,109,95,110,97,109,101,218,10,101,120,101, From e7f0db3e04a0c6a3a87b7a8bc92a5af6d73f14cc Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 4 Mar 2025 17:05:51 +0000 Subject: [PATCH 5/8] Fix up dis to understand oparg of END_ASYNC_FOR --- Lib/dis.py | 7 +++++-- Lib/test/test_dis.py | 12 +++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/Lib/dis.py b/Lib/dis.py index 72cc2d19456467..3137a6aebad8c8 100644 --- a/Lib/dis.py +++ b/Lib/dis.py @@ -52,6 +52,7 @@ STORE_FAST_STORE_FAST = opmap['STORE_FAST_STORE_FAST'] IS_OP = opmap['IS_OP'] CONTAINS_OP = opmap['CONTAINS_OP'] +END_ASYNC_FOR = opmap['END_ASYNC_FOR'] CACHE = opmap["CACHE"] @@ -605,7 +606,8 @@ def get_argval_argrepr(self, op, arg, offset): argval = self.offset_from_jump_arg(op, arg, offset) lbl = self.get_label_for_offset(argval) assert lbl is not None - argrepr = f"to L{lbl}" + preposition = "from" if deop == END_ASYNC_FOR else "to" + argrepr = f"{preposition} L{lbl}" elif deop in (LOAD_FAST_LOAD_FAST, STORE_FAST_LOAD_FAST, STORE_FAST_STORE_FAST): arg1 = arg >> 4 arg2 = arg & 15 @@ -745,7 +747,8 @@ def _parse_exception_table(code): def _is_backward_jump(op): return opname[op] in ('JUMP_BACKWARD', - 'JUMP_BACKWARD_NO_INTERRUPT') + 'JUMP_BACKWARD_NO_INTERRUPT', + 'END_ASYNC_FOR') # No really a jump, but it has a target def _get_instructions_bytes(code, linestarts=None, line_offset=0, co_positions=None, original_code=None, arg_resolver=None): diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index f2e12d8a98e35a..6e1d94bd535663 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -1256,15 +1256,21 @@ def test__try_compile_no_context_exc_on_error(self): except Exception as e: self.assertIsNone(e.__context__) - def test_async_for(self): + def test_async_for_presentation(self): async def afunc(): async for letter in async_iter1: l2 l3 - expected = "" - self.do_disassembly_test(afunc, expected) + disassembly = self.get_disassembly(afunc) + for line in disassembly.split("\n"): + if "END_ASYNC_FOR" in line: + break + else: + self.fail("No END_ASYNC_FOR in disassembly of async for") + self.assertNotIn("to", line) + self.assertIn("from", line) @staticmethod From d2996f2ece3e0a424764386318710382b72e3e8e Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 4 Mar 2025 17:12:42 +0000 Subject: [PATCH 6/8] Fix typo --- Lib/dis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/dis.py b/Lib/dis.py index 3137a6aebad8c8..c0a25dea2a9a95 100644 --- a/Lib/dis.py +++ b/Lib/dis.py @@ -748,7 +748,7 @@ def _parse_exception_table(code): def _is_backward_jump(op): return opname[op] in ('JUMP_BACKWARD', 'JUMP_BACKWARD_NO_INTERRUPT', - 'END_ASYNC_FOR') # No really a jump, but it has a target + 'END_ASYNC_FOR') # Not really a jump, but it has a "target" def _get_instructions_bytes(code, linestarts=None, line_offset=0, co_positions=None, original_code=None, arg_resolver=None): From 7d3dafabd80f45c87f49bb4798bb99c2438c5d4e Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 6 Mar 2025 10:43:17 +0000 Subject: [PATCH 7/8] Clarifications --- .../2025-03-04-15-12-32.gh-issue-128534.3A0K3D.rst | 2 +- Python/assemble.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-03-04-15-12-32.gh-issue-128534.3A0K3D.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-03-04-15-12-32.gh-issue-128534.3A0K3D.rst index 130f55c8164c3e..025847fdd6620b 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-03-04-15-12-32.gh-issue-128534.3A0K3D.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-03-04-15-12-32.gh-issue-128534.3A0K3D.rst @@ -1,2 +1,2 @@ -Ensure that both and left branches have same source for ``async for`` loops. +Ensure that both left and right branches have the same source for ``async for`` loops. Add these branches to the ``co_branches()`` iterator. diff --git a/Python/assemble.c b/Python/assemble.c index 1dbcfe51da6eec..e33918edf8e4b9 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -675,7 +675,8 @@ resolve_jump_offsets(instr_sequence *instrs) instruction *target = &instrs->s_instrs[instr->i_target]; instr->i_oparg = target->i_offset; if (instr->i_opcode == END_ASYNC_FOR) { - // Monitoring needs the target to be the END_SEND + // sys.monitoring needs to be able to find the matching END_SEND + // but the target is the SEND, so we adjust it here. instr->i_oparg = offset - instr->i_oparg - END_SEND_OFFSET; } else if (instr->i_oparg < offset) { From 15bf47bee1eb21e289df42542e65ee8117e0ed32 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 7 Mar 2025 12:15:55 +0000 Subject: [PATCH 8/8] Assert that END_SEND is the source of the branch event for async for --- Python/bytecodes.c | 1 + Python/generated_cases.c.h | 1 + 2 files changed, 2 insertions(+) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 6f4fc78b883370..189b729eb2764f 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1358,6 +1358,7 @@ dummy_func( } tier1 op(_MONITOR_END_ASYNC_FOR, ( -- )) { + assert((next_instr-oparg)->op.code == END_SEND || (next_instr-oparg)->op.code >= MIN_INSTRUMENTED_OPCODE); INSTRUMENTED_JUMP(next_instr-oparg, this_instr+1, PY_MONITORING_EVENT_BRANCH_RIGHT); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index f3182ce9d0c71e..2ace4f1c702547 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -6618,6 +6618,7 @@ _PyStackRef exc_st; // _MONITOR_END_ASYNC_FOR { + assert((next_instr-oparg)->op.code == END_SEND || (next_instr-oparg)->op.code >= MIN_INSTRUMENTED_OPCODE); INSTRUMENTED_JUMP(next_instr-oparg, this_instr+1, PY_MONITORING_EVENT_BRANCH_RIGHT); } // _END_ASYNC_FOR