Skip to content

Source location of return instruction in a with block is incorrect #98442

Description

@iritkatriel
def f():
 with x:
   return 42

import dis
from pprint import pprint as pp
def pos(p):
    return (p.lineno, p.end_lineno, p.col_offset, p.end_col_offset)

pp([(pos(x.positions), x.opname, x.argval) for x in dis.get_instructions(f)])

Output:

[((1, 1, 0, 0), 'RESUME', 0),
 ((2, 2, 6, 7), 'LOAD_GLOBAL', 'x'),
 ((2, 3, 1, 12), 'BEFORE_WITH', None),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((3, 3, 10, 12), 'NOP', None),
 ((2, 3, 1, 12), 'LOAD_CONST', None),
 ((2, 3, 1, 12), 'LOAD_CONST', None),
 ((2, 3, 1, 12), 'LOAD_CONST', None),
 ((2, 3, 1, 12), 'CALL', 2),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((2, 3, 1, 12), 'LOAD_CONST', 42),                 <-- incorrect
 ((2, 3, 1, 12), 'RETURN_VALUE', None),             <-- incorrect
 ((2, 3, 1, 12), 'PUSH_EXC_INFO', None),
 ((2, 3, 1, 12), 'WITH_EXCEPT_START', None),
 ((2, 3, 1, 12), 'POP_JUMP_IF_TRUE', 50),
 ((2, 3, 1, 12), 'RERAISE', 2),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((2, 3, 1, 12), 'POP_EXCEPT', None),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((2, 3, 1, 12), 'POP_TOP', None),
 ((2, 3, 1, 12), 'LOAD_CONST', None),
 ((2, 3, 1, 12), 'RETURN_VALUE', None),
 ((None, None, None, None), 'COPY', 3),
 ((None, None, None, None), 'POP_EXCEPT', None),
 ((None, None, None, None), 'RERAISE', 1)]

Linked PRs

Activity

  1. self-assigned this
    on Oct 19, 2022
  2. added a commit that references this issue on Oct 19, 2022
  3. changed the title [-]location of return instruction in a with block is incorrect[/-] [+]Source locations of return instruction in a with block is incorrect[/+] on Oct 19, 2022
  4. changed the title [-]Source locations of return instruction in a with block is incorrect[/-] [+]Source location of return instruction in a with block is incorrect[/+] on Oct 19, 2022
  5. iritkatriel commented on Oct 19, 2022

    @iritkatriel
    MemberAuthor

    @nedbat FYI.

  6. iritkatriel commented on Jun 19, 2024

    @iritkatriel
    MemberAuthor

    This is now less wrong, but still there are instructions (in the __exit__ call) that span the whole body of the with-block:

    The return instruction takes its location from there, so it will be fixed once this is.

    [((1, 1, 0, 0), 'RESUME', 0),
     ((2, 2, 6, 7), 'LOAD_GLOBAL', 'x'),
     ((2, 2, 6, 7), 'COPY', 1),
     ((2, 2, 6, 7), 'LOAD_SPECIAL', 1),
     ((2, 2, 6, 7), 'SWAP', 2),
     ((2, 2, 6, 7), 'SWAP', 3),
     ((2, 2, 6, 7), 'LOAD_SPECIAL', 0),
     ((2, 2, 6, 7), 'CALL', 0),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((3, 3, 10, 12), 'NOP', None),
     ((2, 3, 1, 12), 'LOAD_CONST', None),      <-- Too wide
     ((2, 3, 1, 12), 'LOAD_CONST', None),
     ((2, 3, 1, 12), 'LOAD_CONST', None),
     ((2, 3, 1, 12), 'CALL', 3),
     ((2, 3, 1, 12), 'POP_TOP', None),
     ((2, 3, 1, 12), 'RETURN_CONST', 42),
     ((2, 2, 6, 7), 'PUSH_EXC_INFO', None),
     ((2, 2, 6, 7), 'WITH_EXCEPT_START', None),
     ((2, 2, 6, 7), 'TO_BOOL', None),
     ((2, 2, 6, 7), 'POP_JUMP_IF_TRUE', 70),
     ((2, 2, 6, 7), 'RERAISE', 2),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((2, 2, 6, 7), 'POP_EXCEPT', None),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((2, 2, 6, 7), 'RETURN_CONST', None),
     ((None, None, None, None), 'COPY', 3),
     ((None, None, None, None), 'POP_EXCEPT', None),
     ((None, None, None, None), 'RERAISE', 1)]
    
  7. added a commit that references this issue on Jun 19, 2024
  8. iritkatriel commented on Jun 19, 2024

    @iritkatriel
    MemberAuthor

    With the fix in the PR we get:

    [((1, 1, 0, 0), 'RESUME', 0),
     ((2, 2, 6, 7), 'LOAD_GLOBAL', 'x'),
     ((2, 2, 6, 7), 'COPY', 1),
     ((2, 2, 6, 7), 'LOAD_SPECIAL', 1),
     ((2, 2, 6, 7), 'SWAP', 2),
     ((2, 2, 6, 7), 'SWAP', 3),
     ((2, 2, 6, 7), 'LOAD_SPECIAL', 0),
     ((2, 2, 6, 7), 'CALL', 0),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((3, 3, 10, 12), 'NOP', None),
     ((2, 2, 6, 7), 'LOAD_CONST', None),
     ((2, 2, 6, 7), 'LOAD_CONST', None),
     ((2, 2, 6, 7), 'LOAD_CONST', None),
     ((2, 2, 6, 7), 'CALL', 3),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((2, 2, 6, 7), 'RETURN_CONST', 42),
     ((2, 2, 6, 7), 'PUSH_EXC_INFO', None),
     ((2, 2, 6, 7), 'WITH_EXCEPT_START', None),
     ((2, 2, 6, 7), 'TO_BOOL', None),
     ((2, 2, 6, 7), 'POP_JUMP_IF_TRUE', 70),
     ((2, 2, 6, 7), 'RERAISE', 2),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((2, 2, 6, 7), 'POP_EXCEPT', None),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((2, 2, 6, 7), 'POP_TOP', None),
     ((2, 2, 6, 7), 'RETURN_CONST', None),
     ((None, None, None, None), 'COPY', 3),
     ((None, None, None, None), 'POP_EXCEPT', None),
     ((None, None, None, None), 'RERAISE', 1)]
    
  9. added a commit that references this issue on Jun 20, 2024
  10. added 2 commits that reference this issue on Jun 20, 2024
  11. added a commit that references this issue on Jun 30, 2024
  12. added a commit that references this issue on Jul 11, 2024
  13. added a commit that references this issue on Jul 17, 2024
  14. added a commit that references this issue on Sep 2, 2024
  15. added a commit that references this issue on Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

3.12only security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-bugAn unexpected behavior, bug, or error

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions