Add "interposition" to the fuzzer's mutate() method

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<Pop>(child).list.empty()) {
+      child = with;
+      return true;
+    }
+  }
+  return false;
 }
 
 void TranslateToFuzzReader::mutate(Function* func) {
@@ -929,16 +950,15 @@ void TranslateToFuzzReader::mutate(Function* func) {
   percentChance = std::max(percentChance, Index(3));
 
   struct Modder : public PostWalker<Modder, UnifiedExpressionVisitor<Modder>> {
-    Module& wasm;
     TranslateToFuzzReader& parent;
     Index percentChance;
 
     // Whether to replace with unreachable. This can lead to less code getting
     // 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<Const>()) {
-          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

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     

test/passes/translate-to-fuzz_all-features_metrics_noprint.txt

@@ -1,42 +1,56 @@
 total
  [exports]      : 3       
- [funcs]        : 6       
+ [funcs]        : 5       
  [globals]      : 1       
  [imports]      : 5       
  [memories]     : 1       
  [memory-data]  : 20      
  [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