-
Notifications
You must be signed in to change notification settings - Fork 745
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
Add "interposition" to the fuzzer's mutate() method #6427
Conversation
@@ -929,16 +930,15 @@ void TranslateToFuzzReader::mutate(Function* func) { | |||
percentChance = std::max(percentChance, Index(3)); | |||
|
|||
struct Modder : public PostWalker<Modder, UnifiedExpressionVisitor<Modder>> { | |||
Module& wasm; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can reuse children without significantly more work by walking the children of the replacement and randomly swapping them with the original children as long as the types are compatible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise, but it would be nice to put the ascii art from the PR description into the code comments.
Good ideas. I added the art and also to replace new children with old content. |
// Only drop the child if we can't replace it as one of NEW's | ||
// children. This does a linear scan of |rep| which is the reason | ||
// for the above limit on the number of children. | ||
if (!replaceChildWith(rep, child)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now this will always replace the first child of rep
with compatible type. For example, if rep
is an i32.add
and the children are i32 expressions, the LHS of rep
will always end up being the last original child and the RHS will never be replaced. Should we have replaceChildWith
collect and randomly pick from all of the possible child locations to increase the number of possible outcomes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, but we do have recombine()
which will mix around things that way. Though we could perhaps add more logic there to reorder the children of an expression, which seems useful. How about leaving that as a TODO?
Before this PR we only mutated by replacing an expression with another, which
replaced all the children. With this PR we also do these two patterns:
Even better would be to reuse the children by the new (
(NEW)
) expression,but that would take significantly more work.