Skip to content
This repository has been archived by the owner on Jul 5, 2023. It is now read-only.

[2.7, 3.6] Fix invalid read memory errors #38

Merged
merged 1 commit into from
Apr 7, 2017
Merged

Conversation

ddfisher
Copy link
Collaborator

@ddfisher ddfisher commented Apr 7, 2017

Potential fix for #36.

@gvanrossum
Copy link
Member

This seems a reasonable hypothesis. You can merge it if either you have proof that this fixes it or you need this to be merged before you can construct said proof.

@gvanrossum
Copy link
Member

(And please say which it is when you do merge it. :-)

@ddfisher
Copy link
Collaborator Author

ddfisher commented Apr 7, 2017

Even if this doesn't fix #36, these are definitely invalid memory access errors and should be fixed. Most of them are harmless, but it looks like there are two that could be causing the errors we've been seeing.

So far, I've been unable to repro the problems locally, so I can't be confident that this fixes them.


if (TYPE(CHILD(n, i)) == TYPE_COMMENT) {
if (i < NCH(n) && TYPE(CHILD(n, i)) == TYPE_COMMENT) {
Copy link
Collaborator Author

@ddfisher ddfisher Apr 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gvanrossum: It seems very likely to me that this error is the cause of the Function has duplicate type signatures you saw in #36. A spurious type comment here (which could happen from the uninitialized read) would produce the correct error, and this only happens with a *arg, which lines up with what you saw.

@@ -1530,7 +1530,7 @@ ast_for_arguments(struct compiling *c, const node *n)
int res = 0;
i += 2; /* now follows keyword only arguments */

if (TYPE(CHILD(n, i)) == TYPE_COMMENT) {
if (i < NCH(n) && TYPE(CHILD(n, i)) == TYPE_COMMENT) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gvanrossum: This could plausibly be the cause of SystemError: <built-in function _parse> returned a result with an error set with crazy line numbers. The line number here is clearly based on uninitialized memory in the spurious case.

@gvanrossum
Copy link
Member

gvanrossum commented Apr 7, 2017 via email

@gvanrossum gvanrossum merged commit bd92216 into master Apr 7, 2017
@gvanrossum gvanrossum deleted the fix-memory-errors branch April 7, 2017 22:04
@gvanrossum
Copy link
Member

I had an internal diff that was consistently failing with this problem and it is passing with this PR applied, so I am confident that we've nailed the issue.

gvanrossum pushed a commit to python/mypy that referenced this pull request Apr 10, 2017
This includes the important uninitialized read fix (python/typed_ast#38).

For cross-reference, see also #3127 (which is now closed).
tbbharaj pushed a commit to tbbharaj/typed_ast that referenced this pull request Dec 6, 2021
Issue python#16: accept arbitrary buffer-compatible objects
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants