Better memory fuzzing (#1987)

Hash the contents of all of memory and log that out in random places in the fuzzer, so we are more sensitive there and can catch memory bugs.

Fix UB that was uncovered by this in the binary writing code - if a segment is empty, we should not look at &vector[0], and instead use vector.data().

Add Builder::addExport convenience method.

5 files changed, 77 insertions, 12 deletions

diff --git a/src/tools/fuzzing.h b/src/tools/fuzzing.h
index b8434353a..7b47dea8f 100644
--- a/src/tools/fuzzing.h
+++ b/src/tools/fuzzing.h
@@ -255,6 +255,7 @@ private:
   }
 
   void setupMemory() {
+    // Add memory itself
     MemoryUtils::ensureExists(wasm.memory);
     if (features.hasBulkMemory()) {
       size_t memCovered = 0;
@@ -283,6 +284,41 @@ private:
         wasm.memory.segments[0].data.push_back(value >= 256 ? 0 : (value & 0xff));
       }
     }
+    // Add memory hasher helper (for the hash, see hash.h). The function looks like:
+    // function hashMemory() {
+    //   hash = 5381;
+    //   hash = ((hash << 5) + hash) ^ mem[0];
+    //   hash = ((hash << 5) + hash) ^ mem[1];
+    //   ..
+    //   return hash;
+    // }
+    std::vector<Expression*> contents;
+    contents.push_back(
+      builder.makeSetLocal(0, builder.makeConst(Literal(uint32_t(5381))))
+    );
+    for (Index i = 0; i < USABLE_MEMORY; i++) {
+      contents.push_back(
+        builder.makeSetLocal(0,
+          builder.makeBinary(XorInt32,
+            builder.makeBinary(AddInt32,
+              builder.makeBinary(ShlInt32,
+                builder.makeGetLocal(0, i32),
+                builder.makeConst(Literal(uint32_t(5)))
+              ),
+              builder.makeGetLocal(0, i32)
+            ),
+            builder.makeLoad(1, false, i, 1, builder.makeConst(Literal(uint32_t(0))), i32)
+          )
+        )
+      );
+    }
+    contents.push_back(
+      builder.makeGetLocal(0, i32)
+    );
+    auto* body = builder.makeBlock(contents);
+    auto* hasher = wasm.addFunction(builder.makeFunction("hashMemory", std::vector<Type>{}, i32, { i32 }, body));
+    hasher->type = ensureFunctionType(getSig(hasher), &wasm)->name;
+    wasm.addExport(builder.makeExport(hasher->name, hasher->name, ExternalKind::Function));
   }
 
   void setupTable() {
@@ -675,6 +711,10 @@ private:
         invoke = builder.makeDrop(invoke);
       }
       invocations.push_back(invoke);
+      // log out memory in some cases
+      if (oneIn(2)) {
+        invocations.push_back(makeMemoryHashLogging());
+      }
     }
     if (invocations.empty()) return;
     auto* invoker = new Function;
@@ -770,7 +810,13 @@ private:
 
   Expression* _makenone() {
     auto choice = upTo(100);
-    if (choice < LOGGING_PERCENT) return makeLogging();
+    if (choice < LOGGING_PERCENT) {
+      if (choice < LOGGING_PERCENT / 2) {
+        return makeLogging();
+      } else {
+        return makeMemoryHashLogging();
+      }
+    }
     choice = upTo(100);
     if (choice < 50) return makeSetLocal(none);
     if (choice < 60) return makeBlock(none);
@@ -1814,6 +1860,11 @@ private:
     return builder.makeCall(std::string("log-") + printType(type), { make(type) }, none);
   }
 
+  Expression* makeMemoryHashLogging() {
+    auto* hash = builder.makeCall(std::string("hashMemory"), {}, i32);
+    return builder.makeCall(std::string("log-i32"), { hash }, none);
+  }
+
   // special getters
 
   Type getType() {
diff --git a/src/wasm-builder.h b/src/wasm-builder.h
index 8c50ff2dc..0bbc8eebc 100644
--- a/src/wasm-builder.h
+++ b/src/wasm-builder.h
@@ -40,7 +40,7 @@ public:
   Builder(MixedArena& allocator) : allocator(allocator) {}
   Builder(Module& wasm) : allocator(wasm.allocator) {}
 
-  // make* functions, create nodes
+  // make* functions, other globals
 
   Function* makeFunction(Name name,
                          std::vector<Type>&& params,
@@ -80,6 +80,16 @@ public:
     return func;
   }
 
+  Export* makeExport(Name name, Name value, ExternalKind kind) {
+    auto* export_ = new Export();
+    export_->name = name;
+    export_->value = value;
+    export_->kind = ExternalKind::Function;
+    return export_;
+  }
+
+  // IR nodes
+
   Nop* makeNop() {
     return allocator.alloc<Nop>();
   }
diff --git a/src/wasm.h b/src/wasm.h
index 763a4e764..700938ef9 100644
--- a/src/wasm.h
+++ b/src/wasm.h
@@ -947,10 +947,10 @@ public:
   Global* getGlobalOrNull(Name name);
 
   FunctionType* addFunctionType(std::unique_ptr<FunctionType> curr);
-  void addExport(Export* curr);
-  void addFunction(Function* curr);
-  void addFunction(std::unique_ptr<Function> curr);
-  void addGlobal(Global* curr);
+  Export* addExport(Export* curr);
+  Function* addFunction(Function* curr);
+  Function* addFunction(std::unique_ptr<Function> curr);
+  Global* addGlobal(Global* curr);
 
   void addStart(const Name& s);
 
diff --git a/src/wasm/wasm-binary.cpp b/src/wasm/wasm-binary.cpp
index b1fc03017..2ee528392 100644
--- a/src/wasm/wasm-binary.cpp
+++ b/src/wasm/wasm-binary.cpp
@@ -330,7 +330,7 @@ void WasmBinaryWriter::writeDataSegments() {
       writeExpression(segment.offset);
       o << int8_t(BinaryConsts::End);
     }
-    writeInlineBuffer(&segment.data[0], segment.data.size());
+    writeInlineBuffer(segment.data.data(), segment.data.size());
   }
   finishSection(start);
 }
diff --git a/src/wasm/wasm.cpp b/src/wasm/wasm.cpp
index ea1ff2a7c..8b8285e35 100644
--- a/src/wasm/wasm.cpp
+++ b/src/wasm/wasm.cpp
@@ -863,7 +863,7 @@ FunctionType* Module::addFunctionType(std::unique_ptr<FunctionType> curr) {
   return p;
 }
 
-void Module::addExport(Export* curr) {
+Export* Module::addExport(Export* curr) {
   if (!curr->name.is()) {
     Fatal() << "Module::addExport: empty name";
   }
@@ -872,10 +872,11 @@ void Module::addExport(Export* curr) {
   }
   exports.push_back(std::unique_ptr<Export>(curr));
   exportsMap[curr->name] = curr;
+  return curr;
 }
 
 // TODO(@warchant): refactor all usages to use variant with unique_ptr
-void Module::addFunction(Function* curr) {
+Function* Module::addFunction(Function* curr) {
   if (!curr->name.is()) {
     Fatal() << "Module::addFunction: empty name";
   }
@@ -884,20 +885,22 @@ void Module::addFunction(Function* curr) {
   }
   functions.push_back(std::unique_ptr<Function>(curr));
   functionsMap[curr->name] = curr;
+  return curr;
 }
 
-void Module::addFunction(std::unique_ptr<Function> curr) {
+Function* Module::addFunction(std::unique_ptr<Function> curr) {
   if (!curr->name.is()) {
     Fatal() << "Module::addFunction: empty name";
   }
   if (getFunctionOrNull(curr->name)) {
     Fatal() << "Module::addFunction: " << curr->name << " already exists";
   }
-  functionsMap[curr->name] = curr.get();
+  auto* ret = functionsMap[curr->name] = curr.get();
   functions.push_back(std::move(curr));
+  return ret;
 }
 
-void Module::addGlobal(Global* curr) {
+Global* Module::addGlobal(Global* curr) {
   if (!curr->name.is()) {
     Fatal() << "Module::addGlobal: empty name";
   }
@@ -906,6 +909,7 @@ void Module::addGlobal(Global* curr) {
   }
   globals.push_back(std::unique_ptr<Global>(curr));
   globalsMap[curr->name] = curr;
+  return curr;
 }
 
 void Module::addStart(const Name& s) {