Skip to content

bpo-40661: Fix segfaults when parsing invalid input #20159

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
wants to merge 1 commit into from

Conversation

gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented May 17, 2020

Fixes bpo-40661. (In fact this just is the patch from @pablogsal from
that issue.)

Note that for an extreme benchmark (xxl.py) this caused a 40% slowdown
on my Mac. However for an "average" benchmark (stdlib) there was no
significant difference.

https://bugs.python.org/issue40661

Fixes bpo-40661.  (In fact this just is the patch from @pablogsal from
that issue.)

Note that for an extreme benchmark (xxl.py) this caused a 40% slowdown
on my Mac.  However for an "average" benchmark (stdlib) there was no
significant difference.
@gvanrossum gvanrossum changed the title Fix segfaults when parsing invalid input bpo-40661: Fix segfaults when parsing invalid input May 17, 2020
@lysnikolaou
Copy link
Member

reproducer.py, the file @pablogsal uploaded on the issue, does not segfault even if we only check for p->error_indicator == 1 and leave the call to PyErr_Occurred() out.

Comment on lines +684 to +688
self.print("if (p->error_indicator == 1 || PyErr_Occurred()) {")
with self.indent():
self.print("p->error_indicator = 1;")
self.print("return NULL;")
self.print("}")
Copy link
Member

Choose a reason for hiding this comment

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

I'd actually move this within the scope block of the alternative (after the with self.indent() below).

@gvanrossum
Copy link
Member Author

gvanrossum commented May 17, 2020 via email

@lysnikolaou
Copy link
Member

Yes, of course! Should we close it and open a new one, since I can't push to a branch of the upstream repo?

@gvanrossum
Copy link
Member Author

gvanrossum commented May 17, 2020 via email

@lysnikolaou
Copy link
Member

Ok, closing.

@zware zware deleted the fix-segfaults branch May 18, 2020 23:05
@gvanrossum
Copy link
Member Author

Note that for an extreme benchmark (xxl.py) this caused a 40% slowdown
on my Mac. However for an "average" benchmark (stdlib) there was no
significant difference.

To follow up: I believe this may have been an exaggerated outcome because I built with debug flags on (and had forgotten about it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants