Skip to content

The case generator tool may generate incorrect code for explicit gotos. #102294

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

Closed
markshannon opened this issue Feb 27, 2023 · 2 comments
Closed
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@markshannon
Copy link
Member

Either the tool needs to reject all explicit gotos, or it needs to understand them.
Rejecting them all seems easier, and not that much of a burden on the user.

Example of incorrectly generated code: #102287

@markshannon markshannon added the type-bug An unexpected behavior, bug, or error label Feb 27, 2023
@gvanrossum
Copy link
Member

gvanrossum commented Feb 27, 2023

It's not that simple. If the user code wants an error exit before it has DECREF'ed all its stack inputs, it must use goto error, otherwise the stack values are leaked. OTOH all error exits after the inputs have been DECREF'ed must use ERROR_IF(), else the values will be double-DECREF'ed.

In order to properly implement the behavior you ask for, the generator would have to understand all forms of DECREF, so it can track what is the correct way to exit. If we insist there are no "bare goto error" statements, we will need to rewrite ~50 instructions to get rid of those one way or another (probably inserting extra DECREFs).

(I counted and there are nearly 100 ERROR_IF() calls but nearly 50 goto error statements, in Python/bytecodes.c.)

@iritkatriel iritkatriel added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 28, 2023
@markshannon
Copy link
Member Author

bytecodes.c now only contains gotos to a small set of special labels, which seems fine for correctness.
The only exception is CALL_LIST_APPEND which does its own refcount handling.
I think we can tolerate one special case.

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-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants