Fix features section handling in the fuzzer #3980
Merged
Conversation
This file contains hidden or 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
tlively
reviewed
Jul 12, 2021
scripts/fuzz_opt.py
Outdated
| def update_feature_opts(wasm): | ||
| global FEATURE_OPTS | ||
| # we will re-compute the features; leave all other things as they are | ||
| EXTRA = [x for x in FEATURE_OPTS if not x.startswith('--enable') and not x.startswith('--disable') and x != '--all-features'] |
Member
There was a problem hiding this comment.
What is EXTRA for? Why is --all-features excluded but not -all or --mvp-features or -mvp?
Member
Author
There was a problem hiding this comment.
Ah, I just handled the actually used ones. But good point, I generalized it now.
scripts/fuzz_opt.py
Outdated
| return False | ||
| # wasm2c doesn't support most features | ||
| return all([x in FEATURE_OPTS for x in ['--disable-exception-handling', '--disable-simd', '--disable-threads', '--disable-bulk-memory', '--disable-nontrapping-float-to-int', '--disable-tail-call', '--disable-sign-ext', '--disable-reference-types', '--disable-multivalue', '--disable-gc']]) | ||
| return not any([x in FEATURE_OPTS for x in ['--enable-exception-handling', '--enable-simd', '--enable-threads', '--enable-bulk-memory', '--enable-nontrapping-float-to-int', '--enable-tail-call', '--enable-sign-ext', '--enable-reference-types', '--enable-multivalue', '--enable-gc']]) |
Member
There was a problem hiding this comment.
It might be good to factor this pattern out into a helper function that takes a list of feature names.
tlively
approved these changes
Jul 13, 2021
Co-authored-by: Thomas Lively <[email protected]>
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.
The features section is additive since #3960. For the fuzzer to know which features
are used, it therefore needs to also scan the features section. To do this,
run
--print-featuresto get the total features used from both flags + thefeatures section.
A result of this is that we now have a list of enabled features instead of
"enable all, then disable". This is actually clearer I think, but it does require
inverting the logic in some places.
May help with #3979 , although I haven't checked yet (cc @MaxGraey ).