Skip to content

Commit 036d309

Browse files
authored
gh-130279: Fix missing exception handlers for jumps in loops (#159460)
1 parent ac42dc4 commit 036d309

6 files changed

Lines changed: 127 additions & 69 deletions

File tree

‎Lib/test/test_compile.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1583,6 +1583,28 @@ def f():
15831583

15841584
self.check_line_numbers(f.__code__, 'JUMP_BACKWARD')
15851585

1586+
def test_backward_jump_conditional_in_loop_in_try(self):
1587+
# gh-130279: The jumps in a loop body, including the backward jump
1588+
# for the untaken branch of a conditional at the end of the body,
1589+
# must be covered by the exception handler of the enclosing try.
1590+
def f(items):
1591+
try:
1592+
for x in items:
1593+
if x < 0:
1594+
raise ValueError(x)
1595+
except KeyboardInterrupt:
1596+
return 'handled'
1597+
1598+
bytecode = dis.Bytecode(f)
1599+
jumps = [instr for instr in bytecode
1600+
if instr.opcode in dis.hasjump]
1601+
self.assertTrue(jumps)
1602+
for instr in jumps:
1603+
with self.subTest(instr=instr):
1604+
self.assertTrue(
1605+
any(entry.start <= instr.offset < entry.end
1606+
for entry in bytecode.exception_entries))
1607+
15861608
def test_big_dict_literal(self):
15871609
# The compiler has a flushing point in "compiler_dict" that calls compiles
15881610
# a portion of the dictionary literal when the loop that iterates over the items

‎Lib/test/test_dis.py‎

Lines changed: 56 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -454,53 +454,52 @@ def foo(a: int, b: str) -> str:
454454
"""
455455

456456
dis_traceback = """\
457-
%4d RESUME 0
457+
%4d RESUME 0
458458
459-
%4d NOP
459+
%4d NOP
460460
461-
%4d L1: LOAD_SMALL_INT 1
462-
LOAD_SMALL_INT 0
463-
--> BINARY_OP 11 (/)
464-
POP_TOP
461+
%4d L1: LOAD_SMALL_INT 1
462+
LOAD_SMALL_INT 0
463+
--> BINARY_OP 11 (/)
464+
POP_TOP
465465
466-
%4d L2: LOAD_FAST_CHECK 1 (tb)
467-
RETURN_VALUE
466+
%4d L2: LOAD_FAST_CHECK 1 (tb)
467+
RETURN_VALUE
468468
469-
-- L3: PUSH_EXC_INFO
469+
-- L3: PUSH_EXC_INFO
470470
471-
%4d LOAD_GLOBAL 0 (Exception)
472-
CHECK_EXC_MATCH
473-
POP_JUMP_IF_FALSE 24 (to L9)
474-
L4: NOT_TAKEN
475-
L5: STORE_FAST 0 (e)
471+
%4d LOAD_GLOBAL 0 (Exception)
472+
CHECK_EXC_MATCH
473+
POP_JUMP_IF_FALSE 24 (to L7)
474+
NOT_TAKEN
475+
STORE_FAST 0 (e)
476476
477-
%4d L6: LOAD_FAST 0 (e)
478-
LOAD_ATTR 2 (__traceback__)
479-
STORE_FAST 1 (tb)
480-
L7: POP_EXCEPT
481-
LOAD_COMMON_CONSTANT 7 (None)
482-
STORE_FAST 0 (e)
483-
DELETE_FAST 0 (e)
477+
%4d L4: LOAD_FAST 0 (e)
478+
LOAD_ATTR 2 (__traceback__)
479+
STORE_FAST 1 (tb)
480+
L5: POP_EXCEPT
481+
LOAD_COMMON_CONSTANT 7 (None)
482+
STORE_FAST 0 (e)
483+
DELETE_FAST 0 (e)
484484
485-
%4d LOAD_FAST 1 (tb)
486-
RETURN_VALUE
485+
%4d LOAD_FAST 1 (tb)
486+
RETURN_VALUE
487487
488-
-- L8: LOAD_COMMON_CONSTANT 7 (None)
489-
STORE_FAST 0 (e)
490-
DELETE_FAST 0 (e)
491-
RERAISE 1
488+
-- L6: LOAD_COMMON_CONSTANT 7 (None)
489+
STORE_FAST 0 (e)
490+
DELETE_FAST 0 (e)
491+
RERAISE 1
492492
493-
%4d L9: RERAISE 0
493+
%4d L7: RERAISE 0
494494
495-
-- L10: COPY 3
496-
POP_EXCEPT
497-
RERAISE 1
495+
-- L8: COPY 3
496+
POP_EXCEPT
497+
RERAISE 1
498498
ExceptionTable:
499499
L1 to L2 -> L3 [0]
500-
L3 to L4 -> L10 [1] lasti
501-
L5 to L6 -> L10 [1] lasti
502-
L6 to L7 -> L8 [1] lasti
503-
L8 to L10 -> L10 [1] lasti
500+
L3 to L4 -> L8 [1] lasti
501+
L4 to L5 -> L6 [1] lasti
502+
L6 to L8 -> L8 [1] lasti
504503
""" % (TRACEBACK_CODE.co_firstlineno,
505504
TRACEBACK_CODE.co_firstlineno + 1,
506505
TRACEBACK_CODE.co_firstlineno + 2,
@@ -569,11 +568,11 @@ def _with(c):
569568
%4d L3: PUSH_EXC_INFO
570569
WITH_EXCEPT_START
571570
TO_BOOL
572-
POP_JUMP_IF_TRUE 2 (to L6)
573-
L4: NOT_TAKEN
574-
L5: RERAISE 2
575-
L6: POP_TOP
576-
L7: POP_EXCEPT
571+
POP_JUMP_IF_TRUE 2 (to L4)
572+
NOT_TAKEN
573+
RERAISE 2
574+
L4: POP_TOP
575+
L5: POP_EXCEPT
577576
POP_TOP
578577
POP_TOP
579578
POP_TOP
@@ -583,13 +582,12 @@ def _with(c):
583582
LOAD_COMMON_CONSTANT 7 (None)
584583
RETURN_VALUE
585584
586-
-- L8: COPY 3
585+
-- L6: COPY 3
587586
POP_EXCEPT
588587
RERAISE 1
589588
ExceptionTable:
590589
L1 to L2 -> L3 [2] lasti
591-
L3 to L4 -> L8 [4] lasti
592-
L5 to L7 -> L8 [4] lasti
590+
L3 to L5 -> L6 [4] lasti
593591
""" % (_with.__code__.co_firstlineno,
594592
_with.__code__.co_firstlineno + 1,
595593
_with.__code__.co_firstlineno + 2,
@@ -664,11 +662,11 @@ async def _asyncwith(c):
664662
L20: CLEANUP_THROW
665663
L21: END_SEND
666664
TO_BOOL
667-
POP_JUMP_IF_TRUE 2 (to L24)
668-
L22: NOT_TAKEN
669-
L23: RERAISE 2
670-
L24: POP_TOP
671-
L25: POP_EXCEPT
665+
POP_JUMP_IF_TRUE 2 (to L22)
666+
NOT_TAKEN
667+
RERAISE 2
668+
L22: POP_TOP
669+
L23: POP_EXCEPT
672670
POP_TOP
673671
POP_TOP
674672
POP_TOP
@@ -678,25 +676,24 @@ async def _asyncwith(c):
678676
LOAD_COMMON_CONSTANT 7 (None)
679677
RETURN_VALUE
680678
681-
-- L26: COPY 3
679+
-- L24: COPY 3
682680
POP_EXCEPT
683681
RERAISE 1
684-
L27: CALL_INTRINSIC_1 3 (INTRINSIC_STOPITERATION_ERROR)
682+
L25: CALL_INTRINSIC_1 3 (INTRINSIC_STOPITERATION_ERROR)
685683
RERAISE 1
686684
ExceptionTable:
687-
L1 to L3 -> L27 [0] lasti
685+
L1 to L3 -> L25 [0] lasti
688686
L3 to L4 -> L12 [5]
689-
L4 to L6 -> L27 [0] lasti
687+
L4 to L6 -> L25 [0] lasti
690688
L6 to L7 -> L16 [2] lasti
691-
L7 to L9 -> L27 [0] lasti
689+
L7 to L9 -> L25 [0] lasti
692690
L9 to L10 -> L14 [3]
693-
L10 to L13 -> L27 [0] lasti
694-
L14 to L15 -> L27 [0] lasti
695-
L16 to L18 -> L26 [4] lasti
691+
L10 to L13 -> L25 [0] lasti
692+
L14 to L15 -> L25 [0] lasti
693+
L16 to L18 -> L24 [4] lasti
696694
L18 to L19 -> L20 [8]
697-
L19 to L22 -> L26 [4] lasti
698-
L23 to L25 -> L26 [4] lasti
699-
L25 to L27 -> L27 [0] lasti
695+
L19 to L23 -> L24 [4] lasti
696+
L23 to L25 -> L25 [0] lasti
700697
""" % (_asyncwith.__code__.co_firstlineno,
701698
_asyncwith.__code__.co_firstlineno + 1,
702699
_asyncwith.__code__.co_firstlineno + 2,

‎Lib/test/test_monitoring.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2529,6 +2529,37 @@ def foo():
25292529
foo()
25302530
sys.monitoring.set_local_events(0, foo.__code__, 0)
25312531

2532+
def test_130279(self):
2533+
# gh-130279
2534+
# An exception raised by the BRANCH_LEFT callback of a forward
2535+
# branch must be caught by the enclosing try
2536+
class Boom(Exception):
2537+
pass
2538+
2539+
raised = False
2540+
2541+
def callback(code, src, dst):
2542+
nonlocal raised
2543+
if not raised:
2544+
raised = True
2545+
raise Boom
2546+
2547+
def foo(x):
2548+
try:
2549+
if x:
2550+
pass
2551+
except Boom:
2552+
return "caught"
2553+
return "not raised"
2554+
2555+
sys.monitoring.register_callback(TEST_TOOL, E.BRANCH_LEFT, callback)
2556+
sys.monitoring.set_local_events(TEST_TOOL, foo.__code__, E.BRANCH_LEFT)
2557+
try:
2558+
self.assertEqual(foo(True), "caught")
2559+
finally:
2560+
sys.monitoring.set_local_events(TEST_TOOL, foo.__code__, 0)
2561+
sys.monitoring.register_callback(TEST_TOOL, E.BRANCH_LEFT, None)
2562+
25322563

25332564
class TestOptimizer(MonitoringTestBase, unittest.TestCase):
25342565

‎Misc/ACKS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,6 +1144,7 @@ Ivan Levkivskyi
11441144
Ben Lewis
11451145
William Lewis
11461146
Akira Li
1147+
Han Li
11471148
Jiahao Li
11481149
Robert Li
11491150
Xuanji Li
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix bug where compiler did not set the right exception handler for some jump
2+
instructions.

‎Python/flowgraph.c‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,8 @@ cfg_builder_new_block(cfg_builder *g)
188188
}
189189

190190
static int
191-
basicblock_addop(basicblock *b, int opcode, int oparg, location loc)
191+
basicblock_addop(basicblock *b, int opcode, int oparg, location loc,
192+
basicblock *except)
192193
{
193194
assert(IS_WITHIN_OPCODE_RANGE(opcode));
194195
assert(!IS_ASSEMBLER_OPCODE(opcode));
@@ -203,23 +204,24 @@ basicblock_addop(basicblock *b, int opcode, int oparg, location loc)
203204
i->i_opcode = opcode;
204205
i->i_oparg = oparg;
205206
i->i_loc = loc;
207+
i->i_except = except;
206208
// memory is already zero initialized
207209
assert(i->i_target == NULL);
208-
assert(i->i_except == NULL);
209210

210211
return SUCCESS;
211212
}
212213

213214
static int
214-
basicblock_add_jump(basicblock *b, int opcode, basicblock *target, location loc)
215+
basicblock_add_jump(basicblock *b, int opcode, basicblock *target, location loc,
216+
basicblock *except)
215217
{
216218
cfg_instr *last = basicblock_last_instr(b);
217219
if (last && is_jump(last)) {
218220
return ERROR;
219221
}
220222

221223
RETURN_IF_ERROR(
222-
basicblock_addop(b, opcode, target->b_label.id, loc));
224+
basicblock_addop(b, opcode, target->b_label.id, loc, except));
223225
last = basicblock_last_instr(b);
224226
assert(last && last->i_opcode == opcode);
225227
last->i_target = target;
@@ -493,7 +495,7 @@ int
493495
_PyCfgBuilder_Addop(cfg_builder *g, int opcode, int oparg, location loc)
494496
{
495497
RETURN_IF_ERROR(cfg_builder_maybe_start_new_block(g));
496-
return basicblock_addop(g->g_curblock, opcode, oparg, loc);
498+
return basicblock_addop(g->g_curblock, opcode, oparg, loc, NULL);
497499
}
498500

499501

@@ -554,7 +556,7 @@ normalize_jumps_in_block(cfg_builder *g, basicblock *b) {
554556
bool is_forward = last->i_target->b_visited == 0;
555557
if (is_forward) {
556558
RETURN_IF_ERROR(
557-
basicblock_addop(b, NOT_TAKEN, 0, last->i_loc));
559+
basicblock_addop(b, NOT_TAKEN, 0, last->i_loc, last->i_except));
558560
return SUCCESS;
559561
}
560562

@@ -583,9 +585,11 @@ normalize_jumps_in_block(cfg_builder *g, basicblock *b) {
583585
return ERROR;
584586
}
585587
RETURN_IF_ERROR(
586-
basicblock_addop(backwards_jump, NOT_TAKEN, 0, last->i_loc));
588+
basicblock_addop(backwards_jump, NOT_TAKEN, 0, last->i_loc,
589+
last->i_except));
587590
RETURN_IF_ERROR(
588-
basicblock_add_jump(backwards_jump, JUMP, target, last->i_loc));
591+
basicblock_add_jump(backwards_jump, JUMP, target, last->i_loc,
592+
last->i_except));
589593
backwards_jump->b_startdepth = target->b_startdepth;
590594
last->i_opcode = reversed_opcode;
591595
last->i_target = b->b_next;
@@ -1293,7 +1297,8 @@ jump_thread(basicblock *bb, cfg_instr *inst, cfg_instr *target, int opcode)
12931297

12941298
RETURN_IF_ERROR(
12951299
basicblock_add_jump(
1296-
bb, opcode, target->i_target, target->i_loc));
1300+
bb, opcode, target->i_target, target->i_loc,
1301+
target->i_except));
12971302

12981303
return true;
12991304
}
@@ -3547,7 +3552,7 @@ push_cold_blocks_to_end(cfg_builder *g) {
35473552
b->b_next->b_label.id = next_lbl++;
35483553
}
35493554
basicblock_addop(explicit_jump, JUMP_NO_INTERRUPT, b->b_next->b_label.id,
3550-
NO_LOCATION);
3555+
NO_LOCATION, NULL);
35513556
explicit_jump->b_cold = 1;
35523557
explicit_jump->b_next = b->b_next;
35533558
explicit_jump->b_predecessors = 1;

0 commit comments

Comments
 (0)