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

in debug mode, compile(ast) fails with an assertion error if an AST node has no line number information #65584

Closed
ztane mannequin opened this issue Apr 29, 2014 · 7 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@ztane
Copy link
Mannequin

ztane mannequin commented Apr 29, 2014

BPO 21385
Nosy @terryjreedy, @vstinner, @benjaminp, @bitdancer, @PCManticore, @ztane
Superseder
  • bpo-26107: PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer
  • Files
  • astlinenotest.py: an example of how to trigger the assertion failure using ASTs and compile
  • 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 = <Date 2016-01-20.11:35:39.832>
    created_at = <Date 2014-04-29.09:00:10.830>
    labels = ['interpreter-core', 'type-crash']
    title = 'in debug mode, compile(ast) fails with an assertion error if an AST node has no line number information'
    updated_at = <Date 2016-01-20.11:35:39.830>
    user = 'https://github.com/ztane'

    bugs.python.org fields:

    activity = <Date 2016-01-20.11:35:39.830>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2016-01-20.11:35:39.832>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2014-04-29.09:00:10.830>
    creator = 'ztane'
    dependencies = []
    files = ['35090']
    hgrepos = []
    issue_num = 21385
    keywords = []
    message_count = 7.0
    messages = ['217502', '217806', '218383', '218412', '218700', '218723', '258670']
    nosy_count = 6.0
    nosy_names = ['terry.reedy', 'vstinner', 'benjamin.peterson', 'r.david.murray', 'Claudiu.Popa', 'ztane']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'needs patch'
    status = 'closed'
    superseder = '26107'
    type = 'crash'
    url = 'https://bugs.python.org/issue21385'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @ztane
    Copy link
    Mannequin Author

    ztane mannequin commented Apr 29, 2014

    We had had problems with our web service occasionally hanging and performing poorly, and as we didn't have much clue about the cause of these, we decided to continuously run our staging build under debug enabled python 3.4, and then attaching gdb as needed. To much dismay we found out that our code generating code that builds AST trees and then compiles them to modules is dumping cores on the debug version.

    The assertion is the much discussed "linenumbers must grow monotonically" at http://hg.python.org/cpython/file/04f714765c13/Python/compile.c#l3969

    In our case, the AST is generated from a HTML template with embedded python parts; as we could approximately point out much of the corresponding code in the source template, we decided to reuse the linenumbers in AST, and things seemed to work quite nicely and usually we could get usable tracebacks too.

    Under debug build, however, as the ordering of some constructs in the source language are different from python, we need to discard *all* linenumbers and only after then use fix_missing_locations, and thus get completely unusable traces from these parts of code, all happening on line 1. Just using fix_missing_locations does not work. Likewise the rules for which parts of the tree should come in which order in the lnotab is quite hard to deduce.

    It seems to me that when the lnotab was created, no one even had in mind that there would be an actually useful AST module that would be used for code generation. Considering that there have been other calls for breaking the correspondence of bytecode addresses to monotonically growing linenumbers, I want to reopen the discussion about changing the lnotab structures now to allow arbitrary mapping of source code locations to bytecode, and especially about the need for this assertion in the debug builds at all.

    Attached is an example of code that appends a function to an existing module syntax tree, run under python*-dbg it dumps the core with "Python/compile.c:nnnn: assemble_lnotab: Assertion `d_lineno >= 0' failed." Ofc in this simple case it is easy to just modify the linenumbers so that function "bar" would come after "foo", however in some cases it is hard to know the actual rules; fix_missing_locations does not do this right at all.

    I am also pretty sure most of the existing code that combine parsed and generated ASTs and then compile the resulting trees also would fail that assert, but no one is ever running their code under debug builds.

    @ztane ztane mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Apr 29, 2014
    @terryjreedy
    Copy link
    Member

    3.1 to 3.3 only get internet security patcches, and I don't believe this is such an issue.

    It is not clear from the title (a statement, not a request) and your text what specific patch you would like. If you want to 'reopen a discussion', please post on python-ideas. It appears that your idea is to abandon a rule that does not work. But it is not clear from a first reading exactly what alternative you want.

    @vstinner
    Copy link
    Member

    With the development version (Python 3.5), I reproduce the crash when Python is compiled in debug mode:

    $ ./python astlinenotest.py 
    python: Python/compile.c:3975: assemble_lnotab: Assertion `d_lineno >= 0' failed.
    Abandon (core dumped)

    The problem is that astlinenotest.py creates an AST node without lineno information, which makes an assertion to fail in the compiler.

    In my astoptimizer project, I use this function to not have to worry of the lineno:

    def copy_lineno(node, new_node):
        ast.fix_missing_locations(new_node)
        ast.copy_location(new_node, node)
        return new_node

    @vstinner vstinner changed the title Compiling modified AST crashes on debug build unless linenumbering discarded in debug mode, compile(ast) fails with an assertion error if an AST node has no line number information May 12, 2014
    @vstinner
    Copy link
    Member

    We can maybe modify the compiler to use the line number 1 if the line information is missing?

    @bitdancer
    Copy link
    Member

    Victor: in the production code discussed in the original posting, there *are* line numbers, and they are meaningful; they just aren't monotonically increasing.

    I believe the request here is to simply remove the assert. (If we did that, we'd have to also add tests that python itself was generating monotonically increasing line number, and make some doc changes.)

    @terryjreedy
    Copy link
    Member

    Summary of this post: compile currently checks user input with assert; this is a bug that should be changed.

    I re-read astlinenotest.py and realized that FunctionDef is included in '*' and not some omitted import. (The latter is common for code posted on python-list, if not here. This illustrates why PEP-8 deprecates 'import *'; import ast ... ast.FunctionDef would have been clear on first reading.) So I ran the module in installed 3.4 and repository debug 3.5 and got the assert with the latter.

    "It seems to me that when the lnotab was created, no one even had in mind that there would be an actually useful AST module that would be used for code generation." I am pretty sure this is correct. I suspect that the assert in question was originally intended to test the logic of the internal syntax tree line number generation. But now it also tests user input, which I and may others think is a bad idea.

    A solution between removing the assert (and the internal check) and converting it to, say, ValueError("decreasing line no in input ast"), and thereby stopping code that now normally and legitimately works, would be to skip the asserts when the compile input is an ast instead of code.

    @vstinner
    Copy link
    Member

    Good news: this issue has been fixed by the commit of the issue bpo-26107.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    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) type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants