gh-130279: Fix missing exception handlers for jumps in loops

[LH-and-FPGA]

When a loop body ends with an if whose false branch jumps back to the loop head, the backward jump ended up outside the exception table ranges of the enclosing try/with. An exception raised when that JUMP_BACKWARD checks the eval breaker (e.g. KeyboardInterrupt, or an async exception set via PyThreadState_SetAsyncExc) then skipped the except/finally blocks, and __exit__ was not called for with blocks.

def scan(items):
    try:
        for x in items:
            if x < 0:
                raise ValueError(x)
    except KeyboardInterrupt:
        return "handled"

Two places in Python/flowgraph.c created jump instructions without an exception handler after label_exception_targets() had already run:

• jump_thread() replaced the original jump with a new one, dropping its i_except.
• normalize_jumps_in_block() added NOT_TAKEN + JUMP for backward conditional jumps without copying the handler.

Both now copy the handler of the original jump.

• Issue: KeyboardInterrupt not handled while evaluating a tight while loop condition on Python 3.13 #130279

[python-cla-bot]

All commit authors signed the Contributor License Agreement.

[iritkatriel]

We could add an except arg to basicblock_addop and basicblock_add_jump (pass NULL if there is none).

[LH-and-FPGA]

Thanks, I'll add an except on those places which replaces the post-hoc fixups.

[LH-and-FPGA]

While doing this I noticed that the forward-branch NOT_TAKEN in normalize_jumps_in_block() also gets no handler, which leaves a gap in the exception table. It could matter when a sys.monitoring BRANCH_LEFT callback raises. Should I fix it here, or in a separate issue?

[iritkatriel]

Fix it here.

[LH-and-FPGA]

Sure

[iritkatriel]

I don't think this comment is necessary. We copy i_loc and i_except, it makes sense.

[iritkatriel]

Does this need to be backported?

[LH-and-FPGA]

Does this need to be backported?

Do you mean fix this for 3.13 3.14 and 3.15?

[iritkatriel]

Yes.

[LH-and-FPGA]

Yes.

3.14 and 3.15 have the same code, so I think it should go to both. 3.13 and 3.12 has the backward jump issue as well, but it's security-only now. so how should I backport in general? Thanks

[LH-and-FPGA]

Does this need to be backported?

I found this https://devguide.python.org/getting-started/git-boot-camp/#backporting-merged-changes, so basically I don't need to do anything I think?

[iritkatriel]

Why are we taking location from target but handler from inst?

[LH-and-FPGA]

they should be the same in practice. jumps don't change the exception stack, and at this stage the NOPs left by POP_BLOCK haven't been removed yet, so threading never crosses a try boundary ( and I checked by compiling the stdlib with a debug check, and there were no mismatches). But the new jump stands in for target, so does that mean taking both from target is more consistent? Thanks.

[iritkatriel]

Let's take it from target then, so it doesn't look strange.

I'm seeing some failures in test_dis on the builedbot.

[LH-and-FPGA]

The test_dis failures are indeed caused by this PR. The expected disassembly for dis_traceback, with and async with captured the old behaviour i think. but the NOT_TAKEN in the except clause wasn't covered by the exception table. With the forward NOT_TAKEN now getting the handler, that gap is gone. The adjacent exception table entries merge and there are fewer labels, so I think the expected output just needs updating. I missed this locally because of stale .pyc files in pycache. Should I push a fix to the expected output?

[iritkatriel]

Yes, if the changes are expected and not due to bugs then update the tests.

[LH-and-FPGA]

Sure,

[LH-and-FPGA]

Updated.

[iritkatriel]

Does this need to be backported?

I found this https://devguide.python.org/getting-started/git-boot-camp/#backporting-merged-changes, so basically I don't need to do anything I think?

I put the labels for back porting. If it goes smoothly you don't do anything. If there are conflicts you'll need to resolve them (create the back port manually off the 3.14/3.15 branches).

[iritkatriel]

Is this your first PR? If so then add your name to the contributors file.

[miss-islington-app]

Thanks @LH-and-FPGA for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14, 3.15.
🐍🍒⛏🤖

[miss-islington-app]

Sorry, @LH-and-FPGA and @iritkatriel, I could not cleanly backport this to 3.14 due to a conflict.

Please backport manually with cherry_picker, see the devguide for more information.

cherry_picker 036d309cf542986a39eeba5ca805b2ec7619731d 3.14

[bedevere-app]

GH-159698 is a backport of this pull request to the 3.15 branch.

[iritkatriel]

Thank you @LH-and-FPGA!

[iritkatriel]

The 3.15 back port worked but 3.14 needs help.

[LH-and-FPGA]

Thanks for the review, @iritkatriel! I’ll look into backporting this to 3.14.

[bedevere-app]

GH-159703 is a backport of this pull request to the 3.14 branch.