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

Fuzz the multi-value support #918

Merged
merged 2 commits into from
Feb 6, 2020

Conversation

alexcrichton
Copy link
Member

This commit enables multi-value by default for the fuzzers, in theory
allowing us to find panics and such in the multi-value implementation.
Or even runtime errors through the differential fuzzing!

@fitzgen do you think that the implementation is ready for fuzzing? Or should we hold off and fix more bugs before we throw fuzzers at it?

This commit enables multi-value by default for the fuzzers, in theory
allowing us to find panics and such in the multi-value implementation.
Or even runtime errors through the differential fuzzing!
@alexcrichton alexcrichton requested a review from fitzgen February 6, 2020 22:47
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Yep, I think we should fuzz the multi-value stuff, at least to find out if its ready or not ;)

I don't think it makes sense to vary multi-value on/off in differential testing though, as described inline below, however. r=me with that bit removed

@@ -52,6 +52,7 @@ impl Arbitrary for WasmOptTtf {
pub struct DifferentialConfig {
strategy: DifferentialStrategy,
opt_level: DifferentialOptLevel,
multi_value: bool,
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it makes sense to do differential testing between a config with and without multi-value enabled. At best, one side will instantiate and run the wasm OK and the other won't (if the wasm uses multi-value) or they will both instantiate and run the wasm OK (if the wasm doesn't use multi-value). We're not really testing and comparing different compiler pipelines (eg with vs without optimizations) in this scenario, which is what differential testing is supposed to exercise.

@alexcrichton alexcrichton merged commit 344bf2d into bytecodealliance:master Feb 6, 2020
@alexcrichton alexcrichton deleted the fuzz-multi-value branch February 6, 2020 23:36
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.

2 participants