Skip to content
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

Apply the recursion limit to the parser instead of just to the printer. #49

Merged
merged 1 commit into from
Jun 18, 2021

Conversation

pcwalton
Copy link
Contributor

@pcwalton pcwalton commented Jun 17, 2021

Raphael Salas found more stack overflows involving very large symbols that
failed while the parser was trying to skip over tokens. These are fixed by
moving the recursion limiter from the printer to the parser, and updating the
printer to use the parser's limiter.

The failing cases are added as unit tests.

I'm submitting this for review while I run the fuzzer overnight.

r? @alexcrichton

@pcwalton
Copy link
Contributor Author

Let me know if you'd like me to move the tests in v0.rs to a separate file :)

@pcwalton pcwalton force-pushed the parse-limit branch 2 times, most recently from 231371a to 74dccc6 Compare June 17, 2021 04:13
@pcwalton pcwalton marked this pull request as draft June 17, 2021 13:55
@pcwalton
Copy link
Contributor Author

Fuzzing found a few more stack overflows. Let me fix them before merging this.

@alexcrichton
Copy link
Member

Oh dear, thanks for this though!

I'm happy to merge when you feel this is ready, though

Raphael Salas found more stack overflows involving very large symbols that
failed while the parser was trying to skip over tokens. These are fixed by
moving the recursion limiter from the printer to the parser, and updating the
printer to use the parser's limiter.

The failing cases are added as unit tests.
@pcwalton pcwalton changed the title Apply the recursion limit to the parser instead of the printer. Apply the recursion limit to the parser instead of just to the printer. Jun 18, 2021
@pcwalton pcwalton marked this pull request as ready for review June 18, 2021 18:57
@pcwalton
Copy link
Contributor Author

Didn't find any more fuzz bugs. I think this is good to go.

@alexcrichton alexcrichton merged commit 98bbcff into rust-lang:main Jun 18, 2021
@alexcrichton
Copy link
Member

Thanks!

src/v0.rs Show resolved Hide resolved
src/v0.rs Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants