Print and compare prototypes in fuzzer interpreter (#8457)

When a struct flows out to JS, if it has a descriptor and that
desriptor's first field is an externref, that field's value becomes the
JS prototype of the struct. There is a class of bugs where we
misoptimize something in this setup so that the JS-observable prototype
changes. To catch those bugs in the fuzzer, update the fuzzer
interpreter and fuzz_shell.js to print the prototypes of objects. This
lets the fuzzer make sure that engines like V8 and interpreter agree on
whether there is a prototype. Also update the fuzzer interpreter to
compare configured prototypes when comparing two execution traces. This
lets --fuzz-exec detect when optimizations have changed a prototype.

Author: tlively

scripts/fuzz_shell.js

@@ -144,16 +144,29 @@ function printed(x, y) {
     // Print bigints in legalized form, which is two 32-bit numbers of the low
     // and high bits.
     return (Number(x & 0xffffffffn) | 0) + ' ' + (Number(x >> 32n) | 0)
-  } else if (typeof x !== 'number') {
-    // Something that is not a number or string, like a reference. We can't
-    // print a reference because it could look different after opts - imagine
-    // that a function gets renamed internally (that is, the problem is that
-    // JS printing will emit some info about the reference and not a stable
-    // external representation of it). In those cases just print the type,
-    // which will be 'object' or 'function'.
-    return typeof x;
+  } else if (typeof x === 'object') {
+    // This may be one of the externref imports, in which case we can print its
+    // payload.
+    if (Object.hasOwn(x, 'payload')) {
+      return 'externref(' + x.payload + ')';
+    }
+    // Or maybe this is a JS error we caught.
+    if (x instanceof Error) {
+      return 'jserror';
+    }
+    // If this is a Wasm object, we can't access its type or any of its
+    // internal structure, which might have been changed by optimizations
+    // anyway. It might have a configured prototype, though, and that
+    // prototype may be an imported externref global we can identify by the
+    // payload we gave it.
+    return 'object(' + printed(Object.getPrototypeOf(x)) + ')';
+  } else if (typeof x === 'function') {
+    // We cannot print function names because they might have been changed by
+    // optimizations.
+    return 'function';
   } else {
     // A number. Print the whole thing.
+    assert(typeof x === 'number');
     return '' + x;
   }
 }
@@ -469,8 +482,15 @@ function makeImports(module) {
         baseImports[module] = {};
       }
       if (!baseImports[module][name]) {
-        // TODO: Use different payloads for different imports.
-        baseImports[module][name] = {};
+        // Compute a payload from the import names. This must be kept in sync
+        // with execution-results.h.
+        var payload = 0;
+        for (var name of [module, name]) {
+          for (var c of name) {
+            payload = (payload + c.charCodeAt(0)) % 251;
+          }
+        }
+        baseImports[module][name] = { payload };
       }
     }
   }

src/literal.h

@@ -737,6 +737,14 @@ class Literal {
   Literal externalize() const;
   Literal internalize() const;
 
+  // Internalize an externalized value or externalize an internalized value,
+  // otherwise return the literal unmodified.
+  Literal unwrap() const;
+
+  // Get the JS prototype configured via this struct's descriptor, if it exists,
+  // or null. Assumes this is a reference value.
+  Literal getJSPrototype() const;
+
 private:
   Literal addSatSI8(const Literal& other) const;
   Literal addSatUI8(const Literal& other) const;

src/tools/execution-results.h

@@ -21,6 +21,7 @@
 #include <deque>
 #include <memory>
 
+#include "ir/import-names.h"
 #include "ir/import-utils.h"
 #include "shell-interface.h"
 #include "support/utilities.h"
@@ -58,20 +59,9 @@ Tag& getJsTag() {
   return tag;
 }
 
-void printValue(Literal value) {
-  // Unwrap an externalized GC value to get the actual value, but not strings,
-  // which are normally a subtype of ext.
-  if (Type::isSubType(value.type, Type(HeapType::ext, Nullable)) &&
-      !value.type.isString()) {
-    value = value.internalize();
-  }
-
-  // An anyref literal is a string.
-  if (value.type.isRef() &&
-      value.type.getHeapType().isMaybeShared(HeapType::any)) {
-    value = value.externalize();
-  }
+constexpr Index jsErrorPayload = 0xbad;
 
+void printValue(Literal value) {
   // Don't print most reference values, as e.g. funcref(N) contains an index,
   // which is not guaranteed to remain identical after optimizations. Do not
   // print the type in detail (as even that may change due to closed-world
@@ -81,22 +71,32 @@ void printValue(Literal value) {
   //
   // The only references we print in full are strings and i31s, which have
   // simple and stable internal structures that optimizations will not alter.
-  auto type = value.type;
-  if (type.isRef()) {
-    if (type.isString() || type.getHeapType().isMaybeShared(HeapType::i31)) {
-      std::cout << value;
-    } else if (value.isNull()) {
-      std::cout << "null";
-    } else if (type.isFunction()) {
-      std::cout << "function";
-    } else {
-      std::cout << "object";
-    }
+  //
+  // Non-references can be printed in full.
+  if (!value.type.isRef()) {
+    std::cout << value;
     return;
   }
-
-  // Non-references can be printed in full.
-  std::cout << value;
+  value = value.unwrap();
+  auto heapType = value.type.getHeapType();
+  if (heapType.isMaybeShared(HeapType::ext) &&
+      value.getExternPayload() == jsErrorPayload) {
+    std::cout << "jserror";
+    return;
+  }
+  if (heapType.isString() || heapType.isMaybeShared(HeapType::ext) ||
+      heapType.isMaybeShared(HeapType::i31)) {
+    std::cout << value;
+  } else if (value.isNull()) {
+    std::cout << "null";
+  } else if (heapType.isFunction()) {
+    std::cout << "function";
+  } else {
+    // Print 'object' and its JS-visible prototype, which may be null.
+    std::cout << "object(";
+    printValue(value.getJSPrototype());
+    std::cout << ')';
+  }
 }
 
 } // namespace
@@ -275,7 +275,7 @@ struct LoggingExternalInterface : public ShellExternalInterface {
 
   void throwJSException() {
     // JS exceptions contain an externref.
-    Literals arguments = {Literal::makeExtern(0, Unshared)};
+    Literals arguments = {Literal::makeExtern(jsErrorPayload, Unshared)};
     auto payload = std::make_shared<ExnData>(&jsTag, arguments);
     throwException(WasmException{Literal(payload)});
   }
@@ -403,8 +403,18 @@ class FuzzerImportResolver
     if (mut || !type.isRef() || type.getHeapType() != HeapType::ext) {
       return nullptr;
     }
-    // TODO: Generate a distinct payload for each global.
-    synthesizedGlobals.emplace_back(Literals{Literal::makeExtern(0, Unshared)});
+    // Optimizations may reorder or remove imports, so we need a distinct
+    // payload that is independent of the import order. Just compute a simple
+    // payload integer from the import names. This must be kept in sync with
+    // fuzz_shell.js.
+    Index payload = 0;
+    for (auto name : {name.module, name.name}) {
+      for (auto c : name.str) {
+        payload = (payload + static_cast<Index>(c)) % 251;
+      }
+    }
+    synthesizedGlobals.emplace_back(
+      Literals{Literal::makeExtern(payload, Unshared)});
     return &synthesizedGlobals.back();
   }
 
@@ -527,28 +537,56 @@ struct ExecutionResults {
   }
 
   bool areEqual(Literal a, Literal b) {
-    // Don't compare references. There are several issues here that we can't
-    // fully handle, see https://github.com/WebAssembly/binaryen/issues/3378,
-    // but the core issue is that since we optimize assuming a closed world, the
-    // types and structure of GC data can arbitrarily change after
-    // optimizations, even in ways that are externally visible from outside
-    // the module.
-    //
-    // We can, however, compare strings as they refer to simple data that has a
-    // consistent representation (the same reasons as why we can print them in
-    // printValue(), above).
+    // Only compare some references. In general the optimizer may change
+    // identities and structures of functions, types, and GC values in ways that
+    // are not externally observable. We must therefore limit ourselves to
+    // comparing information that _is_ externally observable.
     //
-    // TODO: Once we support optimizing under some form of open-world
-    // assumption, we should be able to check that the types and/or structure of
-    // GC data passed out of the module does not change.
-    if (a.type.isRef() && !a.type.isString() &&
-        !a.type.getHeapType().isMaybeShared(HeapType::i31)) {
-      return true;
+    // TODO: We could compare more information when we know it will be
+    // externally visible, for example when the type of the value is public.
+    if (!a.type.isRef() || !b.type.isRef()) {
+      return a == b;
+    }
+    // The environment always sees externalized references and is able to
+    // observe the difference between external references and externalized
+    // internal references. Make sure this is accounted for below by unwrapping
+    // the references.
+    a = a.unwrap();
+    b = b.unwrap();
+    auto htA = a.type.getHeapType();
+    auto htB = b.type.getHeapType();
+    // What type hierarchy a heap type is in is generally observable.
+    if (htA.getTop() != htB.getTop()) {
+      return false;
     }
-    if (a != b) {
-      std::cout << "values not identical! " << a << " != " << b << '\n';
+    // Null values are observable.
+    if (htA.isBottom() || htB.isBottom()) {
+      return a == b;
+    }
+    // String values are observable.
+    if (htA.isString() || htB.isString()) {
+      return a == b;
+    }
+    // i31 values are observable.
+    if (htA.isMaybeShared(HeapType::i31) || htB.isMaybeShared(HeapType::i31)) {
+      return a == b;
+    }
+    // External references are observable. (These cannot be externalized
+    // internal references because they've already been unwrapped.)
+    if (htA.isMaybeShared(HeapType::ext) || htB.isMaybeShared(HeapType::ext)) {
+      return a == b;
+    }
+    // Configured prototypes are observable. Even if they are also opaque Wasm
+    // references, their having different pointer identities is observable.
+    // However, we have no way of comparing pointer identities across
+    // executions, so just recursively look for externally observable
+    // differences in the prototypes.
+    if (!areEqual(a.getJSPrototype(), b.getJSPrototype())) {
       return false;
     }
+
+    // Other differences are not observable, so conservatively consider the
+    // values equal.
     return true;
   }
 
@@ -559,6 +597,7 @@ struct ExecutionResults {
     }
     for (Index i = 0; i < a.size(); i++) {
       if (!areEqual(a[i], b[i])) {
+        std::cout << "values not identical! " << a[i] << " != " << b[i] << '\n';
         return false;
       }
     }
@@ -628,7 +667,13 @@ struct ExecutionResults {
     } catch (const TrapException&) {
       return Trap{};
     } catch (const WasmException& e) {
-      std::cout << "[exception thrown: " << e << "]" << std::endl;
+      auto& exn = *e.exn.getExnData();
+      std::cout << "[exception thrown: " << exn.tag->name;
+      for (auto val : exn.payload) {
+        std::cout << ' ';
+        printValue(val);
+      }
+      std::cout << "]" << std::endl;
       return Exception{};
     } catch (const HostLimitException&) {
       // This should be ignored and not compared with, as optimizations can

src/tools/wasm-ctor-eval.cpp

@@ -467,7 +467,11 @@ struct CtorEvalExternalInterface : EvallingModuleRunner::ExternalInterface {
 
   void throwException(const WasmException& exn) override {
     std::stringstream ss;
-    ss << "exception thrown: " << exn;
+    auto& data = *exn.exn.getExnData();
+    ss << "exception thrown: " << data.tag->name;
+    if (!data.payload.empty()) {
+      ss << ' ' << data.payload;
+    }
     throw FailToEvalException(ss.str());
   }
 

src/wasm-interpreter.h

@@ -61,7 +61,6 @@ namespace wasm {
 struct WasmException {
   Literal exn;
 };
-std::ostream& operator<<(std::ostream& o, const WasmException& exn);
 
 // An exception thrown when we try to execute non-constant code, that is, code
 // that we cannot properly evaluate at compile time (e.g. if it refers to an

src/wasm/CMakeLists.txt

@@ -7,7 +7,6 @@ set(wasm_SOURCES
   wasm-binary.cpp
   wasm-debug.cpp
   wasm-emscripten.cpp
-  wasm-interpreter.cpp
   wasm-io.cpp
   wasm-ir-builder.cpp
   wasm-stack.cpp

src/wasm/literal.cpp

@@ -19,6 +19,7 @@
 
 #include "emscripten-optimizer/simple_ast.h"
 #include "ir/bits.h"
+#include "ir/js-utils.h"
 #include "literal.h"
 #include "pretty_printing.h"
 #include "support/bits.h"
@@ -514,6 +515,12 @@ bool Literal::operator==(const Literal& other) const {
       return i32 == other.i32;
     }
     if (heapType.isMaybeShared(HeapType::ext)) {
+      if (hasExternPayload()) {
+        if (!other.hasExternPayload()) {
+          return false;
+        }
+        return getExternPayload() == other.getExternPayload();
+      }
       return internalize() == other.internalize();
     }
     if (heapType.isMaybeShared(HeapType::any)) {
@@ -782,7 +789,14 @@ std::ostream& operator<<(std::ostream& o, Literal literal) {
       assert(literal.isData());
       auto data = literal.getGCData();
       assert(data);
-      o << "[ref " << literal.type.getHeapType() << ' ' << data->values << ']';
+      o << "[ref " << literal.type.getHeapType() << ' ' << data->values;
+      if (!data->desc.isNull()) {
+        if (!data->values.empty()) {
+          o << ", ";
+        }
+        o << "desc=" << data->desc;
+      }
+      o << ']';
     }
   }
   restoreNormalColor(o);
@@ -3009,4 +3023,37 @@ Literal Literal::internalize() const {
   return gcData->values[0];
 }
 
+Literal Literal::unwrap() const {
+  if (!type.isRef()) {
+    return *this;
+  }
+  if (type.getHeapType().isMaybeShared(HeapType::any)) {
+    // An internalized external reference (possibly a string).
+    return externalize();
+  }
+  if (type.getHeapType().isMaybeShared(HeapType::ext) && !hasExternPayload()) {
+    // An externalized internal reference.
+    return internalize();
+  }
+  // Something other reference that is not wrapped.
+  return *this;
+}
+
+Literal Literal::getJSPrototype() const {
+  assert(type.isRef());
+  if (auto desc = type.getHeapType().getDescriptorType();
+      desc && JSUtils::hasPossibleJSPrototypeField(*desc)) {
+    auto proto = gcData->desc.getGCData()->values[0].unwrap();
+    // Strings and numbers are not valid prototypes, so they appear as null.
+    // Externref nulls are also converted to nullref.
+    auto protoType = proto.type.getHeapType();
+    if (protoType.isMaybeShared(HeapType::i31) ||
+        protoType.isMaybeShared(HeapType::string) || protoType.isBottom()) {
+      return Literal::makeNull(HeapType::none);
+    }
+    return proto;
+  }
+  return Literal::makeNull(HeapType::none);
+}
+
 } // namespace wasm