Skip to content

Conversation

DinoV
Copy link
Contributor

@DinoV DinoV commented Aug 11, 2025

basicblock_addop zero initializes some members but not all. If we remove some nops and then later re-add an instruction we can have previously initialized memory that can include an exception handler. If we compile something like:

def foo():
    try:
        [x for x in abc]
    except OSError:
        pass

    return```

We'll end up with different exception handling table because the `NOT_TAKEN` instruction is inserted after we do the exception handling analysis and gets a previously initialized exception handler.

<!-- gh-issue-number: gh-137681 -->
* Issue: gh-137681
<!-- /gh-issue-number -->

@DinoV DinoV force-pushed the flowgraph_init_except branch 2 times, most recently from 9e390cb to 3a3a975 Compare August 12, 2025 15:45
@DinoV DinoV added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Aug 12, 2025
@DinoV DinoV force-pushed the flowgraph_init_except branch from 3a3a975 to 17de2a3 Compare August 12, 2025 16:19
@DinoV DinoV changed the title Always initialize exception handler for new instruction gh-137681: Always initialize exception handler for new instruction Aug 12, 2025
@DinoV DinoV force-pushed the flowgraph_init_except branch from 17de2a3 to 36a5650 Compare August 12, 2025 16:23
@DinoV DinoV marked this pull request as ready for review August 12, 2025 17:07
@iritkatriel
Copy link
Member

This change is fine, but I think basicblock_addop should get the target and except as args. There is a future bug here where setting the except matters, and someone will forget.

@DinoV
Copy link
Contributor Author

DinoV commented Aug 12, 2025

This change is fine, but I think basicblock_addop should get the target and except as args. There is a future bug here where setting the except matters, and someone will forget.

Do you think we should back port this to 3.14? While the NOT_TAKEN case is pretty benign I'm a little worried there could be something inserted where it does matter.

@iritkatriel
Copy link
Member

If we don't have a bug in 3.14 I think we can leave it alone. We're not likely to insert new bytecode transformations there.

@DinoV DinoV merged commit b78e9c0 into python:main Aug 13, 2025
42 checks passed
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
…ion (python#137655)

Always initialize exception handler for new instruction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants