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
compile() raises SystemError if called from except clause #55650
Comments
Normal:
>>> compile('1 = 1', '<string>', 'exec')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "<string>", line 1
SyntaxError: can't assign to literal
SystemError is raised instead of SyntaxError:
>>> try: abcde
... except NameError:
... compile('1 = 1', '<string>', 'exec')
...
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
NameError: name 'abcde' is not defined
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 3, in <module>
SystemError: Objects/tupleobject.c:126: bad argument to internal function Error can be discovered by calling dis.dis('1 = 1'). |
Apparently ast_error_finish calls PyTuple_GetItem with a value that is not a tuple, but a SyntaxError instance (in Python/ast.c line 112). It seems that ast_error_finish expects that PyErr_Fetch will return the exception value as a tuple, and in some cases this seems correct (for example when not in an except clause), but not in this case. I don't know much about Python exception handling in C, but it seems to me, that it shouldn't expect always a tuple (see also http://docs.python.org/dev/py3k/c-api/exceptions.html#PyErr_NormalizeException). |
Right. In most cases, "PyErr_SetObject(PyExc_SyntaxError, tuple);" will store the untouched tuple in tstate->curexc_value, *except* when "Implicit exception chaining" occurs, in which case the exception is normalized. ast_error_finish() should not expect a tuple in all cases, and should probably normalize the exception as well. |
Here is a patch. I wasn't sure, where to put the test, so I put it in test_ast. |
You can put the test in test_compile. |
Okay, here is a new patch with the test in the correct place (I hope). |
Why is the exception normalized at the end? I suppose it's because when value is an exception instance, it's replaced by a tuple, but the original value has to be recreated at the end. So in some cases, the SyntaxError object is created twice... If PyErr_NormalizeException() can't be avoided, I suggest to call it at the start, just after PyErr_Fetch, and use the PySyntaxErrorObject* structure directly to get the file name and line numbers. |
Yeah, it is because ast_error_finish creates a new tuple to use as the exception value. (It creates a new (errstr, (filename, lineno, offset, loc)) tuple from the original (errstr, lineno, offset) tuple). And yes, in some cases the SyntaxError instance is created twice. I wasn't sure if it's okay to simply replace the args field of a PyBaseExceptionObject. I don't know, if PyErr_NormalizeException() can be avoided, you wrote, that it "should probably normalize the exception as well". Would it be better, if we, when got an exception instance, create the new tuple from the info, and replace the args field of the instance with it? (But it also seems to me, that the SyntaxError objects have other fields as well, so probably we should modify them also. That's why I thought that calling PyErr_NormalizeException with the new tuple is the simplest thing to do, becuase I guess that'll take care of all fields automatically.) |
There is an XXX just before the definition of ast_error. Wouldn't it be The idea is to merge ast_error() and ast_error_finish(). |
You could also call PyErr_NormalizeException at the beginning, and update the fields directly in the PySyntaxErrorObject structure. No need to deal with any tuple. |
Sorry, but I don't really understand. If I call PyErr_NormalizeException at the beginning, the SyntaxError instance will be initialized with the wrong 3-tuple: (errstr, lineno, offset). If after that I update the msg, filename, ... fields, they will be correct, but I think the args field still will be the wrong 3-tuple. So I'll have to still create the new tuple, and replace the args field, right? |
hmm, you are right, of course. I forgot that e.args is part of the SyntaxError members. |
So, I see four possible solutions:
My patch currently does 1. I can write a new patch, if the experts tell me what is the best solution from the four (or some other I didn't think of). |
I'd choose solution 3, but instead of calling SyntaxError.__init__, call PyErr_NormalizeException(). |
Err... sorry, I don't understand again: If we get a tuple, create a new, store it without normalization. That's okay, I understand. If we get a SyntaxError instance, then take its args field, create the new tuple. Then call PyErr_NormalizeException(), with: Sorry for all these questions... I'd just like to help. |
This no longer seems to be a problem in Python 3.2, 3.3, or 3.4. |
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: