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

3 bugs fixed: handling of SyntaxErrors in symbol table build #38750

Closed
staschuk mannequin opened this issue Jun 30, 2003 · 2 comments
Closed

3 bugs fixed: handling of SyntaxErrors in symbol table build #38750

staschuk mannequin opened this issue Jun 30, 2003 · 2 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs)

Comments

@staschuk
Copy link
Mannequin

staschuk mannequin commented Jun 30, 2003

BPO 763201
Files
  • symtable-errors.patch
  • 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 2003-07-01.17:31:21.000>
    created_at = <Date 2003-06-30.13:48:26.000>
    labels = ['interpreter-core']
    title = '3 bugs fixed: handling of SyntaxErrors in symbol table build'
    updated_at = <Date 2003-07-01.17:31:21.000>
    user = 'https://bugs.python.org/staschuk'

    bugs.python.org fields:

    activity = <Date 2003-07-01.17:31:21.000>
    actor = 'staschuk'
    assignee = 'jhylton'
    closed = True
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2003-06-30.13:48:26.000>
    creator = 'staschuk'
    dependencies = []
    files = ['5429']
    hgrepos = []
    issue_num = 763201
    keywords = ['patch']
    message_count = 2.0
    messages = ['44170', '44171']
    nosy_count = 2.0
    nosy_names = ['jhylton', 'staschuk']
    pr_nums = []
    priority = 'high'
    resolution = 'fixed'
    stage = None
    status = 'closed'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue763201'
    versions = ['Python 2.3']

    @staschuk
    Copy link
    Mannequin Author

    staschuk mannequin commented Jun 30, 2003

    All three bugs found in both 2.2.3 and 2.3b2. It's one
    patch
    instead of three because the fixes share a refactoring of
    Python/compile.c:symtable_build.

    Bug 1: Wrong file name

        import symtable
        symtable.symtable('def foo(a): global a', 'spam', 'exec')

    The error message says '<string>' instead of 'spam'.
    (Cause: PyNode_CompileSymtable doesn't set
    st_filename.)

    Bug 2: Memory leak

        while True:
            try:
                compile('def foo(a): global a', 'spam', 'exec')
            except SyntaxError:
                pass

    (Cause: symtable_build doesn't free c->c_symtable on
    error.)

    The patch is missing a test case for this one; I don't see
    how to write it.

    Bug 3: Exception clobbered

        def foo(a):
            global a  # SyntaxError
    
        def bar():
            b = 1
            global b  # SyntaxWarning

    Running this as a script, the SyntaxWarning is issued,
    and then
    the interpreter dies silently (that is, without printing a
    traceback reporting the SyntaxError). Compiling it with
    compile()
    causes a SystemError. (Cause: see below.)

    What to do about bugs 1 and 2 is obvious, but bug 3
    (which was
    actually reported on c.l.py) is not so clear to me. Here's
    how
    the problem occurs:

    symtable_global() sees the global statement in foo(),
    raises a
    SyntaxError, increments st_errors, and returns.
    Processing
    continues. Later, symtable_global() sees the global
    statement in
    bar() and issues a warning by (indirectly) calling
    PyErr_WarnExplicit(). This call clears the SyntaxError,
    which is
    still pending at that time. (It's actually cleared during the
    attempt to import the warnings module, in
    PyImport_Import, which
    seems to think the exception was raised in
    PyEval_GetGlobals.)
    But st_errors is still > 0, as it should be, so
    symtable_build()
    returns -1 (resp. PyNode_CompileSymtable, NULL), its
    callers
    return their error values, etc., until eventually PyErr_Print
    tries to print the exception that isn't there.

    What the patch implements is this:

    Do not issue SyntaxWarnings if an exception is pending;
    fail
    instead and let that exception propagate. Also, as a
    defensive
    measure against other bugs of this type (present and
    future), when
    returning with error from symtable_build(), verify that
    there's an
    exception pending (and raise SystemError if not). Finally,
    refactor symtable_build() so PyNode_CompileSymtable
    can use it and
    thereby benefit from that defensive measure.

    Alternatives (and why I don't like them):

    1. Do not try to continue processing after a SyntaxError is
      raised. (Seems like the Right Thing to me, but also
      seems to be
      contrary to the intent of the existing code. There are
      oodles of
      places in compile.c which call symtable_node without
      checking
      st_errors immediately afterwards.)

    2. Put the check for a pending exception in
      PyErr_WarnExplicit()
      instead of in the helper function in compile.c. (Doesn't
      seem
      like a common enough coding error to merit a check
      there. In
      symtable_node etc we deliberately let SyntaxErrors, er,
      "pend"
      while we do a bit more compiling, so *there* it's worth
      checking.
      Note that jcompile() already has a check for something
      similar,
      though not for the symbol-table-building phase.)

    3. Squirrel the pending exception away, issue the
      warning, then
      restore the exception. (Not worth the bother, IMO. And
      if the
      warning gets strengthened into an exception, should that
      exception
      or the squirrelled one propagate? Ick.)

    @staschuk staschuk mannequin closed this as completed Jun 30, 2003
    @staschuk staschuk mannequin assigned jhylton Jun 30, 2003
    @staschuk staschuk mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jun 30, 2003
    @staschuk staschuk mannequin closed this as completed Jun 30, 2003
    @staschuk staschuk mannequin assigned jhylton Jun 30, 2003
    @staschuk staschuk mannequin added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jun 30, 2003
    @staschuk
    Copy link
    Mannequin Author

    staschuk mannequin commented Jul 1, 2003

    Logged In: YES
    user_id=666873

    Just noticed another alternative: leave the checking of
    whether a global statement declares a function parameter
    global to the later pass. There's already a check for this
    condition in symtable_load_symbols (with a different error
    message, which is a subject for another patch).

    The problem with this idea is that it breaks
    symtable.symtable(), which only uses symtable_build. Thus this
    function would no longer raise SyntaxErrors for this case.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 9, 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)
    Projects
    None yet
    Development

    No branches or pull requests

    0 participants