Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LOAD_GLOBAL instruction with wrong source position #91409

Closed
15r10nk mannequin opened this issue Apr 7, 2022 · 7 comments
Closed

LOAD_GLOBAL instruction with wrong source position #91409

15r10nk mannequin opened this issue Apr 7, 2022 · 7 comments
Assignees
Labels
3.11 only security fixes 3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@15r10nk
Copy link
Mannequin

15r10nk mannequin commented Apr 7, 2022

BPO 47253
Nosy @15r10nk
Files
  • test3.py: code which shows the problem
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2022-04-07.21:05:52.344>
    labels = ['type-bug', '3.11']
    title = 'LOAD_GLOBAL instruction with wrong source position'
    updated_at = <Date 2022-04-07.21:05:52.344>
    user = 'https://github.com/15r10nk'

    bugs.python.org fields:

    activity = <Date 2022-04-07.21:05:52.344>
    actor = '15r10nk'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = []
    creation = <Date 2022-04-07.21:05:52.344>
    creator = '15r10nk'
    dependencies = []
    files = ['50727']
    hgrepos = []
    issue_num = 47253
    keywords = []
    message_count = 1.0
    messages = ['416945']
    nosy_count = 1.0
    nosy_names = ['15r10nk']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue47253'
    versions = ['Python 3.11']

    @15r10nk
    Copy link
    Mannequin Author

    15r10nk mannequin commented Apr 7, 2022

    The LOAD_GLOBAL instruction has a different/wrong source position if it refers the name of an imported module.

    @15r10nk 15r10nk mannequin added 3.11 only security fixes type-bug An unexpected behavior, bug, or error labels Apr 7, 2022
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @terryjreedy terryjreedy added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Apr 15, 2022
    @terryjreedy
    Copy link
    Member

    You should have pasted in the actual position tuples you saw that show the presumed bug.
    With 3.11.0a7, released 10 days ago, just before your report, the positions are identical (on Win 10).

    >>> import dis
    >>> dix=dis
    >>> def f():
    ...     dis.b()
    ...     dix.b()
    ...
    >>> for i in dis.Bytecode(f):
    ...     if i.opname == "LOAD_GLOBAL":
    ...         print(i.starts_lines, i.positions)
    ...   
    2 Positions(lineno=2, end_lineno=2, col_offset=4, end_col_offset=7)
    3 Positions(lineno=2, end_lineno=2, col_offset=4, end_col_offset=7)
    

    I don't know why the Positions.lineno does not match starts_lines, but your report is about columns. Here they match.
    I am presuming that you used an earlier alpha and that the OS does not matter and that this was recently discovered and fixed.
    Reopen if I missed something

    @15r10nk
    Copy link
    Contributor

    15r10nk commented Jul 16, 2022

    @terryjreedy The problem seems is be back in 3.11.0b4 (on ubuntu)

    import dis
    
    dix = 5
    
    
    def f():
        dis.b()  # wrong source range
        dix.b()  # correct source range
    
    
    d = [bc for bc in dis.Bytecode(f) if bc.opname == "LOAD_GLOBAL"]
    
    print(d)
    
    assert len(d) == 2
    
    # LOAD_GLOBAL has a wrong end column if the global value is an imported module
    assert d[0].positions[2:] == d[1].positions[2:] == (4, 7)

    output:

    [Instruction(opname='LOAD_GLOBAL', opcode=116, arg=1, argval='dis', argrepr='NULL + dis', offset=2, starts_line=7, is_jump_target=False, positions=Positions(lineno=7, end_lineno=7, col_offset=4, end_col_offset=9))
    , Instruction(opname='LOAD_GLOBAL', opcode=116, arg=4, argval='dix', argrepr='dix', offset=40, starts_line=8, is_jump_target=False, positions=Positions(lineno=8, end_lineno=8, col_offset=4, end_col_offset=7))]
    Traceback (most recent call last):
      File "/home/frank/projects/executing/test2.py", line 18, in <module>
        assert d[0].positions[2:] == d[1].positions[2:] == (4, 7)
    AssertionError
    

    can we open this issue again?

    @terryjreedy
    Copy link
    Member

    You changed from 'global name is an imported module name' to 'global object is an imported module. I revised by example to cover both and ran with freshly updated and compiled 'main' and '3.11' and installed 3.11.0b4'.

    import dis
    dix = dis
    dum = 5
    def f(): dis, dix, dum
    
    for i in dis.Bytecode(f):
        if i.opname == "LOAD_GLOBAL":
            print(i.positions)
    

    Output on Win 10:

    Positions(lineno=4, end_lineno=4, col_offset=9, end_col_offset=12)
    Positions(lineno=4, end_lineno=4, col_offset=14, end_col_offset=17)
    Positions(lineno=4, end_lineno=4, col_offset=19, end_col_offset=22)
    

    For all 3 names, end - start == 3. Same with each on separate line.

    Please run a pasted copy of the code above on your machine. If output is different, paste a copy.
    If same but minimal editing gives anomaly, paste code and output.

    @15r10nk
    Copy link
    Contributor

    15r10nk commented Jul 17, 2022

    I got the same results like you and there are no problems if we only lookup the global variable.
    Sorry for my wording error. I hope this Example will make it clear what I mean.

    The source range extends to the name of the called function if we call an function on a imported module.

    import dis
    dix = dis
    dum = 5
    def f():
        dis.a()
        dis.abc()
        dis.a
        dis
        dix.a()
        dum.abc()
    
    for i in dis.Bytecode(f):
        if i.opname == "LOAD_GLOBAL":
            print(i.positions)

    output:

    Positions(lineno=5, end_lineno=5, col_offset=4, end_col_offset=9)
    Positions(lineno=6, end_lineno=6, col_offset=4, end_col_offset=11)
    Positions(lineno=7, end_lineno=7, col_offset=4, end_col_offset=7)
    Positions(lineno=8, end_lineno=8, col_offset=4, end_col_offset=7)
    Positions(lineno=9, end_lineno=9, col_offset=4, end_col_offset=7)
    Positions(lineno=10, end_lineno=10, col_offset=4, end_col_offset=7)
    

    you can see that the method name gets included in the range. This should be not the case in my opinion. Or are there any reasons for this Behaviour? There is also no Problem for attribute lookups like dis.a.

    I also bisected the bahavior change down to 3011a097bd. Maybe this helps.

    @terryjreedy terryjreedy added the 3.12 bugs and security fixes label Jul 17, 2022
    @terryjreedy
    Copy link
    Member

    terryjreedy commented Jul 17, 2022

    @markshannon @pablogsal LOAD_GLOBAL of a name bound by import has an erroneous end_col_offset if followed by LOAD_METHOD and calls. OP bisected to #31933, 3011a097bd. (15... Thanks for doing that.)

    Tests in additional to those already posted above:

    import dis, abc
    import ast as art
    dix = dis
    ast = art
    
    def f():
        dix.a()  # Is first name affected if not bound name? (No.)
        dis.a()  # Is bound-file name affected if not first? (Yes.)
        abc.a()  # Are names other than 'dis' affected?  (Yes.)
        art.a()  # Is bound name not file name affected? (Yes.)
        ast.a()  # Is file name not bound name affected?  (No.)
    
    for i in dis.Bytecode(f):
        if i.opname == "LOAD_GLOBAL":
            print(f'{i.opname:12s}', i.positions)
    

    outputs

    LOAD_GLOBAL  Positions(lineno=8, end_lineno=8, col_offset=4, end_col_offset=7)
    LOAD_GLOBAL  Positions(lineno=9, end_lineno=9, col_offset=4, end_col_offset=9)
    LOAD_GLOBAL  Positions(lineno=10, end_lineno=10, col_offset=4, end_col_offset=9)
    LOAD_GLOBAL  Positions(lineno=11, end_lineno=11, col_offset=4, end_col_offset=9)
    LOAD_GLOBAL  Positions(lineno=12, end_lineno=12, col_offset=4, end_col_offset=7)
    

    @terryjreedy
    Copy link
    Member

    Thank you Brandt for the nice fix and tests.
    @15r10nk Thank you for persisting through the fog so we fixed a real bug which would surely have bitten someone if released.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes 3.12 bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants