Bug 1784268: Fix small issues blocking GC spec tests. r=rhunt

- Fix wasm module serialization order. Element segments sometimes need to refer to types during deserialization, but element segments were being deserialized before code objects (which contain types).
- Do function subtype checks on import, rather than requiring strict equality.
- Allow explicit null function references in table initializers.
- Initialize type defs and globals before tables when instantiating modules.

Differential Revision: https://phabricator.services.mozilla.com/D188737

js/src/jit-test/tests/wasm/gc/tables.js

@@ -0,0 +1,167 @@
+// |jit-test| skip-if: !wasmGcEnabled()
+
+// implicit null funcref value
+{
+  const { t, get } = wasmEvalText(`(module
+    (table (export "t") 3 funcref)
+    (func (export "get") (param i32) (result funcref)
+      (table.get (local.get 0))
+    )
+  )`).exports;
+
+  assertEq(t.get(0), null);
+  assertEq(t.get(1), null);
+  assertEq(t.get(2), null);
+  assertEq(get(0), null);
+  assertEq(get(1), null);
+  assertEq(get(2), null);
+}
+
+// explicit null funcref value
+{
+  const { t, get } = wasmEvalText(`(module
+    (table (export "t") 3 funcref (ref.null func))
+    (func (export "get") (param i32) (result funcref)
+      (table.get (local.get 0))
+    )
+  )`).exports;
+
+  assertEq(t.get(0), null);
+  assertEq(t.get(1), null);
+  assertEq(t.get(2), null);
+  assertEq(get(0), null);
+  assertEq(get(1), null);
+  assertEq(get(2), null);
+}
+
+// actual funcref value
+{
+  const { t, get, foo } = wasmEvalText(`(module
+    (func $foo (export "foo"))
+
+    (table (export "t") 3 funcref (ref.func $foo))
+    (func (export "get") (param i32) (result funcref)
+      (table.get (local.get 0))
+    )
+  )`).exports;
+
+  assertEq(t.get(0), foo);
+  assertEq(t.get(1), foo);
+  assertEq(t.get(2), foo);
+  assertEq(get(0), foo);
+  assertEq(get(1), foo);
+  assertEq(get(2), foo);
+}
+
+// implicit null anyref value
+{
+  const { t, get } = wasmEvalText(`(module
+    (table (export "t") 3 anyref)
+    (func (export "get") (param i32) (result anyref)
+      (table.get (local.get 0))
+    )
+  )`).exports;
+
+  assertEq(t.get(0), null);
+  assertEq(t.get(1), null);
+  assertEq(t.get(2), null);
+  assertEq(get(0), null);
+  assertEq(get(1), null);
+  assertEq(get(2), null);
+}
+
+// explicit null anyref value
+{
+  const { t, get } = wasmEvalText(`(module
+    (table (export "t") 3 anyref (ref.null any))
+    (func (export "get") (param i32) (result anyref)
+      (table.get (local.get 0))
+    )
+  )`).exports;
+
+  assertEq(t.get(0), null);
+  assertEq(t.get(1), null);
+  assertEq(t.get(2), null);
+  assertEq(get(0), null);
+  assertEq(get(1), null);
+  assertEq(get(2), null);
+}
+
+// actual anyref value
+{
+  const { t, get } = wasmEvalText(`(module
+    (type $s (struct))
+
+    (table (export "t") 3 anyref (struct.new $s))
+    (func (export "get") (param i32) (result anyref)
+      (table.get (local.get 0))
+    )
+  )`).exports;
+
+  assertEq(!!t.get(0), true);
+  assertEq(!!t.get(1), true);
+  assertEq(!!t.get(2), true);
+  assertEq(!!get(0), true);
+  assertEq(!!get(1), true);
+  assertEq(!!get(2), true);
+}
+
+// implicit null externref value
+{
+  const { t, get } = wasmEvalText(`(module
+    (table (export "t") 3 externref)
+    (func (export "get") (param i32) (result externref)
+      (table.get (local.get 0))
+    )
+  )`).exports;
+
+  assertEq(t.get(0), null);
+  assertEq(t.get(1), null);
+  assertEq(t.get(2), null);
+  assertEq(get(0), null);
+  assertEq(get(1), null);
+  assertEq(get(2), null);
+}
+
+// explicit null externref value
+{
+  const { t, get } = wasmEvalText(`(module
+    (table (export "t") 3 externref (ref.null extern))
+    (func (export "get") (param i32) (result externref)
+      (table.get (local.get 0))
+    )
+  )`).exports;
+
+  assertEq(t.get(0), null);
+  assertEq(t.get(1), null);
+  assertEq(t.get(2), null);
+  assertEq(get(0), null);
+  assertEq(get(1), null);
+  assertEq(get(2), null);
+}
+
+// actual externref value (from an imported global, which is visible to tables)
+{
+  const foo = "wowzers";
+  const { t, get } = wasmEvalText(`(module
+    (global (import "" "foo") externref)
+
+    (table (export "t") 3 externref (global.get 0))
+    (func (export "get") (param i32) (result externref)
+      (table.get (local.get 0))
+    )
+  )`, { "": { "foo": foo } }).exports;
+
+  assertEq(t.get(0), foo);
+  assertEq(t.get(1), foo);
+  assertEq(t.get(2), foo);
+  assertEq(get(0), foo);
+  assertEq(get(1), foo);
+  assertEq(get(2), foo);
+}
+
+// non-imported globals come after tables and are therefore not visible
+assertErrorMessage(() => wasmEvalText(`(module
+  (global anyref (ref.null any))
+  (table 3 anyref (global.get 0))
+)`), WebAssembly.CompileError, /global.get index out of range/);

js/src/wasm/WasmCode.h

@@ -417,6 +417,9 @@ struct Metadata : public ShareableBase<Metadata>, public MetadataCacheablePod {
   MetadataCacheablePod& pod() { return *this; }
   const MetadataCacheablePod& pod() const { return *this; }
 
+  const TypeDef& getFuncImportTypeDef(const FuncImport& funcImport) const {
+    return types->type(funcImport.typeIndex());
+  }
   const FuncType& getFuncImportType(const FuncImport& funcImport) const {
     return types->type(funcImport.typeIndex()).funcType();
   }

js/src/wasm/WasmInstance.cpp

@@ -1984,6 +1984,79 @@ bool Instance::init(JSContext* cx, const JSObjectVector& funcImports,
   addressOfNeedsIncrementalBarrier_ =
       cx->compartment()->zone()->addressOfNeedsIncrementalBarrier();
 
+  // Initialize type definitions in the instance data.
+  const SharedTypeContext& types = metadata().types;
+  Zone* zone = realm()->zone();
+  for (uint32_t typeIndex = 0; typeIndex < types->length(); typeIndex++) {
+    const TypeDef& typeDef = types->type(typeIndex);
+    TypeDefInstanceData* typeDefData = typeDefInstanceData(typeIndex);
+
+    // Set default field values.
+    new (typeDefData) TypeDefInstanceData();
+
+    // Store the runtime type for this type index
+    typeDefData->typeDef = &typeDef;
+    typeDefData->superTypeVector = typeDef.superTypeVector();
+
+    if (typeDef.kind() == TypeDefKind::Struct ||
+        typeDef.kind() == TypeDefKind::Array) {
+      // Compute the parameters that allocation will use.  First, the class
+      // and alloc kind for the type definition.
+      const JSClass* clasp;
+      gc::AllocKind allocKind;
+
+      if (typeDef.kind() == TypeDefKind::Struct) {
+        clasp = WasmStructObject::classForTypeDef(&typeDef);
+        allocKind = WasmStructObject::allocKindForTypeDef(&typeDef);
+      } else {
+        clasp = &WasmArrayObject::class_;
+        allocKind = WasmArrayObject::allocKind();
+      }
+
+      // Move the alloc kind to background if possible
+      if (CanChangeToBackgroundAllocKind(allocKind, clasp)) {
+        allocKind = ForegroundToBackgroundAllocKind(allocKind);
+      }
+
+      // Find the shape using the class and recursion group
+      const ObjectFlags objectFlags = {ObjectFlag::NotExtensible};
+      typeDefData->shape =
+          WasmGCShape::getShape(cx, clasp, cx->realm(), TaggedProto(),
+                                &typeDef.recGroup(), objectFlags);
+      if (!typeDefData->shape) {
+        return false;
+      }
+
+      typeDefData->clasp = clasp;
+      typeDefData->allocKind = allocKind;
+
+      // Initialize the allocation site for pre-tenuring.
+      typeDefData->allocSite.initWasm(zone);
+
+      // If `typeDef` is a struct, cache its size here, so that allocators
+      // don't have to chase back through `typeDef` to determine that.
+      // Similarly, if `typeDef` is an array, cache its array element size
+      // here.
+      MOZ_ASSERT(typeDefData->unused == 0);
+      if (typeDef.kind() == TypeDefKind::Struct) {
+        typeDefData->structTypeSize = typeDef.structType().size_;
+        // StructLayout::close ensures this is an integral number of words.
+        MOZ_ASSERT((typeDefData->structTypeSize % sizeof(uintptr_t)) == 0);
+      } else {
+        uint32_t arrayElemSize = typeDef.arrayType().elementType_.size();
+        typeDefData->arrayElemSize = arrayElemSize;
+        MOZ_ASSERT(arrayElemSize == 16 || arrayElemSize == 8 ||
+                   arrayElemSize == 4 || arrayElemSize == 2 ||
+                   arrayElemSize == 1);
+      }
+    } else if (typeDef.kind() == TypeDefKind::Func) {
+      // Nothing to do; the default values are OK.
+    } else {
+      MOZ_ASSERT(typeDef.kind() == TypeDefKind::None);
+      MOZ_CRASH();
+    }
+  }
+
   // Initialize function imports in the instance data
   Tier callerTier = code_->bestTier();
   for (size_t i = 0; i < metadata(callerTier).funcImports.length(); i++) {
@@ -2023,6 +2096,66 @@ bool Instance::init(JSContext* cx, const JSObjectVector& funcImports,
     }
   }
 
+  // Initialize globals in the instance data.
+  //
+  // This must be performed after we have initialized runtime types as a global
+  // initializer may reference them.
+  //
+  // We increment `maxInitializedGlobalsIndexPlus1_` every iteration of the
+  // loop, as we call out to `InitExpr::evaluate` which may call
+  // `constantGlobalGet` which uses this value to assert we're never accessing
+  // uninitialized globals.
+  maxInitializedGlobalsIndexPlus1_ = 0;
+  for (size_t i = 0; i < metadata().globals.length();
+       i++, maxInitializedGlobalsIndexPlus1_ = i) {
+    const GlobalDesc& global = metadata().globals[i];
+
+    // Constants are baked into the code, never stored in the global area.
+    if (global.isConstant()) {
+      continue;
+    }
+
+    uint8_t* globalAddr = data() + global.offset();
+    switch (global.kind()) {
+      case GlobalKind::Import: {
+        size_t imported = global.importIndex();
+        if (global.isIndirect()) {
+          *(void**)globalAddr =
+              (void*)&globalObjs[imported]->val().get().cell();
+        } else {
+          globalImportValues[imported].writeToHeapLocation(globalAddr);
+        }
+        break;
+      }
+      case GlobalKind::Variable: {
+        RootedVal val(cx);
+        const InitExpr& init = global.initExpr();
+        Rooted<WasmInstanceObject*> instanceObj(cx, object());
+        if (!init.evaluate(cx, instanceObj, &val)) {
+          return false;
+        }
+
+        if (global.isIndirect()) {
+          // Initialize the cell
+          wasm::GCPtrVal& cell = globalObjs[i]->val();
+          cell = val.get();
+          // Link to the cell
+          void* address = (void*)&cell.get().cell();
+          *(void**)globalAddr = address;
+        } else {
+          val.get().writeToHeapLocation(globalAddr);
+        }
+        break;
+      }
+      case GlobalKind::Constant: {
+        MOZ_CRASH("skipped at the top");
+      }
+    }
+  }
+
+  // All globals were initialized
+  MOZ_ASSERT(maxInitializedGlobalsIndexPlus1_ == metadata().globals.length());
+
   // Initialize memories in the instance data
   for (size_t i = 0; i < memories.length(); i++) {
     const MemoryDesc& md = metadata().memories[i];
@@ -2110,139 +2243,6 @@ bool Instance::init(JSContext* cx, const JSObjectVector& funcImports,
     }
   }
 
-  // Initialize type definitions in the instance data.
-  const SharedTypeContext& types = metadata().types;
-  Zone* zone = realm()->zone();
-  for (uint32_t typeIndex = 0; typeIndex < types->length(); typeIndex++) {
-    const TypeDef& typeDef = types->type(typeIndex);
-    TypeDefInstanceData* typeDefData = typeDefInstanceData(typeIndex);
-
-    // Set default field values.
-    new (typeDefData) TypeDefInstanceData();
-
-    // Store the runtime type for this type index
-    typeDefData->typeDef = &typeDef;
-    typeDefData->superTypeVector = typeDef.superTypeVector();
-
-    if (typeDef.kind() == TypeDefKind::Struct ||
-        typeDef.kind() == TypeDefKind::Array) {
-      // Compute the parameters that allocation will use.  First, the class
-      // and alloc kind for the type definition.
-      const JSClass* clasp;
-      gc::AllocKind allocKind;
-
-      if (typeDef.kind() == TypeDefKind::Struct) {
-        clasp = WasmStructObject::classForTypeDef(&typeDef);
-        allocKind = WasmStructObject::allocKindForTypeDef(&typeDef);
-      } else {
-        clasp = &WasmArrayObject::class_;
-        allocKind = WasmArrayObject::allocKind();
-      }
-
-      // Move the alloc kind to background if possible
-      if (CanChangeToBackgroundAllocKind(allocKind, clasp)) {
-        allocKind = ForegroundToBackgroundAllocKind(allocKind);
-      }
-
-      // Find the shape using the class and recursion group
-      const ObjectFlags objectFlags = {ObjectFlag::NotExtensible};
-      typeDefData->shape =
-          WasmGCShape::getShape(cx, clasp, cx->realm(), TaggedProto(),
-                                &typeDef.recGroup(), objectFlags);
-      if (!typeDefData->shape) {
-        return false;
-      }
-
-      typeDefData->clasp = clasp;
-      typeDefData->allocKind = allocKind;
-
-      // Initialize the allocation site for pre-tenuring.
-      typeDefData->allocSite.initWasm(zone);
-
-      // If `typeDef` is a struct, cache its size here, so that allocators
-      // don't have to chase back through `typeDef` to determine that.
-      // Similarly, if `typeDef` is an array, cache its array element size
-      // here.
-      MOZ_ASSERT(typeDefData->unused == 0);
-      if (typeDef.kind() == TypeDefKind::Struct) {
-        typeDefData->structTypeSize = typeDef.structType().size_;
-        // StructLayout::close ensures this is an integral number of words.
-        MOZ_ASSERT((typeDefData->structTypeSize % sizeof(uintptr_t)) == 0);
-      } else {
-        uint32_t arrayElemSize = typeDef.arrayType().elementType_.size();
-        typeDefData->arrayElemSize = arrayElemSize;
-        MOZ_ASSERT(arrayElemSize == 16 || arrayElemSize == 8 ||
-                   arrayElemSize == 4 || arrayElemSize == 2 ||
-                   arrayElemSize == 1);
-      }
-    } else if (typeDef.kind() == TypeDefKind::Func) {
-      // Nothing to do; the default values are OK.
-    } else {
-      MOZ_ASSERT(typeDef.kind() == TypeDefKind::None);
-      MOZ_CRASH();
-    }
-  }
-
-  // Initialize globals in the instance data.
-  //
-  // This must be performed after we have initialized runtime types as a global
-  // initializer may reference them.
-  //
-  // We increment `maxInitializedGlobalsIndexPlus1_` every iteration of the
-  // loop, as we call out to `InitExpr::evaluate` which may call
-  // `constantGlobalGet` which uses this value to assert we're never accessing
-  // uninitialized globals.
-  maxInitializedGlobalsIndexPlus1_ = 0;
-  for (size_t i = 0; i < metadata().globals.length();
-       i++, maxInitializedGlobalsIndexPlus1_ = i) {
-    const GlobalDesc& global = metadata().globals[i];
-
-    // Constants are baked into the code, never stored in the global area.
-    if (global.isConstant()) {
-      continue;
-    }
-
-    uint8_t* globalAddr = data() + global.offset();
-    switch (global.kind()) {
-      case GlobalKind::Import: {
-        size_t imported = global.importIndex();
-        if (global.isIndirect()) {
-          *(void**)globalAddr =
-              (void*)&globalObjs[imported]->val().get().cell();
-        } else {
-          globalImportValues[imported].writeToHeapLocation(globalAddr);
-        }
-        break;
-      }
-      case GlobalKind::Variable: {
-        RootedVal val(cx);
-        const InitExpr& init = global.initExpr();
-        Rooted<WasmInstanceObject*> instanceObj(cx, object());
-        if (!init.evaluate(cx, instanceObj, &val)) {
-          return false;
-        }
-
-        if (global.isIndirect()) {
-          // Initialize the cell
-          wasm::GCPtrVal& cell = globalObjs[i]->val();
-          cell = val.get();
-          // Link to the cell
-          void* address = (void*)&cell.get().cell();
-          *(void**)globalAddr = address;
-        } else {
-          val.get().writeToHeapLocation(globalAddr);
-        }
-        break;
-      }
-      case GlobalKind::Constant: {
-        MOZ_CRASH("skipped at the top");
-      }
-    }
-  }
-
-  // All globals were initialized
-  MOZ_ASSERT(maxInitializedGlobalsIndexPlus1_ == metadata().globals.length());
-
   // Take references to the passive data segments
   if (!passiveDataSegments_.resize(dataSegments.length())) {
     return false;

js/src/wasm/WasmModule.cpp

@@ -37,6 +37,7 @@
 #include "wasm/WasmInstance.h"
 #include "wasm/WasmIonCompile.h"
 #include "wasm/WasmJS.h"
+#include "wasm/WasmModuleTypes.h"
 #include "wasm/WasmSerialize.h"
 #include "wasm/WasmUtility.h"
 
@@ -476,12 +477,12 @@ bool Module::instantiateFunctions(JSContext* cx,
     Instance& instance = ExportedFunctionToInstance(f);
     Tier otherTier = instance.code().stableTier();
 
-    const FuncType& exportFuncType = instance.metadata().getFuncExportType(
+    const TypeDef& exportFuncType = instance.metadata().getFuncExportTypeDef(
         instance.metadata(otherTier).lookupFuncExport(funcIndex));
-    const FuncType& importFuncType =
-        metadata().getFuncImportType(metadata(tier).funcImports[i]);
+    const TypeDef& importFuncType =
+        metadata().getFuncImportTypeDef(metadata(tier).funcImports[i]);
 
-    if (!FuncType::strictlyEquals(exportFuncType, importFuncType)) {
+    if (!TypeDef::isSubTypeOf(&exportFuncType, &importFuncType)) {
       const Import& import = FindImportFunction(imports_, i);
       UniqueChars importModuleName = import.module.toQuotedString(cx);
       UniqueChars importFieldName = import.field.toQuotedString(cx);

js/src/wasm/WasmSerialize.cpp

@@ -1163,10 +1163,20 @@ CoderResult CodeModule(Coder<MODE_DECODE>& coder, MutableModule* item) {
 
   MOZ_RELEASE_ASSERT(EqualContainers(currentBuildId, deserializedBuildId));
 
+  CustomSectionVector customSections;
+  MOZ_TRY(Magic(coder, Marker::CustomSections));
+  MOZ_TRY(
+      (CodeVector<MODE_DECODE, CustomSection, &CodeCustomSection<MODE_DECODE>>(
+          coder, &customSections)));
+
   LinkData linkData(Tier::Serialized);
   MOZ_TRY(Magic(coder, Marker::LinkData));
   MOZ_TRY(CodeLinkData(coder, &linkData));
 
+  SharedCode code;
+  MOZ_TRY(Magic(coder, Marker::Code));
+  MOZ_TRY(CodeSharedCode(coder, &code, linkData, customSections));
+
   ImportVector imports;
   MOZ_TRY(Magic(coder, Marker::Imports));
   MOZ_TRY((CodeVector<MODE_DECODE, Import, &CodeImport<MODE_DECODE>>(
@@ -1184,22 +1194,13 @@ CoderResult CodeModule(Coder<MODE_DECODE>& coder, MutableModule* item) {
                                   &CodeDataSegment<MODE_DECODE>>>(
       coder, &dataSegments)));
 
+  // This must happen after deserializing code so we get type definitions.
   ModuleElemSegmentVector elemSegments;
   MOZ_TRY(Magic(coder, Marker::ElemSegments));
   MOZ_TRY(
       (CodeVector<MODE_DECODE, ModuleElemSegment,
                   &CodeModuleElemSegment<MODE_DECODE>>(coder, &elemSegments)));
 
-  CustomSectionVector customSections;
-  MOZ_TRY(Magic(coder, Marker::CustomSections));
-  MOZ_TRY(
-      (CodeVector<MODE_DECODE, CustomSection, &CodeCustomSection<MODE_DECODE>>(
-          coder, &customSections)));
-
-  SharedCode code;
-  MOZ_TRY(Magic(coder, Marker::Code));
-  MOZ_TRY(CodeSharedCode(coder, &code, linkData, customSections));
-
   *item = js_new<Module>(*code, std::move(imports), std::move(exports),
                          std::move(dataSegments), std::move(elemSegments),
                          std::move(customSections), nullptr,
@@ -1220,8 +1221,13 @@ CoderResult CodeModule(Coder<mode>& coder, CoderArg<mode, Module> item,
     return Err(OutOfMemory());
   }
   MOZ_TRY(CodePodVector(coder, &currentBuildId));
+  MOZ_TRY(Magic(coder, Marker::CustomSections));
+  MOZ_TRY((CodeVector<mode, CustomSection, &CodeCustomSection<mode>>(
+      coder, &item->customSections_)));
   MOZ_TRY(Magic(coder, Marker::LinkData));
   MOZ_TRY(CodeLinkData(coder, &linkData));
+  MOZ_TRY(Magic(coder, Marker::Code));
+  MOZ_TRY(CodeSharedCode(coder, &item->code_, linkData));
   MOZ_TRY(Magic(coder, Marker::Imports));
   MOZ_TRY(
       (CodeVector<mode, Import, &CodeImport<mode>>(coder, &item->imports_)));
@@ -1236,11 +1242,6 @@ CoderResult CodeModule(Coder<mode>& coder, CoderArg<mode, Module> item,
   MOZ_TRY(Magic(coder, Marker::ElemSegments));
   MOZ_TRY((CodeVector<mode, ModuleElemSegment, CodeModuleElemSegment<mode>>(
       coder, &item->elemSegments_)));
-  MOZ_TRY(Magic(coder, Marker::CustomSections));
-  MOZ_TRY((CodeVector<mode, CustomSection, &CodeCustomSection<mode>>(
-      coder, &item->customSections_)));
-  MOZ_TRY(Magic(coder, Marker::Code));
-  MOZ_TRY(CodeSharedCode(coder, &item->code_, linkData));
   return Ok();
 }
 

js/src/wasm/WasmValue.cpp

@@ -859,6 +859,10 @@ bool wasm::ToJSValue(JSContext* cx, const void* src, ValType type,
 
 /* static */
 wasm::FuncRef wasm::FuncRef::fromAnyRefUnchecked(AnyRef p) {
+  if (p.isNull()) {
+    return FuncRef::null();
+  }
+
   MOZ_ASSERT(p.isJSObject() && p.toJSObject().is<JSFunction>());
   return FuncRef(&p.toJSObject().as<JSFunction>());
 }

js/src/wasm/WasmValue.h

@@ -118,6 +118,8 @@ class FuncRef {
   // FuncRef.
   static FuncRef fromAnyRefUnchecked(AnyRef p);
 
+  static FuncRef null() { return FuncRef(nullptr); }
+
   AnyRef toAnyRef() { return AnyRef::fromJSObjectOrNull((JSObject*)value_); }
 
   void* forCompiledCode() const { return value_; }