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
Bug in IMPORT_NAME #71539
Comments
Seems there is a bug in the implementation of the IMPORT_NAME opcode in ceval.c. If the level argument is -1, it is not passed to __import__ (should never happen, but looks correct). If it is an integer != -1 in C long range, it is passed to __import__ (this is correct). But if it is not integer (e.g. None) or can't be converted to C long, an exception is set and __import__ is called with level and not cleared error (this is wrong). In correct bytecode the level argument can be either integer or None. Default __import__ accepts only integers as the level argument and checks the range. Proposed patch makes the code always passing the level argument to __import__ unless it is None. |
My suspicion is the -1 aspect is a hold-over from that being the default level value in Python 2.7 that no one removed. The patch LGTM. Do we need a test for this for some reason? |
Thak you for the explanation Brett. Since current "import" without "from" is not broken, I suppose that the raised exception is cleared somewhere later. The patch doesn't have visible effect (unless may be using nonstandard loaders or __import__, don't know). It just cleans up the code, avoid raising unneeded exception (small performance boost) and may be fix some hidden non-reproducible bug. |
Then I say go ahead and commit it. |
Sorry, I was wrong. The level argument is always non-negative integer (if a bytecode is not corrupted). The second branch can be just removed. Following patch also fixes the validation of the ImportFrom AST node. |
LGTM |
New changeset e3164c9edb0b by Serhiy Storchaka in branch 'default': |
Thanks Brett. |
You broke Python! Example: http://buildbot.python.org/all/builders/AMD64%20Snow%20Leop%203.x/builds/4853/steps/test/logs/stdio ====================================================================== Traceback (most recent call last):
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_ast.py", line 757, in test_importfrom
self.stmt(imp, "level less than -1")
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_ast.py", line 579, in stmt
self.mod(mod, msg)
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_ast.py", line 571, in mod
self.assertIn(msg, str(cm.exception))
AssertionError: 'level less than -1' not found in 'Negative ImportFrom level' |
New changeset e5063a82f490 by Serhiy Storchaka in branch 'default': |
Thanks Victor! |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: