[threads] Update the fuzzer for shared types #6771
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
kripken
reviewed
Jul 18, 2024
scripts/fuzz_opt.py
Outdated
|
|
||
| # The shared-everything feature is new and we want to fuzz it, but it | ||
| # also currently disables fuzzing V8, so disable it half the time. | ||
| if random.random() < 0.5: |
There was a problem hiding this comment.
Skipping V8 is pretty significant, how about 0.9 here?
There was a problem hiding this comment.
Sure, I guess in my local fuzzing I can always comment this out.
Comment on lines
+2636
to
+2642
| static HeapType sharedTrivialStruct = []() { | ||
| TypeBuilder builder(1); | ||
| builder[0] = Struct{}; | ||
| builder[0].setShared(); | ||
| return (*builder.build())[0]; | ||
| }(); | ||
| auto ht = share == Shared ? sharedTrivialStruct : trivialStruct; |
There was a problem hiding this comment.
Suggested change
| static HeapType sharedTrivialStruct = []() { | |
| TypeBuilder builder(1); | |
| builder[0] = Struct{}; | |
| builder[0].setShared(); | |
| return (*builder.build())[0]; | |
| }(); | |
| auto ht = share == Shared ? sharedTrivialStruct : trivialStruct; | |
| static HeapType makeSharedTrivialStruct = []() { | |
| TypeBuilder builder(1); | |
| builder[0] = Struct{}; | |
| builder[0].setShared(); | |
| return (*builder.build())[0]; | |
| }(); | |
| auto ht = share == Shared ? makeSharedTrivialStruct() : trivialStruct; |
There was a problem hiding this comment.
(also below, if this makes sense)
There was a problem hiding this comment.
Note the () after the lambda expression making this an IIFE meant to be executed once when the static local is initialized.
| @@ -759,7 +759,8 @@ void Inhabitator::markExternRefsNullable() { | |||
| auto children = type.getTypeChildren(); | |||
| for (size_t i = 0; i < children.size(); ++i) { | |||
| auto child = children[i]; | |||
| if (child.isRef() && child.getHeapType() == HeapType::ext && | |||
| if (child.isRef() && child.getHeapType().isBasic() && | |||
| child.getHeapType().getBasic(Unshared) == HeapType::ext && | |||
There was a problem hiding this comment.
Perhaps we should have a helper for this common pattern of x.isBasic() && x.getHeapType.getBasic(Unshared) == Y, something like x.isEqualToUnsharedBasic(Y)?
There was a problem hiding this comment.
How about doing this as a follow-up? It could apply more broadly than just in these files.
kripken
approved these changes
Jul 18, 2024
| @@ -759,7 +759,8 @@ void Inhabitator::markExternRefsNullable() { | |||
| auto children = type.getTypeChildren(); | |||
| for (size_t i = 0; i < children.size(); ++i) { | |||
| auto child = children[i]; | |||
| if (child.isRef() && child.getHeapType() == HeapType::ext && | |||
| if (child.isRef() && child.getHeapType().isBasic() && | |||
| child.getHeapType().getBasic(Unshared) == HeapType::ext && | |||
tlively
force-pushed
the
validate-elem-type-feats
branch
from
36a96ef to
79dfd9d
Compare
tlively
force-pushed
the
threads-shared-fuzzing
branch
from
0774d77 to
2d87cd1
Compare
tlively
force-pushed
the
validate-elem-type-feats
branch
from
79dfd9d to
0065859
Compare
tlively
force-pushed
the
threads-shared-fuzzing
branch
from
2d87cd1 to
35c227f
Compare
tlively
added a commit
that referenced
this pull request
Jul 18, 2024
This abbreviates a common pattern where we first had to check whether a heap type was basic, then if it was, get its unshared version and compare it to some expected BasicHeapType. Suggested in #6771 (comment).
tlively
force-pushed
the
validate-elem-type-feats
branch
from
0065859 to
0b0ce0d
Compare
tlively
force-pushed
the
threads-shared-fuzzing
branch
from
35c227f to
a6c1419
Compare
tlively
added a commit
that referenced
this pull request
Jul 18, 2024
This abbreviates a common pattern where we first had to check whether a heap type was basic, then if it was, get its unshared version and compare it to some expected BasicHeapType. Suggested in #6771 (comment).
Update the fuzzer to both handle shared types in initial contents and create and use new shared types without crashing or producing invalid modules. Since V8 does not have a complete implementation of shared-everything-threads yet, disable fuzzing V8 when shared-everything is enabled. To avoid losing too much coverage of V8, disable shared-everything in the fuzzer more frequently than other features.
tlively
force-pushed
the
threads-shared-fuzzing
branch
from
a6c1419 to
ec3bc3e
Compare
tlively
added a commit
that referenced
this pull request
Jul 18, 2024
This abbreviates a common pattern where we first had to check whether a heap type was basic, then if it was, get its unshared version and compare it to some expected BasicHeapType. Suggested in #6771 (comment).
tlively
added a commit
that referenced
this pull request
Jul 18, 2024
This abbreviates a common pattern where we first had to check whether a heap type was basic, then if it was, get its unshared version and compare it to some expected BasicHeapType. Suggested in #6771 (comment).
tlively
added a commit
that referenced
this pull request
Jul 18, 2024
This abbreviates a common pattern where we first had to check whether a heap type was basic, then if it was, get its unshared version and compare it to some expected BasicHeapType. Suggested in #6771 (comment).
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.
Update the fuzzer to both handle shared types in initial contents and
create and use new shared types without crashing or producing invalid
modules. Since V8 does not have a complete implementation of
shared-everything-threads yet, disable fuzzing V8 when shared-everything
is enabled. To avoid losing too much coverage of V8, disable
shared-everything in the fuzzer more frequently than other features.