diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp index 1c4ee4cc5f4..2c20f23ed19 100644 --- a/src/tools/fuzzing/fuzzing.cpp +++ b/src/tools/fuzzing/fuzzing.cpp @@ -16,6 +16,7 @@ #include "tools/fuzzing.h" #include "ir/gc-type-utils.h" +#include "ir/iteration.h" #include "ir/local-structural-dominance.h" #include "ir/module-utils.h" #include "ir/subtypes.h" @@ -899,6 +900,26 @@ void TranslateToFuzzReader::recombine(Function* func) { }; Modder modder(wasm, scanner, *this); modder.walk(func->body); + // TODO: A specific form of recombination we should perhaps do more often is + // to recombine among an expression's children, and in particular to + // reorder them. +} + +// Given two expressions, try to replace one of the children of the first with +// the second. For example, given i32.add and an i32.const, the i32.const could +// be placed as either of the children of the i32.add. This tramples the +// existing content there. Returns true if we found a place. +static bool replaceChildWith(Expression* expr, Expression* with) { + for (auto*& child : ChildIterator(expr)) { + // To replace, we must have an appropriate type, and we cannot replace a + // Pop under any circumstances. + if (Type::isSubType(with->type, child->type) && + FindAll(child).list.empty()) { + child = with; + return true; + } + } + return false; } void TranslateToFuzzReader::mutate(Function* func) { @@ -929,7 +950,6 @@ void TranslateToFuzzReader::mutate(Function* func) { percentChance = std::max(percentChance, Index(3)); struct Modder : public PostWalker> { - Module& wasm; TranslateToFuzzReader& parent; Index percentChance; @@ -937,8 +957,8 @@ void TranslateToFuzzReader::mutate(Function* func) { // executed, so we don't want to do it all the time even in a big function. bool allowUnreachable; - Modder(Module& wasm, TranslateToFuzzReader& parent, Index percentChance) - : wasm(wasm), parent(parent), percentChance(percentChance) { + Modder(TranslateToFuzzReader& parent, Index percentChance) + : parent(parent), percentChance(percentChance) { // If the parent allows it then sometimes replace with an unreachable, and // sometimes not. Even if we allow it, only do it in certain functions // (half the time) and only do it rarely (see below). @@ -948,29 +968,135 @@ void TranslateToFuzzReader::mutate(Function* func) { void visitExpression(Expression* curr) { if (parent.upTo(100) < percentChance && parent.canBeArbitrarilyReplaced(curr)) { - if (allowUnreachable && parent.oneIn(20)) { + // We can replace in various modes, see below. Generate a random number + // up to 100 to help us there. + int mode = parent.upTo(100); + + if (allowUnreachable && mode < 5) { replaceCurrent(parent.make(Type::unreachable)); return; } + // For constants, perform only a small tweaking in some cases. + // TODO: more minor tweaks to immediates, like making a load atomic or + // not, changing an offset, etc. if (auto* c = curr->dynCast()) { - if (parent.oneIn(2)) { + if (mode < 50) { c->value = parent.tweak(c->value); - return; + } else { + // Just replace the entire thing. + replaceCurrent(parent.make(curr->type)); } + return; } - // TODO: more minor tweaks to immediates, like making a load atomic or - // not, changing an offset, etc. - // Perform a general replacement. (This is not always valid due to - // nesting of labels, but we'll fix that up later.) Note that make() - // picks a subtype, so this has a chance to replace us with anything - // that is valid to put here. - replaceCurrent(parent.make(curr->type)); + + // Generate a replacement for the expression, and by default replace all + // of |curr| (including children) with that replacement, but in some + // cases we can do more subtle things. + // + // Note that such a replacement is not always valid due to nesting of + // labels, but we'll fix that up later. Note also that make() picks a + // subtype, so this has a chance to replace us with anything that is + // valid to put here. + auto* rep = parent.make(curr->type); + if (mode < 33 && rep->type != Type::none) { + // This has a non-none type. Replace the output, keeping the + // expression and its children in a drop. This "interposes" between + // this expression and its parent, something like this: + // + // (D + // (A + // (B) + // (C) + // ) + // ) + //// + // => ;; keep A, replace it in the parent + // + // (D + // (block + // (drop + // (A + // (B) + // (C) + // ) + // ) + // (NEW) + // ) + // ) + // + // We also sometimes try to insert A as a child of NEW, so we actually + // interpose directly: + // + // (D + // (NEW + // (A + // (B) + // (C) + // ) + // ) + // ) + // + // We do not do that all the time, as inserting a drop is actually an + // important situation to test: the drop makes the output of A unused, + // which may let optimizations remove it. + if ((mode & 1) && replaceChildWith(rep, curr)) { + // We managed to replace one of the children with curr, and have + // nothing more to do. + } else { + // Drop curr and append. + rep = + parent.builder.makeSequence(parent.builder.makeDrop(curr), rep); + } + } else if (mode >= 66 && !Properties::isControlFlowStructure(curr)) { + ChildIterator children(curr); + auto numChildren = children.getNumChildren(); + if (numChildren > 0 && numChildren < 5) { + // This is a normal (non-control-flow) expression with at least one + // child (and not an excessive amount of them; see the processing + // below). "Interpose" between the children and this expression by + // keeping them and replacing the parent |curr|. We do this by + // generating drops of the children, like this: + // + // (A + // (B) + // (C) + // ) + // + // => ;; keep children, replace A + // + // (block + // (drop (B)) + // (drop (C)) + // (NEW) + // ) + // + auto* block = parent.builder.makeBlock(); + for (auto* child : children) { + // 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)) { + block->list.push_back(parent.builder.makeDrop(child)); + } + } + + if (!block->list.empty()) { + // We need the block, that is, we did not find a place for all the + // children. + block->list.push_back(rep); + block->finalize(); + rep = block; + } + } + } + replaceCurrent(rep); } } }; - Modder modder(wasm, *this, percentChance); - modder.walk(func->body); + + Modder modder(*this, percentChance); + modder.walkFunctionInModule(func, &wasm); } void TranslateToFuzzReader::fixAfterChanges(Function* func) { @@ -1077,6 +1203,10 @@ void TranslateToFuzzReader::modifyInitialFunctions() { recombine(func); mutate(func); fixAfterChanges(func); + // TODO: This triad of functions appears in another place as well, and + // could be handled by a single function. That function could also + // decide to reorder recombine and mutate or even run more cycles of + // them. } } // Remove a start function - the fuzzing harness expects code to run only diff --git a/test/passes/fuzz_metrics_noprint.bin.txt b/test/passes/fuzz_metrics_noprint.bin.txt index 703c2aeb411..f6af82a5ad0 100644 --- a/test/passes/fuzz_metrics_noprint.bin.txt +++ b/test/passes/fuzz_metrics_noprint.bin.txt @@ -1,34 +1,34 @@ total - [exports] : 29 - [funcs] : 39 + [exports] : 18 + [funcs] : 21 [globals] : 9 [imports] : 4 [memories] : 1 [memory-data] : 2 - [table-data] : 6 + [table-data] : 7 [tables] : 1 [tags] : 0 - [total] : 5494 - [vars] : 119 - Binary : 400 - Block : 892 - Break : 210 - Call : 232 - CallIndirect : 12 - Const : 898 - Drop : 49 - GlobalGet : 421 - GlobalSet : 333 - If : 289 - Load : 113 - LocalGet : 434 - LocalSet : 306 - Loop : 118 - Nop : 85 - RefFunc : 6 - Return : 62 - Select : 52 - Store : 45 - Switch : 1 - Unary : 380 - Unreachable : 156 + [total] : 11685 + [vars] : 64 + Binary : 848 + Block : 1846 + Break : 456 + Call : 275 + CallIndirect : 117 + Const : 1952 + Drop : 86 + GlobalGet : 844 + GlobalSet : 679 + If : 625 + Load : 236 + LocalGet : 1050 + LocalSet : 764 + Loop : 301 + Nop : 143 + RefFunc : 7 + Return : 103 + Select : 77 + Store : 111 + Switch : 3 + Unary : 835 + Unreachable : 327 diff --git a/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt b/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt index 9697da8bd15..241d8471879 100644 --- a/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt +++ b/test/passes/translate-to-fuzz_all-features_metrics_noprint.txt @@ -1,6 +1,6 @@ total [exports] : 3 - [funcs] : 6 + [funcs] : 5 [globals] : 1 [imports] : 5 [memories] : 1 @@ -8,35 +8,49 @@ total [table-data] : 1 [tables] : 1 [tags] : 2 - [total] : 314 - [vars] : 38 - ArrayNew : 2 - ArrayNewFixed : 1 + [total] : 661 + [vars] : 21 + ArrayGet : 1 + ArrayLen : 1 + ArrayNew : 4 + ArrayNewFixed : 6 AtomicFence : 1 - Binary : 58 - Block : 28 - Break : 6 - Call : 10 - Const : 72 - Drop : 3 - GlobalGet : 10 - GlobalSet : 10 + AtomicRMW : 1 + Binary : 87 + Block : 78 + Break : 17 + Call : 11 + Const : 125 + DataDrop : 1 + Drop : 7 + GlobalGet : 26 + GlobalSet : 26 I31Get : 1 - If : 7 - Load : 18 - LocalGet : 36 - LocalSet : 21 - Loop : 1 - Nop : 2 - RefEq : 1 - RefFunc : 2 - RefI31 : 2 - RefNull : 1 - Return : 2 - Select : 1 - Store : 1 - StructGet : 1 + If : 24 + Load : 22 + LocalGet : 65 + LocalSet : 38 + Loop : 9 + MemoryCopy : 1 + Nop : 9 + RefAs : 8 + RefCast : 1 + RefEq : 2 + RefFunc : 1 + RefI31 : 3 + RefIsNull : 2 + RefNull : 13 + RefTest : 1 + Return : 4 + SIMDExtract : 3 + SIMDLoad : 1 + Select : 2 + Store : 4 + StructGet : 2 StructNew : 3 - TupleMake : 2 - Unary : 6 - Unreachable : 5 + Throw : 1 + Try : 1 + TupleExtract : 2 + TupleMake : 5 + Unary : 28 + Unreachable : 13