Fix roundtrip bug #41
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Discovered by AFL and includes regression tests for the found inputs. Closes image-rs#39.
|
Ah, thanks for the work, I haven't gotten around to investigating this. Will have to check if the write_literal! call below needs to be altered as well. I'll wait with merging until you've tested. |
When running the regression test, print out filename so that regressions are easy to localize to particular cases.
Since these largely branch differently, this increases coverage.
|
This fixed all but three of the crashes fuzzing found so far. Will open the remaining failures as a new issue for tracking. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should be looked at by someone who understands the algorithm. I looked at which paths of
process_chunk_lazythese inputs went through versus which paths they went through with a byte chopped off the end of the buffer (when there was no bug), and noticed that the failing cases exited in this macro call. This led me to suspect it was a wrong return value; removing the+1offset fixed it. I was just a monkey with a hammer here.There are ~50 failing cases now from the AFL fuzz, so tomorrow when I have access to box running the fuzz I'll see if this fixes those.
Closes #39.
After this is merged I'll update #40 so that AFL and the regression tests can share the same seed files, then mark that PR as ready.