Fuzzer: Better handling of globals from initial content (#6072)

Previously the fuzzer never added gets or sets of globals from initial content. That was
an oversight, I'm pretty sure - it's just that the code that sets up the lists from which we
pick globals for gets and sets was in another place. That is, any globals in the initial
content file were never used in new random code the fuzzer generates (only new
globals the fuzzer generated were used there).

This PR allows us to use those globals, but also ignores them with some probability,
to avoid breaking patterns like "once" globals (that we want to only be used from
initial content, at least much of the time).

Also simplify the code here: we don't need isInvalidGlobal just to handle the hang
limit global, which is already handled by not being added to the lists we pick names
from anyhow.

2 files changed, 43 insertions, 23 deletions

diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h
index a40a76106..837713dce 100644
--- a/src/tools/fuzzing.h
+++ b/src/tools/fuzzing.h
@@ -289,10 +289,6 @@ private:
   Expression* makeCallRef(Type type);
   Expression* makeLocalGet(Type type);
   Expression* makeLocalSet(Type type);
-  // Some globals are for internal use, and should not be modified by random
-  // fuzz code.
-  bool isValidGlobal(Name name);
-
   Expression* makeGlobalGet(Type type);
   Expression* makeGlobalSet(Type type);
   Expression* makeTupleMake(Type type);
diff --git a/src/tools/fuzzing/fuzzing.cpp b/src/tools/fuzzing/fuzzing.cpp
index ef7b3a5b7..d9e0f6baf 100644
--- a/src/tools/fuzzing/fuzzing.cpp
+++ b/src/tools/fuzzing/fuzzing.cpp
@@ -379,6 +379,24 @@ void TranslateToFuzzReader::setupGlobals() {
       }
     }
   }
+
+  // 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);
+  }
+
+  // Create new random globals.
   for (size_t index = upTo(MAX_GLOBALS); index > 0; --index) {
     auto type = getConcreteType();
     auto* init = makeConst(type);
@@ -394,12 +412,24 @@ void TranslateToFuzzReader::setupGlobals() {
     auto mutability = oneIn(2) ? Builder::Mutable : Builder::Immutable;
     auto global = builder.makeGlobal(
       Names::getValidGlobalName(wasm, "global$"), type, init, mutability);
-    globalsByType[type].push_back(global->name);
-    if (mutability == Builder::Mutable) {
-      mutableGlobalsByType[type].push_back(global->name);
-    }
     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);
+    }
+  }
 }
 
 void TranslateToFuzzReader::setupTags() {
@@ -1696,21 +1726,16 @@ Expression* TranslateToFuzzReader::makeLocalSet(Type type) {
   }
 }
 
-bool TranslateToFuzzReader::isValidGlobal(Name name) {
-  return name != HANG_LIMIT_GLOBAL;
-}
-
 Expression* TranslateToFuzzReader::makeGlobalGet(Type type) {
   auto it = globalsByType.find(type);
   if (it == globalsByType.end() || it->second.empty()) {
-    return makeConst(type);
-  }
-  auto name = pick(it->second);
-  if (isValidGlobal(name)) {
-    return builder.makeGlobalGet(name, type);
-  } else {
     return makeTrivial(type);
   }
+
+  auto name = pick(it->second);
+  // We don't want random fuzz code to use the hang limit global.
+  assert(name != HANG_LIMIT_GLOBAL);
+  return builder.makeGlobalGet(name, type);
 }
 
 Expression* TranslateToFuzzReader::makeGlobalSet(Type type) {
@@ -1720,12 +1745,11 @@ Expression* TranslateToFuzzReader::makeGlobalSet(Type type) {
   if (it == mutableGlobalsByType.end() || it->second.empty()) {
     return makeTrivial(Type::none);
   }
+
   auto name = pick(it->second);
-  if (isValidGlobal(name)) {
-    return builder.makeGlobalSet(name, make(type));
-  } else {
-    return makeTrivial(Type::none);
-  }
+  // We don't want random fuzz code to use the hang limit global.
+  assert(name != HANG_LIMIT_GLOBAL);
+  return builder.makeGlobalSet(name, make(type));
 }
 
 Expression* TranslateToFuzzReader::makeTupleMake(Type type) {