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

Implement PEP 626 -- Precise line numbers for debugging #86412

Closed
markshannon opened this issue Nov 2, 2020 · 28 comments
Closed

Implement PEP 626 -- Precise line numbers for debugging #86412

markshannon opened this issue Nov 2, 2020 · 28 comments
Assignees
Labels
3.10 only security fixes type-feature A feature request or enhancement

Comments

@markshannon
Copy link
Member

BPO 42246
Nosy @nascheme, @nedbat, @methane, @markshannon, @pablogsal, @alexhenrie, @isidentical
PRs
  • bpo-42246: Partial implementation of PEP 626. #23113
  • bpo-42246: Bump magic number. #23245
  • bpo-42246: Eliminate jumps to exit blocks by copying those blocks. #23251
  • bpo-42246: Fix memory leak in compiler #23256
  • bpo-42246: Make sure that line number is correct after a return, as required by PEP 626 #23495
  • bpo-42246: Don't forget the entry block when ensuring that all exits have a line number #23636
  • bpo-42246: Remove DO_NOT_EMIT_BYTECODE macros, so that while loops and if statements conform to PEP 626. #23743
  • bpo-42246: Make sure that f_lasti, and thus f_lineno, is set correctly after raising or reraising an exception #23803
  • bpo-42246: Don't eliminate jumps to jump, if it will break PEP 626. #23896
  • bpo-42246: Delete the now unused c_do_not_emit_bytecode field. #24094
  • bpo-43358: Fix bad free in assemble function #24697
  • bpo-43372: Re-generate frozen code for __hello__. #24708
  • bpo-43372: Use BUILDPYTHON for regen-frozen. #24714
  • bpo-43372: Use _freeze_importlib for regen-frozen. #24759
  • bpo-42246: Mark POP_TOP at end of expression statement as artificial, to conform to PEP 626. #24860
  • bpo-42246: Add PEP 626 to what's new in 3.10. #24892
  • bpo-42246: Mention that code.co_lnotab is deprecated in what's new for 3.10. #24902
  • Files
  • IMAG0629_1(1).jpg
  • IMAG0629_1.jpg
  • IMAG0629_1.jpg
  • 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 = 'https://github.com/markshannon'
    closed_at = <Date 2021-03-02.11:08:24.441>
    created_at = <Date 2020-11-02.15:32:42.250>
    labels = ['type-feature', '3.10']
    title = 'Implement PEP 626 -- Precise line numbers for debugging'
    updated_at = <Date 2021-03-17.11:46:38.644>
    user = 'https://github.com/markshannon'

    bugs.python.org fields:

    activity = <Date 2021-03-17.11:46:38.644>
    actor = 'Mark.Shannon'
    assignee = 'Mark.Shannon'
    closed = True
    closed_date = <Date 2021-03-02.11:08:24.441>
    closer = 'Mark.Shannon'
    components = []
    creation = <Date 2020-11-02.15:32:42.250>
    creator = 'Mark.Shannon'
    dependencies = []
    files = ['49669', '49670', '49671']
    hgrepos = []
    issue_num = 42246
    keywords = ['patch']
    message_count = 28.0
    messages = ['380231', '380245', '380268', '380276', '380809', '380846', '380865', '380880', '382312', '382492', '382871', '382872', '382884', '382886', '382888', '382889', '382890', '382950', '382985', '383042', '383159', '383245', '383643', '387910', '388217', '388836', '388837', '388900']
    nosy_count = 8.0
    nosy_names = ['nascheme', 'nedbat', 'methane', 'Mark.Shannon', 'pablogsal', 'alex.henrie', 'BTaskaya', 'zhtw1234']
    pr_nums = ['23113', '23245', '23251', '23256', '23495', '23636', '23743', '23803', '23896', '24094', '24697', '24708', '24714', '24759', '24860', '24892', '24902']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue42246'
    versions = ['Python 3.10']

    @markshannon
    Copy link
    Member Author

    Implementation of PEP-626 requires:

    1. Implementation of the new line number table and associated APIs.
    2. Removal of BEGIN_DO_NOT_EMIT_BYTECODE and END_DO_NOT_EMIT_BYTECODE from the compiler as they do not understand line numbers and may remove lines from the bytecode that they shouldn't.
    3. Enhance compiler front-end and CFG optimizer to avoid the negative performance impact of PEP.
      a) Duplicate the tests in while blocks to avoid the extra jump instruction at the end of the loop.
      b) Duplicate and renumber terminator blocks that have no line numbers.

    Guaranteeing that f_lineno is correct without hurting performance
    -----------------------------------------------------------------

    PEP-626 mandates that the f_lineno attribute of a frame is always correct, even after a return or raise, but we don't want to hurt performance.
    Since the interpreter ensures that the f_lasti attribute of a frame is always correct, we can ensure correctness of f_lineno at zero cost, by ensuring that all RETURN_VALUE, RAISE_VARARGS and RERAISE instruction have a non-negative line number. Then f_lineno can always be lazily computed from f_lasti.

    The front-end generates artificial RERAISEs and RETURN_VALUE that have no line number. To give these instructions a valid line number we can take advantage of the fact that such terminator blocks (blocks with no successors) can be freely duplicated. Once duplicated, each terminator block will have only one predecessor and can acquire the line number of the preceding block, without causing false line events.

    @markshannon markshannon added the 3.10 only security fixes label Nov 2, 2020
    @markshannon markshannon self-assigned this Nov 2, 2020
    @markshannon markshannon added type-feature A feature request or enhancement 3.10 only security fixes labels Nov 2, 2020
    @markshannon markshannon self-assigned this Nov 2, 2020
    @markshannon markshannon added the type-feature A feature request or enhancement label Nov 2, 2020
    @pablogsal
    Copy link
    Member

    I'm happy that we are removing BEGIN_DO_NOT_EMIT_BYTECODE and END_DO_NOT_EMIT_BYTECODE but could you elaborate how this is related? These macros protect the compiler from emitting bytecode that corresponds to deaf code and by definition, unreachable. Could you give an example of a situation in which they create something that messes up the line numbers? Is this something to be with cleanup blocks in dead code or something similar?

    @markshannon
    Copy link
    Member Author

    The following code is completely eliminated by the macros.

    1. if 0:
    2. secret_debugging_code()
      

    PEP-626 says that all executed lines of code must generate trace events,
    so we need to emit an instruction for line 1.

    Dead code elimination will remove the secret_debugging_code(), but leave the test. The peephole optimiser can then reduce it to a NOP, but won't eliminate it as it is the only instruction for line 1.

    @pablogsal
    Copy link
    Member

    Dead code elimination will remove the secret_debugging_code(), but leave the test. The peephole optimiser can then reduce it to a NOP, but won't eliminate it as it is the only instruction for line 1.

    Gotcha. I am pretty sure that this will have a similar problem as the coverage people were claiming when we were not properly removing all dead code (slightly less coverage percentage). This is not a problem of course, but we should ping the coverage folks so they are aware of this.

    @markshannon
    Copy link
    Member Author

    New changeset 877df85 by Mark Shannon in branch 'master':
    bpo-42246: Partial implementation of PEP-626. (GH-23113)
    877df85

    @markshannon
    Copy link
    Member Author

    New changeset cc75ab7 by Mark Shannon in branch 'master':
    bpo-42246: Eliminate jumps to exit blocks by copying those blocks. (bpo-23251)
    cc75ab7

    @vstinner
    Copy link
    Member

    New changeset 877df85 by Mark Shannon in branch 'master':
    bpo-42246: Partial implementation of PEP-626. (GH-23113)

    This change introduced reference leaks:
    https://buildbot.python.org/all/#builders/384/builds/100

    5 tests failed:
    test_asyncgen test_builtin test_coroutines test_exceptions
    test_syntax

    For example:

    $ ./python -m test test_syntax -R 3:3 -m test.test_syntax.SyntaxTestCase.test_no_indent
    0:00:00 load avg: 1.59 Run tests sequentially
    0:00:00 load avg: 1.59 [1/1] test_syntax
    beginning 6 repetitions
    123456
    ......
    test_syntax leaked [27, 27, 27] references, sum=81
    test_syntax leaked [20, 20, 20] memory blocks, sum=60
    test_syntax failed

    == Tests result: FAILURE ==

    1 test failed:
    test_syntax

    Total duration: 955 ms
    Tests result: FAILURE

    @markshannon
    Copy link
    Member Author

    New changeset fd009e6 by Mark Shannon in branch 'master':
    bpo-42246: Fix memory leak in compiler (GH-23256)
    fd009e6

    @markshannon
    Copy link
    Member Author

    New changeset 5977a79 by Mark Shannon in branch 'master':
    bpo-42246: Make sure that line number is correct after a return, as required by PEP-626 (GH-23495)
    5977a79

    @markshannon
    Copy link
    Member Author

    New changeset eaccc12 by Mark Shannon in branch 'master':
    bpo-42246: Don't forget the entry block when ensuring that all exits have a line number (GH-23636)
    eaccc12

    @nedbat
    Copy link
    Member

    nedbat commented Dec 11, 2020

    Mark, BTW: I have run the coverage.py test suite on 3.10.0a3, and as expected there are failures. I haven't dug into it yet to see what looks expected and what does not. I also see there are still changes happening on master, so I'm not sure when to commit the time.

    @markshannon
    Copy link
    Member Author

    Ned,
    What are the failures?

    I'd like to take a look, to see if things are as expected, and if there are any tests we can add to CPython.

    @zhtw1234
    Copy link
    Mannequin

    zhtw1234 mannequin commented Dec 11, 2020

    幹你娘

    @zhtw1234
    Copy link
    Mannequin

    zhtw1234 mannequin commented Dec 11, 2020

    全家死光

    @zhtw1234
    Copy link
    Mannequin

    zhtw1234 mannequin commented Dec 11, 2020

    幹你娘
    我是渣男專打老婆騙吃拐幹

    @zhtw1234
    Copy link
    Mannequin

    zhtw1234 mannequin commented Dec 11, 2020

    你全家死光

    @zhtw1234
    Copy link
    Mannequin

    zhtw1234 mannequin commented Dec 11, 2020

    幹你娘

    @rhettinger rhettinger changed the title Implement PEP 626 Implement PEP 626 -- Precise line numbers for debugging Dec 11, 2020
    @rhettinger rhettinger changed the title Implement PEP 626 Implement PEP 626 -- Precise line numbers for debugging Dec 11, 2020
    @nedbat
    Copy link
    Member

    nedbat commented Dec 14, 2020

    Mark, I'm categorizing and characterizing the test failures. Here's the start of it: https://gist.github.com/nedbat/6c5dedde9df8d2de13de8a6a39a5f112 Let me know what other information would be useful.

    @markshannon
    Copy link
    Member Author

    Thanks Ned, that's really helpful. I'll go through those points:

    Code after break/continue is no longer compiled.
    Expected

    First line number of modules
    Expected

    Except clause when no exception
    https://bugs.python.org/issue42634

    Double loops (this also covers End-of-loop jumps, I think)
    https://bugs.python.org/issue42635

    I want to merge #23743 before I fix any of the others, but here is a summary of what I think are the root causes.

    if-break
    Exit block duplication does not preserve line number of jump to final block

    Finally handling
    Combination of two things. Not preserving line numbers when performing jump-to-jump elimination and not marking try cleanup code as artificial.

    @markshannon
    Copy link
    Member Author

    New changeset 8473cf8 by Mark Shannon in branch 'master':
    bpo-42246: Remove DO_NOT_EMIT_BYTECODE macros, so that while loops and if statements conform to PEP-626. (GH-23743)
    8473cf8

    @markshannon
    Copy link
    Member Author

    #23780 fixes the finally handling.
    The if-break case was fixed by earlier changes.

    @markshannon
    Copy link
    Member Author

    New changeset bf353f3 by Mark Shannon in branch 'master':
    bpo-42246: Make sure that f_lasti, and thus f_lineno, is set correctly after raising or reraising an exception (GH-23803)
    bf353f3

    @markshannon
    Copy link
    Member Author

    New changeset 28b75c8 by Mark Shannon in branch 'master':
    bpo-42246: Don't eliminate jumps to jumps, if it will break PEP-626. (GH-23896)
    28b75c8

    @markshannon
    Copy link
    Member Author

    All done :)

    @nascheme
    Copy link
    Member

    nascheme commented Mar 6, 2021

    New changeset 87ec26b by Neil Schemenauer in branch 'master':
    bpo-43372: Use _freeze_importlib for regen-frozen. (GH-24759)
    87ec26b

    @nedbat
    Copy link
    Member

    nedbat commented Mar 16, 2021

    Is there a reason PEP-626 isn't yet mentioned in https://docs.python.org/3.10/whatsnew/3.10.html ?

    @markshannon
    Copy link
    Member Author

    No. We should add it.

    @pablogsal
    Copy link
    Member

    PEP-626 deprecates co_lnotab, co_lnotab:

    https://www.python.org/dev/peps/pep-0626/#id15

    This doesn't seem to be mentioned in the What's new document and is quite important. Mark, do you mind creating a PR for this? I could do it and add you as a reviewer if you wish

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants