Fuzzer: Better fuzzing of globals (WebAssembly#6611)

With this PR we generate global.gets in globals, which we did not do before.
We do that by replacing makeConst (the only thing we did before, for the
contents of globals) with makeTrivial, and add code to makeTrivial to sometimes
make a global.get. When no suitable global exists, makeGlobalGet will emit a
constant, so there is no danger in trying.

Also raise the number of globals a little.

Also explicitly note the current limitation of requiring all tuple globals to contain
tuple.make and nothing else, including not global.get, and avoid adding such
invalid global.gets in tuple globals in the fuzzer.

src/support/topological_sort.h

@@ -27,7 +27,7 @@ namespace wasm {
 // CRTP utility that provides an iterator through arbitrary directed acyclic
 // graphs of data that will visit the data in a topologically sorted order
 // (https://en.wikipedia.org/wiki/Topological_sorting). In other words, the
-// iterator will produce each item only after all that items predecessors have
+// iterator will produce each item only after all that item's predecessors have
 // been produced.
 //
 // Subclasses should call `push` on all the root items in their constructors and

src/tools/fuzzing.h

@@ -106,6 +106,8 @@ class TranslateToFuzzReader {
 
   std::unordered_map<Type, std::vector<Name>> globalsByType;
   std::unordered_map<Type, std::vector<Name>> mutableGlobalsByType;
+  std::unordered_map<Type, std::vector<Name>> immutableGlobalsByType;
+  std::unordered_map<Type, std::vector<Name>> importedImmutableGlobalsByType;
 
   std::vector<Type> loggableTypes;
 

src/tools/fuzzing/fuzzing.cpp

@@ -379,26 +379,46 @@ void TranslateToFuzzReader::setupGlobals() {
     }
   }
 
+  auto useGlobalLater = [&](Global* global) {
+    auto type = global->type;
+    auto name = global->name;
+    globalsByType[type].push_back(name);
+    if (global->mutable_) {
+      mutableGlobalsByType[type].push_back(name);
+    } else {
+      immutableGlobalsByType[type].push_back(name);
+      if (global->imported()) {
+        importedImmutableGlobalsByType[type].push_back(name);
+      }
+    }
+  };
+
   // Randomly assign some globals from initial content to be ignored for the
   // fuzzer to use. Such globals will only be used from initial content. This is
   // important to preserve some real-world patterns, like the "once" pattern in
   // which a global is used in one function only. (If we randomly emitted gets
   // and sets of such globals, we'd with very high probability end up breaking
   // that pattern, and not fuzzing it at all.)
-  //
-  // Pick a percentage of initial globals to ignore later down when we decide
-  // which to allow uses from.
-  auto numInitialGlobals = wasm.globals.size();
-  unsigned percentIgnoredInitialGlobals = 0;
-  if (numInitialGlobals) {
-    // Only generate this random number if it will be used.
-    percentIgnoredInitialGlobals = upTo(100);
+  if (!wasm.globals.empty()) {
+    unsigned percentUsedInitialGlobals = upTo(100);
+    for (auto& global : wasm.globals) {
+      if (upTo(100) < percentUsedInitialGlobals) {
+        useGlobalLater(global.get());
+      }
+    }
   }
 
   // Create new random globals.
   for (size_t index = upTo(MAX_GLOBALS); index > 0; --index) {
     auto type = getConcreteType();
-    auto* init = makeConst(type);
+    // Prefer immutable ones as they can be used in global.gets in other
+    // globals, for more interesting patterns.
+    auto mutability = oneIn(3) ? Builder::Mutable : Builder::Immutable;
+
+    // We can only make something trivial (like a constant) in a global
+    // initializer.
+    auto* init = makeTrivial(type);
+
     if (!FindAll<RefAs>(init).list.empty()) {
       // When creating this initial value we ended up emitting a RefAs, which
       // means we had to stop in the middle of an overly-nested struct or array,
@@ -407,27 +427,16 @@ void TranslateToFuzzReader::setupGlobals() {
       // validate in a global. Switch to something safe instead.
       type = getMVPType();
       init = makeConst(type);
+    } else if (type.isTuple() && !init->is<TupleMake>()) {
+      // For now we disallow anything but tuple.make at the top level of tuple
+      // globals (see details in wasm-binary.cpp). In the future we may allow
+      // global.get or other things here.
+      init = makeConst(type);
+      assert(init->is<TupleMake>());
     }
-    auto mutability = oneIn(2) ? Builder::Mutable : Builder::Immutable;
     auto global = builder.makeGlobal(
       Names::getValidGlobalName(wasm, "global$"), type, init, mutability);
-    wasm.addGlobal(std::move(global));
-  }
-
-  // Set up data structures for picking globals later for get/set operations.
-  for (Index i = 0; i < wasm.globals.size(); i++) {
-    auto& global = wasm.globals[i];
-
-    // Apply the chance for initial globals to be ignored, see above.
-    if (i < numInitialGlobals && upTo(100) < percentIgnoredInitialGlobals) {
-      continue;
-    }
-
-    // This is a global we can use later, note it.
-    globalsByType[global->type].push_back(global->name);
-    if (global->mutable_) {
-      mutableGlobalsByType[global->type].push_back(global->name);
-    }
+    useGlobalLater(wasm.addGlobal(std::move(global)));
   }
 }
 
@@ -1477,9 +1486,12 @@ Expression* TranslateToFuzzReader::makeTrivial(Type type) {
     // using it before it was set, which would trap.
     if (funcContext && oneIn(type.isNonNullable() ? 10 : 2)) {
       return makeLocalGet(type);
-    } else {
-      return makeConst(type);
     }
+
+    // Either make a const, or a global.get (which may fail to find a suitable
+    // global, especially in a non-function context, and if so it will make a
+    // constant instead).
+    return oneIn(2) ? makeConst(type) : makeGlobalGet(type);
   } else if (type == Type::none) {
     return makeNop(type);
   }
@@ -1900,9 +1912,17 @@ Expression* TranslateToFuzzReader::makeLocalSet(Type type) {
 }
 
 Expression* TranslateToFuzzReader::makeGlobalGet(Type type) {
-  auto it = globalsByType.find(type);
-  if (it == globalsByType.end() || it->second.empty()) {
-    return makeTrivial(type);
+  // In a non-function context, like in another global, we can only get from an
+  // immutable global. Whether GC is enabled also matters, as it allows getting
+  // from a non-import.
+  auto& relevantGlobals =
+    funcContext ? globalsByType
+                : (wasm.features.hasGC() ? immutableGlobalsByType
+                                         : importedImmutableGlobalsByType);
+  auto it = relevantGlobals.find(type);
+  // If we have no such relevant globals give up and emit a constant instead.
+  if (it == relevantGlobals.end() || it->second.empty()) {
+    return makeConst(type);
   }
 
   auto name = pick(it->second);

src/tools/fuzzing/parameters.h

@@ -30,7 +30,7 @@ constexpr int MAX_PARAMS = 10;
 constexpr int MAX_VARS = 20;
 
 // The maximum number of globals in a module.
-constexpr int MAX_GLOBALS = 20;
+constexpr int MAX_GLOBALS = 30;
 
 // The maximum number of tuple elements.
 constexpr int MAX_TUPLE_SIZE = 6;

src/wasm/wasm-binary.cpp

@@ -572,8 +572,23 @@ void WasmBinaryWriter::writeGlobals() {
       o << U32LEB(global->mutable_);
       if (global->type.size() == 1) {
         writeExpression(global->init);
+      } else if (auto* make = global->init->dynCast<TupleMake>()) {
+        // Emit the proper lane for this global.
+        writeExpression(make->operands[i]);
       } else {
-        writeExpression(global->init->cast<TupleMake>()->operands[i]);
+        // For now tuple globals must contain tuple.make. We could perhaps
+        // support more operations, like global.get, but the code would need to
+        // look something like this:
+        //
+        //   auto parentIndex = getGlobalIndex(get->name);
+        //   o << int8_t(BinaryConsts::GlobalGet) << U32LEB(parentIndex + i);
+        //
+        // That is, we must emit the instruction here, and not defer to
+        // writeExpression, as writeExpression writes an entire expression at a
+        // time (and not just one of the lanes). As emitting an instruction here
+        // is less clean, and there is no important use case for global.get of
+        // one tuple global to another, we disallow this.
+        WASM_UNREACHABLE("unsupported tuple global operation");
       }
       o << int8_t(BinaryConsts::End);
       ++i;

test/passes/fuzz_metrics_noprint.bin.txt

@@ -1,34 +1,34 @@
 total
- [exports]      : 18      
- [funcs]        : 21      
+ [exports]      : 23      
+ [funcs]        : 34      
  [globals]      : 9       
  [imports]      : 4       
  [memories]     : 1       
  [memory-data]  : 2       
- [table-data]   : 7       
+ [table-data]   : 6       
  [tables]       : 1       
  [tags]         : 0       
- [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     
+ [total]        : 4303    
+ [vars]         : 100     
+ Binary         : 355     
+ Block          : 684     
+ Break          : 149     
+ Call           : 219     
+ CallIndirect   : 23      
+ Const          : 643     
+ Drop           : 50      
+ GlobalGet      : 367     
+ GlobalSet      : 258     
+ If             : 206     
+ Load           : 78      
+ LocalGet       : 339     
+ LocalSet       : 236     
+ Loop           : 93      
+ Nop            : 41      
+ RefFunc        : 6       
+ Return         : 45      
+ Select         : 41      
+ Store          : 36      
+ Switch         : 1       
+ Unary          : 304     
+ Unreachable    : 129     

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

@@ -1,47 +1,59 @@
 total
- [exports]      : 5       
- [funcs]        : 7       
- [globals]      : 14      
+ [exports]      : 3       
+ [funcs]        : 4       
+ [globals]      : 24      
  [imports]      : 5       
  [memories]     : 1       
  [memory-data]  : 20      
- [table-data]   : 2       
+ [table-data]   : 3       
  [tables]       : 1       
  [tags]         : 1       
- [total]        : 467     
- [vars]         : 40      
- ArrayGet       : 1       
- ArrayLen       : 1       
- ArrayNew       : 6       
+ [total]        : 750     
+ [vars]         : 30      
+ ArrayCopy      : 1       
+ ArrayGet       : 2       
+ ArrayLen       : 5       
+ ArrayNew       : 24      
  ArrayNewFixed  : 1       
- Binary         : 67      
- Block          : 44      
- Break          : 5       
- Call           : 21      
- Const          : 106     
- Drop           : 7       
- GlobalGet      : 20      
- GlobalSet      : 18      
- If             : 14      
- Load           : 17      
- LocalGet       : 45      
- LocalSet       : 28      
- Loop           : 3       
- MemoryFill     : 1       
- Nop            : 5       
- RefAs          : 1       
- RefCast        : 1       
+ ArraySet       : 1       
+ AtomicCmpxchg  : 1       
+ AtomicFence    : 1       
+ AtomicRMW      : 1       
+ Binary         : 84      
+ Block          : 58      
+ Break          : 12      
+ Call           : 13      
+ Const          : 175     
+ Drop           : 2       
+ GlobalGet      : 45      
+ GlobalSet      : 20      
+ I31Get         : 2       
+ If             : 21      
+ Load           : 20      
+ LocalGet       : 70      
+ LocalSet       : 46      
+ Loop           : 7       
+ MemoryCopy     : 1       
+ Nop            : 11      
+ Pop            : 3       
+ RefAs          : 7       
  RefEq          : 1       
- RefFunc        : 3       
- RefI31         : 4       
- RefNull        : 3       
- Return         : 3       
- Select         : 2       
+ RefFunc        : 5       
+ RefI31         : 7       
+ RefIsNull      : 3       
+ RefNull        : 19      
+ RefTest        : 3       
+ Return         : 2       
+ SIMDTernary    : 1       
+ Select         : 3       
  Store          : 1       
- StringConst    : 3       
+ StringConst    : 8       
  StringEncode   : 1       
- StringEq       : 1       
- StructNew      : 5       
- TupleMake      : 5       
- Unary          : 13      
+ StringMeasure  : 1       
+ StringWTF16Get : 1       
+ StructGet      : 1       
+ StructNew      : 21      
+ Try            : 3       
+ TupleMake      : 6       
+ Unary          : 19      
  Unreachable    : 10