Use RAII in fuzzer for function context creation (#3296)

Followup to #3276 

1 files changed, 64 insertions, 62 deletions

diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h
index 9b3649d79..f24b50d30 100644
--- a/src/tools/fuzzing.h
+++ b/src/tools/fuzzing.h
@@ -601,56 +601,62 @@ private:
 
   // function generation state
 
-  Function* func = nullptr;
-  std::vector<Expression*> breakableStack; // things we can break to
-  Index labelIndex;
+  struct FunctionCreationContext {
+    TranslateToFuzzReader& parent;
 
-  // a list of things relevant to computing the odds of an infinite loop,
-  // which we try to minimize the risk of
-  std::vector<Expression*> hangStack;
+    Function* func;
 
-  std::map<Type, std::vector<Index>>
-    typeLocals; // type => list of locals with that type
+    std::vector<Expression*> breakableStack; // things we can break to
+    Index labelIndex = 0;
 
-  void prepareToCreateFunctionContents(Function* func_) {
-    func = func_;
-    labelIndex = 0;
-    assert(breakableStack.empty());
-    assert(hangStack.empty());
-  }
+    // a list of things relevant to computing the odds of an infinite loop,
+    // which we try to minimize the risk of
+    std::vector<Expression*> hangStack;
 
-  void finishCreatingFunctionContents() {
-    if (HANG_LIMIT > 0) {
-      addHangLimitChecks(func);
+    // type => list of locals with that type
+    std::map<Type, std::vector<Index>> typeLocals;
+
+    FunctionCreationContext(TranslateToFuzzReader& parent, Function* func)
+      : parent(parent), func(func) {
+      parent.funcContext = this;
     }
-    typeLocals.clear();
-    assert(breakableStack.empty());
-    assert(hangStack.empty());
-  }
+
+    ~FunctionCreationContext() {
+      if (parent.HANG_LIMIT > 0) {
+        parent.addHangLimitChecks(func);
+      }
+      assert(breakableStack.empty());
+      assert(hangStack.empty());
+      parent.funcContext = nullptr;
+    }
+  };
+
+  FunctionCreationContext* funcContext = nullptr;
 
   Index numAddedFunctions = 0;
 
   Function* addFunction() {
     LOGGING_PERCENT = upToSquared(100);
-    func = new Function;
+    auto* func = new Function;
     func->name = Names::getValidFunctionName(wasm, "func");
-    assert(typeLocals.empty());
+    FunctionCreationContext context(*this, func);
+    assert(funcContext->typeLocals.empty());
     Index numParams = upToSquared(MAX_PARAMS);
     std::vector<Type> params;
     params.reserve(numParams);
     for (Index i = 0; i < numParams; i++) {
       auto type = getSingleConcreteType();
-      typeLocals[type].push_back(params.size());
+      funcContext->typeLocals[type].push_back(params.size());
       params.push_back(type);
     }
     func->sig = Signature(Type(params), getControlFlowType());
     Index numVars = upToSquared(MAX_VARS);
     for (Index i = 0; i < numVars; i++) {
       auto type = getConcreteType();
-      typeLocals[type].push_back(params.size() + func->vars.size());
+      funcContext->typeLocals[type].push_back(params.size() +
+                                              func->vars.size());
       func->vars.push_back(type);
     }
-    prepareToCreateFunctionContents(func);
     // with small chance, make the body unreachable
     auto bodyType = func->sig.results;
     if (oneIn(10)) {
@@ -693,8 +699,6 @@ private:
     while (oneIn(3) && !finishedInput) {
       wasm.table.segments[0].data.push_back(func->name);
     }
-    // cleanup
-    finishCreatingFunctionContents();
     numAddedFunctions++;
     return func;
   }
@@ -909,7 +913,7 @@ private:
     auto chance = upTo(RESOLUTION + 1);
     for (auto& ref : wasm.functions) {
       auto* func = ref.get();
-      prepareToCreateFunctionContents(func);
+      FunctionCreationContext context(*this, func);
       if (func->imported()) {
         // We can't allow extra imports, as the fuzzing infrastructure wouldn't
         // know what to provide.
@@ -930,9 +934,6 @@ private:
         mutate(func);
         fixLabels(func);
       }
-      // Note that even if we don't fuzz the contents we still need to call
-      // finish so that we add hang limit protection and other general things.
-      finishCreatingFunctionContents();
     }
     // Remove a start function - the fuzzing harness expects code to run only
     // from exports.
@@ -1013,7 +1014,7 @@ private:
   }
 
   Name makeLabel() {
-    return std::string("label$") + std::to_string(labelIndex++);
+    return std::string("label$") + std::to_string(funcContext->labelIndex++);
   }
 
   // Weighting for the core make* methods. Some nodes are important enough that
@@ -1177,8 +1178,8 @@ private:
     }
     assert(type == Type::unreachable);
     Expression* ret = nullptr;
-    if (func->sig.results.isConcrete()) {
-      ret = makeTrivial(func->sig.results);
+    if (funcContext->func->sig.results.isConcrete()) {
+      ret = makeTrivial(funcContext->func->sig.results);
     }
     return builder.makeReturn(ret);
   }
@@ -1189,7 +1190,7 @@ private:
     auto* ret = builder.makeBlock();
     ret->type = type; // so we have it during child creation
     ret->name = makeLabel();
-    breakableStack.push_back(ret);
+    funcContext->breakableStack.push_back(ret);
     Index num = upToSquared(BLOCK_FACTOR - 1); // we add another later
     if (nesting >= NESTING_LIMIT / 2) {
       // smaller blocks past the limit
@@ -1214,7 +1215,7 @@ private:
     } else {
       ret->list.push_back(make(type));
     }
-    breakableStack.pop_back();
+    funcContext->breakableStack.pop_back();
     if (type.isConcrete()) {
       ret->finalize(type);
     } else {
@@ -1232,8 +1233,8 @@ private:
     auto* ret = wasm.allocator.alloc<Loop>();
     ret->type = type; // so we have it during child creation
     ret->name = makeLabel();
-    breakableStack.push_back(ret);
-    hangStack.push_back(ret);
+    funcContext->breakableStack.push_back(ret);
+    funcContext->hangStack.push_back(ret);
     // either create random content, or do something more targeted
     if (oneIn(2)) {
       ret->body = makeMaybeBlock(type);
@@ -1246,8 +1247,8 @@ private:
       list.push_back(make(type)); // final element, so we have the right type
       ret->body = builder.makeBlock(list, type);
     }
-    breakableStack.pop_back();
-    hangStack.pop_back();
+    funcContext->breakableStack.pop_back();
+    funcContext->hangStack.pop_back();
     ret->finalize(type);
     return ret;
   }
@@ -1279,26 +1280,26 @@ private:
 
   Expression* makeIf(Type type) {
     auto* condition = makeCondition();
-    hangStack.push_back(nullptr);
+    funcContext->hangStack.push_back(nullptr);
     auto* ret =
       buildIf({condition, makeMaybeBlock(type), makeMaybeBlock(type)}, type);
-    hangStack.pop_back();
+    funcContext->hangStack.pop_back();
     return ret;
   }
 
   Expression* makeBreak(Type type) {
-    if (breakableStack.empty()) {
+    if (funcContext->breakableStack.empty()) {
       return makeTrivial(type);
     }
     Expression* condition = nullptr;
     if (type != Type::unreachable) {
-      hangStack.push_back(nullptr);
+      funcContext->hangStack.push_back(nullptr);
       condition = makeCondition();
     }
     // we need to find a proper target to break to; try a few times
     int tries = TRIES;
     while (tries-- > 0) {
-      auto* target = pick(breakableStack);
+      auto* target = pick(funcContext->breakableStack);
       auto name = getTargetName(target);
       auto valueType = getTargetType(target);
       if (type.isConcrete()) {
@@ -1308,7 +1309,7 @@ private:
           continue;
         }
         auto* ret = builder.makeBreak(name, make(type), condition);
-        hangStack.pop_back();
+        funcContext->hangStack.pop_back();
         return ret;
       } else if (type == Type::none) {
         if (valueType != Type::none) {
@@ -1316,7 +1317,7 @@ private:
           continue;
         }
         auto* ret = builder.makeBreak(name, nullptr, condition);
-        hangStack.pop_back();
+        funcContext->hangStack.pop_back();
         return ret;
       } else {
         assert(type == Type::unreachable);
@@ -1328,9 +1329,9 @@ private:
         // to a loop, we prefer there to be a condition along the
         // way, to reduce the chance of infinite looping
         size_t conditions = 0;
-        int i = hangStack.size();
+        int i = funcContext->hangStack.size();
         while (--i >= 0) {
-          auto* item = hangStack[i];
+          auto* item = funcContext->hangStack[i];
           if (item == nullptr) {
             conditions++;
           } else if (auto* loop = item->cast<Loop>()) {
@@ -1364,7 +1365,7 @@ private:
     }
     // we failed to find something
     if (type != Type::unreachable) {
-      hangStack.pop_back();
+      funcContext->hangStack.pop_back();
     }
     return makeTrivial(type);
   }
@@ -1374,12 +1375,12 @@ private:
     int tries = TRIES;
     bool isReturn;
     while (tries-- > 0) {
-      Function* target = func;
+      Function* target = funcContext->func;
       if (!wasm.functions.empty() && !oneIn(wasm.functions.size())) {
         target = pick(wasm.functions).get();
       }
       isReturn = type == Type::unreachable && wasm.features.hasTailCall() &&
-                 func->sig.results == target->sig.results;
+                 funcContext->func->sig.results == target->sig.results;
       if (target->sig.results != type && !isReturn) {
         continue;
       }
@@ -1408,7 +1409,7 @@ private:
       // TODO: handle unreachable
       targetFn = wasm.getFunction(data[i]);
       isReturn = type == Type::unreachable && wasm.features.hasTailCall() &&
-                 func->sig.results == targetFn->sig.results;
+                 funcContext->func->sig.results == targetFn->sig.results;
       if (targetFn->sig.results == type || isReturn) {
         break;
       }
@@ -1436,7 +1437,7 @@ private:
   }
 
   Expression* makeLocalGet(Type type) {
-    auto& locals = typeLocals[type];
+    auto& locals = funcContext->typeLocals[type];
     if (locals.empty()) {
       return makeConst(type);
     }
@@ -1451,7 +1452,7 @@ private:
     } else {
       valueType = getConcreteType();
     }
-    auto& locals = typeLocals[valueType];
+    auto& locals = funcContext->typeLocals[valueType];
     if (locals.empty()) {
       return makeTrivial(type);
     }
@@ -2010,10 +2011,10 @@ private:
       // 'func' is the pointer to the last created function and can be null when
       // we set up globals (before we create any functions), in which case we
       // can't use ref.func.
-      if (type == Type::funcref && func && oneIn(2)) {
+      if (type == Type::funcref && funcContext && oneIn(2)) {
         // First set to target to the last created function, and try to select
         // among other existing function if possible
-        Function* target = func;
+        Function* target = funcContext->func;
         if (!wasm.functions.empty() && !oneIn(wasm.functions.size())) {
           target = pick(wasm.functions).get();
         }
@@ -2517,7 +2518,7 @@ private:
 
   Expression* makeSwitch(Type type) {
     assert(type == Type::unreachable);
-    if (breakableStack.empty()) {
+    if (funcContext->breakableStack.empty()) {
       return make(type);
     }
     // we need to find proper targets to break to; try a bunch
@@ -2525,7 +2526,7 @@ private:
     std::vector<Name> names;
     Type valueType = Type::unreachable;
     while (tries-- > 0) {
-      auto* target = pick(breakableStack);
+      auto* target = pick(funcContext->breakableStack);
       auto name = getTargetName(target);
       auto currValueType = getTargetType(target);
       if (names.empty()) {
@@ -2554,8 +2555,9 @@ private:
   }
 
   Expression* makeReturn(Type type) {
-    return builder.makeReturn(
-      func->sig.results.isConcrete() ? make(func->sig.results) : nullptr);
+    return builder.makeReturn(funcContext->func->sig.results.isConcrete()
+                                ? make(funcContext->func->sig.results)
+                                : nullptr);
   }
 
   Expression* makeNop(Type type) {