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

Make Yul proto mutator mutations deterministic. #14605

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

bshastry
Copy link
Contributor

The Yul proto mutator is a set of libprotobuf-mutator callbacks that mutates Yul protobuf inputs using custom mutations e.g., add a store here, flip a bool there etc.

The mutator itself would statically initialize a random number generator to eventually produce mutations. However, this would mean that depending on how many times the mutator has been called, the mutation would differ. Meaning that when debugging a crash, it may no longer reproduce because the statically initialized RNG would not cause a mutation or cause a different one.

This PR fixes this non-determinism by making use of (deterministic) seeds provided by the libprotobuf mutator library. Now, crashes should be reproducible outside fuzzing runs.

@cameel cameel force-pushed the make-ypm-deterministic branch from 0fedd07 to eab92f9 Compare October 13, 2023 13:44
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

Does not look like it could break things so approving.

Hopefully it does improve the situation, though it's hard to judge just by looking at the PR itself. Should we see the effect in one of our CI jobs?

@bshastry bshastry merged commit 106ae7e into develop Oct 13, 2023
1 check passed
@bshastry bshastry deleted the make-ypm-deterministic branch October 13, 2023 14:30
@bshastry
Copy link
Contributor Author

Does not look like it could break things so approving.

Hopefully it does improve the situation, though it's hard to judge just by looking at the PR itself. Should we see the effect in one of our CI jobs?

Sadly not, since we don't have tests for the custom mutator. Adding tests has weighed on my mind before but I've not acted on it, may be I should prioritize 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.

2 participants