[wasm-gc] Typecheck function-typed imported globals

Changes:
- Add optional WasmModuleObject field to WasmGlobalObject
- Introduce DynamicTypeCheckRef. Use it to typecheck imported global
  objects.
- Correctly typecheck imported WasmGlobalObjects.
- Add some testing infrastructure and one test file

Bug: v8:7748
Change-Id: Icc62d378d17696c5808d580f1ec84186c9556ec1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2403248Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69872}

src/wasm/c-api.cc

@@ -1686,7 +1686,8 @@ auto Global::make(Store* store_abs, const GlobalType* type, const Val& val)
   bool is_mutable = (type->mutability() == VAR);
   const int32_t offset = 0;
   i::Handle<i::WasmGlobalObject> obj =
-      i::WasmGlobalObject::New(isolate, i::MaybeHandle<i::JSArrayBuffer>(),
+      i::WasmGlobalObject::New(isolate, i::Handle<i::WasmInstanceObject>(),
+                               i::MaybeHandle<i::JSArrayBuffer>(),
                                i::MaybeHandle<i::FixedArray>(), i_type, offset,
                                is_mutable)
           .ToHandleChecked();

src/wasm/module-instantiate.cc

@@ -1246,10 +1246,17 @@ bool InstanceBuilder::ProcessImportedWasmGlobalObject(
     return false;
   }
 
-  bool is_sub_type =
-      IsSubtypeOf(global_object->type(), global.type, instance->module());
-  bool is_same_type = global_object->type() == global.type;
-  bool valid_type = global.mutability ? is_same_type : is_sub_type;
+  const WasmModule* global_type_module =
+      !global_object->instance().IsUndefined()
+          ? WasmInstanceObject::cast(global_object->instance()).module()
+          : instance->module();
+
+  bool valid_type =
+      global.mutability
+          ? EquivalentTypes(global_object->type(), global.type,
+                            global_type_module, instance->module())
+          : IsSubtypeOf(global_object->type(), global.type, global_type_module,
+                        instance->module());
 
   if (!valid_type) {
     ReportLinkError("imported global does not match the expected type",
@@ -1340,14 +1347,11 @@ bool InstanceBuilder::ProcessImportedGlobal(Handle<WasmInstanceObject> instance,
   }
 
   if (global.type.is_reference_type()) {
-    if (global.type.is_reference_to(HeapType::kFunc)) {
-      if (!value->IsNull(isolate_) &&
-          !WasmExportedFunction::IsWasmExportedFunction(*value)) {
-        ReportLinkError(
-            "imported funcref global must be null or an exported function",
-            import_index, module_name, import_name);
-        return false;
-      }
+    const char* error_message;
+    if (!wasm::DynamicTypeCheckRef(isolate_, module_, value, global.type,
+                                   &error_message)) {
+      ReportLinkError(error_message, global_index, module_name, import_name);
+      return false;
     }
     WriteGlobalExternRef(global, value);
     return true;
@@ -1363,8 +1367,10 @@ bool InstanceBuilder::ProcessImportedGlobal(Handle<WasmInstanceObject> instance,
     return true;
   }
 
-  ReportLinkError("global import must be a number or WebAssembly.Global object",
-                  import_index, module_name, import_name);
+  ReportLinkError(
+      "global import must be a number, valid Wasm reference, or "
+      "WebAssembly.Global object",
+      import_index, module_name, import_name);
   return false;
 }
 
@@ -1805,8 +1811,9 @@ void InstanceBuilder::ProcessExports(Handle<WasmInstanceObject> instance) {
         // Since the global's array untagged_buffer is always provided,
         // allocation should never fail.
         Handle<WasmGlobalObject> global_obj =
-            WasmGlobalObject::New(isolate_, untagged_buffer, tagged_buffer,
-                                  global.type, offset, global.mutability)
+            WasmGlobalObject::New(isolate_, instance, untagged_buffer,
+                                  tagged_buffer, global.type, offset,
+                                  global.mutability)
                 .ToHandleChecked();
         desc.set_value(global_obj);
         break;

src/wasm/wasm-js.cc

@@ -1213,9 +1213,9 @@ bool GetValueType(Isolate* isolate, MaybeLocal<Value> maybe,
   } else if (enabled_features.has_reftypes() &&
              string->StringEquals(v8_str(isolate, "externref"))) {
     *type = i::wasm::kWasmExternRef;
-    // The JS api spec uses 'anyfunc' instead of 'funcref'.
   } else if (enabled_features.has_reftypes() &&
              string->StringEquals(v8_str(isolate, "anyfunc"))) {
+    // The JS api spec uses 'anyfunc' instead of 'funcref'.
     *type = i::wasm::kWasmFuncRef;
   } else if (enabled_features.has_eh() &&
              string->StringEquals(v8_str(isolate, "exnref"))) {
@@ -1279,7 +1279,8 @@ void WebAssemblyGlobal(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
   const uint32_t offset = 0;
   i::MaybeHandle<i::WasmGlobalObject> maybe_global_obj =
-      i::WasmGlobalObject::New(i_isolate, i::MaybeHandle<i::JSArrayBuffer>(),
+      i::WasmGlobalObject::New(i_isolate, i::Handle<i::WasmInstanceObject>(),
+                               i::MaybeHandle<i::JSArrayBuffer>(),
                                i::MaybeHandle<i::FixedArray>(), type, offset,
                                is_mutable);
 
@@ -1820,7 +1821,6 @@ void WebAssemblyGlobalGetValueCommon(
       auto enabled_features = i::wasm::WasmFeatures::FromIsolate(i_isolate);
       if (enabled_features.has_bigint()) {
         Local<BigInt> value = BigInt::New(isolate, receiver->GetI64());
-
         return_value.Set(value);
       } else {
         thrower.TypeError("Can't get the value of i64 WebAssembly.Global");

src/wasm/wasm-objects-inl.h

@@ -120,6 +120,7 @@ SMI_ACCESSORS(WasmMemoryObject, maximum_pages, kMaximumPagesOffset)
 OPTIONAL_ACCESSORS(WasmMemoryObject, instances, WeakArrayList, kInstancesOffset)
 
 // WasmGlobalObject
+ACCESSORS(WasmGlobalObject, instance, HeapObject, kInstanceOffset)
 ACCESSORS(WasmGlobalObject, untagged_buffer, JSArrayBuffer,
           kUntaggedBufferOffset)
 ACCESSORS(WasmGlobalObject, tagged_buffer, FixedArray, kTaggedBufferOffset)
@@ -130,7 +131,7 @@ SMI_ACCESSORS(WasmGlobalObject, raw_type, kRawTypeOffset)
 SMI_ACCESSORS(WasmGlobalObject, is_mutable, kIsMutableOffset)
 
 wasm::ValueType WasmGlobalObject::type() const {
-  return wasm::ValueType::FromRawBitField(raw_type());
+  return wasm::ValueType::FromRawBitField(static_cast<uint32_t>(raw_type()));
 }
 void WasmGlobalObject::set_type(wasm::ValueType value) {
   set_raw_type(static_cast<int>(value.raw_bit_field()));

src/wasm/wasm-objects.cc

@@ -27,6 +27,7 @@
 #include "src/wasm/wasm-limits.h"
 #include "src/wasm/wasm-module.h"
 #include "src/wasm/wasm-objects-inl.h"
+#include "src/wasm/wasm-subtyping.h"
 #include "src/wasm/wasm-value.h"
 
 #define TRACE_IFT(...)              \
@@ -940,7 +941,8 @@ int32_t WasmMemoryObject::Grow(Isolate* isolate,
 
 // static
 MaybeHandle<WasmGlobalObject> WasmGlobalObject::New(
-    Isolate* isolate, MaybeHandle<JSArrayBuffer> maybe_untagged_buffer,
+    Isolate* isolate, Handle<WasmInstanceObject> instance,
+    MaybeHandle<JSArrayBuffer> maybe_untagged_buffer,
     MaybeHandle<FixedArray> maybe_tagged_buffer, wasm::ValueType type,
     int32_t offset, bool is_mutable) {
   Handle<JSFunction> global_ctor(
@@ -950,7 +952,7 @@ MaybeHandle<WasmGlobalObject> WasmGlobalObject::New(
   {
     // Disallow GC until all fields have acceptable types.
     DisallowHeapAllocation no_gc;
-
+    if (!instance.is_null()) global_obj->set_instance(*instance);
     global_obj->set_raw_type(0);
     global_obj->set_type(type);
     global_obj->set_offset(offset);
@@ -2008,6 +2010,90 @@ Handle<AsmWasmData> AsmWasmData::New(
   return result;
 }
 
+namespace wasm {
+
+bool DynamicTypeCheckRef(Isolate* isolate, const WasmModule* module,
+                         Handle<Object> value, ValueType expected,
+                         const char** error_message) {
+  DCHECK(expected.is_reference_type());
+  switch (expected.kind()) {
+    case ValueType::kOptRef:
+      if (value->IsNull(isolate)) return true;
+      V8_FALLTHROUGH;
+    case ValueType::kRef:
+      switch (expected.heap_representation()) {
+        case HeapType::kFunc: {
+          if (!WasmExportedFunction::IsWasmExportedFunction(*value)) {
+            *error_message =
+                "function-typed object must be null (if nullable) or an "
+                "exported function";
+            return false;
+          }
+          return true;
+        }
+        case HeapType::kExtern:
+        case HeapType::kExn:
+          return true;
+        case HeapType::kEq:
+        case HeapType::kI31:
+          // TODO(7748): Implement when the JS API for structs/arrays/i31ref is
+          // decided on.
+          *error_message =
+              "Assigning to eqref/i31ref globals not supported yet.";
+          return false;
+        default:
+          // Tables defined outside a module can't refer to user-defined types.
+          if (module == nullptr) return false;
+          DCHECK(module->has_type(expected.heap_representation()));
+          if (module->has_signature(expected.heap_representation())) {
+            if (WasmExportedFunction::IsWasmExportedFunction(*value)) {
+              WasmExportedFunction function =
+                  WasmExportedFunction::cast(*value);
+              const WasmModule* exporting_module = function.instance().module();
+              ValueType real_type = ValueType::Ref(
+                  exporting_module->functions[function.function_index()]
+                      .sig_index,
+                  kNonNullable);
+              if (!IsSubtypeOf(real_type, expected, exporting_module, module)) {
+                *error_message =
+                    "exported function object must be a subtype of the "
+                    "global's formal type";
+                return false;
+              }
+              return true;
+            }
+
+            if (WasmJSFunction::IsWasmJSFunction(*value)) {
+              // TODO(9495): Implement when we are confident about the type
+              // reflection proposal.
+              *error_message =
+                  "Assigning WasmJSFunction objects to globals not supported "
+                  "yet.";
+              return false;
+            }
+
+            *error_message =
+                "function-typed object must be null (if nullable) or an "
+                "exported function";
+            return false;
+          }
+          // TODO(7748): Implement when the JS API for structs/arrays is decided
+          // on.
+          *error_message =
+              "Assigning to struct/array globals not supported yet.";
+          return false;
+      }
+    case ValueType::kRtt:
+      // TODO(7748): Implement when the JS API for rtts is decided on.
+      *error_message = "Assigning to rtt globals not supported yet.";
+      return false;
+    default:
+      UNREACHABLE();
+  }
+}
+
+}  // namespace wasm
+
 }  // namespace internal
 }  // namespace v8
 

src/wasm/wasm-objects.h

@@ -24,16 +24,11 @@
 namespace v8 {
 namespace internal {
 namespace wasm {
-struct CompilationEnv;
 class InterpretedFrame;
-struct InterpretedFrameDeleter;
 class NativeModule;
-class SignatureMap;
 class WasmCode;
 struct WasmException;
-class WasmFeatures;
 struct WasmGlobal;
-class WasmInterpreter;
 struct WasmModule;
 class WasmValue;
 class WireBytesRef;
@@ -320,6 +315,11 @@ class WasmGlobalObject : public JSObject {
  public:
   DECL_CAST(WasmGlobalObject)
 
+  // The instance in which this WasmGlobalObject is defined.
+  // This field is undefined if the global is defined outside any Wasm module,
+  // i.e., through the JS API (WebAssembly.Global).
+  // Because it might be undefined, we declare it as a HeapObject.
+  DECL_ACCESSORS(instance, HeapObject)
   DECL_ACCESSORS(untagged_buffer, JSArrayBuffer)
   DECL_ACCESSORS(tagged_buffer, FixedArray)
   DECL_INT32_ACCESSORS(offset)
@@ -337,7 +337,8 @@ class WasmGlobalObject : public JSObject {
                                 TORQUE_GENERATED_WASM_GLOBAL_OBJECT_FIELDS)
 
   V8_EXPORT_PRIVATE static MaybeHandle<WasmGlobalObject> New(
-      Isolate* isolate, MaybeHandle<JSArrayBuffer> maybe_untagged_buffer,
+      Isolate* isolate, Handle<WasmInstanceObject> instance,
+      MaybeHandle<JSArrayBuffer> maybe_untagged_buffer,
       MaybeHandle<FixedArray> maybe_tagged_buffer, wasm::ValueType type,
       int32_t offset, bool is_mutable);
 
@@ -956,6 +957,9 @@ Handle<Map> AllocateSubRtt(Isolate* isolate,
                            Handle<WasmInstanceObject> instance, uint32_t type,
                            Handle<Map> parent);
 
+bool DynamicTypeCheckRef(Isolate* isolate, const WasmModule* module,
+                         Handle<Object> value, ValueType expected,
+                         const char** error_message);
 }  // namespace wasm
 
 }  // namespace internal

src/wasm/wasm-objects.tq

@@ -73,6 +73,7 @@ extern class WasmMemoryObject extends JSObject {
 type WasmValueType extends uint8 constexpr 'wasm::ValueType::Kind';
 
 extern class WasmGlobalObject extends JSObject {
+  instance: WasmInstanceObject|Undefined;
   untagged_buffer: JSArrayBuffer|Undefined;
   tagged_buffer: FixedArray|Undefined;
   offset: Smi;

test/mjsunit/wasm/errors.js

@@ -110,7 +110,8 @@ function import_error(index, module, func, msg) {
   b = builder();
   msg = import_error(
       0, 'foo', 'bar',
-      'global import must be a number or WebAssembly.Global object');
+      'global import must be a number, valid Wasm reference, '
+        + 'or WebAssembly.Global object');
   b.addImportedGlobal('foo', 'bar', kWasmI32);
   assertLinkError(b.toBuffer(), {foo: {}}, msg);
   b = builder();

test/mjsunit/wasm/reference-globals.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --experimental-wasm-gc
+
+load("test/mjsunit/wasm/wasm-module-builder.js");
+
+(function Test1() {
+  var exporting_instance = (function () {
+    var builder = new WasmModuleBuilder();
+
+    var sig_index = builder.addType(kSig_i_ii);
+    var wrong_sig_index = builder.addType(kSig_i_i);
+
+    var addition_index = builder.addFunction("addition", sig_index)
+      .addBody([kExprLocalGet, 0, kExprLocalGet, 1, kExprI32Add])
+      .exportFunc();
+
+    var global = builder.addGlobal(wasmRefType(sig_index), false);
+    global.function_index = addition_index;
+    global.exportAs("global");
+    builder.addGlobal(wasmOptRefType(wrong_sig_index), false)
+      .exportAs("mistyped_global");
+
+    return builder.instantiate({});
+  })();
+
+  // Mistyped imported global.
+  assertThrows(
+    () => {
+      var builder = new WasmModuleBuilder();
+      var sig_index = builder.addType(kSig_i_ii);
+      builder.addImportedGlobal("imports", "global", wasmOptRefType(sig_index),
+                                false);
+      builder.instantiate(
+        {imports: { global: exporting_instance.exports.mistyped_global }})},
+    WebAssembly.LinkError,
+    /imported global does not match the expected type/
+  );
+
+  // Mistyped imported global due to cross-module typechecking.
+  assertThrows(
+    () => {
+      var builder = new WasmModuleBuilder();
+      var sig_index = builder.addType(kSig_i_i);
+      builder.addImportedGlobal("imports", "global", wasmOptRefType(sig_index),
+                                false);
+      builder.instantiate(
+        {imports: { global: exporting_instance.exports.global }})},
+    WebAssembly.LinkError,
+    /imported global does not match the expected type/
+  );
+
+  // Non-function imported into function-typed global.
+  assertThrows(
+    () => {
+      var builder = new WasmModuleBuilder();
+      var sig_index = builder.addType(kSig_i_ii);
+      builder.addImportedGlobal("imports", "global", wasmOptRefType(sig_index),
+                                false);
+      builder.instantiate({imports: { global: 42 }})},
+    WebAssembly.LinkError,
+    /function-typed object must be null \(if nullable\) or an exported function/
+  );
+
+  // Mistyped function import.
+  assertThrows(
+    () => {
+      var builder = new WasmModuleBuilder();
+      var sig_index = builder.addType(kSig_i_i);
+      builder.addImportedGlobal("imports", "global", wasmRefType(sig_index),
+                                false);
+      builder.instantiate(
+        {imports: { global: exporting_instance.exports.addition }})},
+    WebAssembly.LinkError,
+    /exported function object must be a subtype of the global's formal type/
+  );
+
+  var instance = (function () {
+    var builder = new WasmModuleBuilder();
+
+    var sig_index = builder.addType(kSig_i_ii);
+
+    builder.addImportedGlobal("imports", "global", wasmOptRefType(sig_index),
+                              false);
+
+    builder.addFunction("test_import", kSig_i_ii)
+      .addBody([kExprLocalGet, 0, kExprLocalGet, 1, kExprGlobalGet, 0,
+                kExprCallRef])
+      .exportFunc();
+
+    return builder.instantiate({imports: {
+      global: exporting_instance.exports.global
+    }});
+  })();
+
+  // This module is valid.
+  assertFalse(instance === undefined);
+  assertFalse(instance === null);
+  assertFalse(instance === 0);
+
+  // The correct function reference has been passed.
+  assertEquals(66, instance.exports.test_import(42, 24));
+})();

test/mjsunit/wasm/wasm-module-builder.js

@@ -99,12 +99,13 @@ let kWasmI64 = 0x7e;
 let kWasmF32 = 0x7d;
 let kWasmF64 = 0x7c;
 let kWasmS128 = 0x7b;
-let kWasmExternRef = 0x6f;
 let kWasmAnyFunc = 0x70;
-let kWasmExnRef = 0x68;
-function wasmRefType(index) { return {opcode: 0x6b, index: index}; }
+let kWasmExternRef = 0x6f;
 function wasmOptRefType(index) { return {opcode: 0x6c, index: index}; }
+function wasmRefType(index) { return {opcode: 0x6b, index: index}; }
+let kWasmI31Ref = 0x6a;
 function wasmRtt(index, depth) { return {opcode: 0x69, index: index, depth: depth}; }
+let kWasmExnRef = 0x68;
 
 let kExternalFunction = 0;
 let kExternalTable = 1;
@@ -395,6 +396,7 @@ let kAtomicPrefix = 0xfe;
 // GC opcodes
 let kExprRttCanon = 0x30;
 let kExprRefCast = 0x41;
+let kExprI31New = 0x20;
 
 // Numeric opcodes.
 let kExprMemoryInit = 0x08;
@@ -1062,7 +1064,8 @@ class WasmModuleBuilder {
       throw new Error('Imported exceptions must be declared before local ones');
     }
     let type_index = (typeof type) == "number" ? type : this.addType(type);
-    let o = {module: module, name: name, kind: kExternalException, type_index: type_index};
+    let o = {module: module, name: name, kind: kExternalException,
+             type_index: type_index};
     this.imports.push(o);
     return this.num_imported_exceptions++;
   }
@@ -1073,6 +1076,12 @@ class WasmModuleBuilder {
   }
 
   addExportOfKind(name, kind, index) {
+    if (index == undefined && kind != kExternalTable && kind != kExternalMemory) {
+      throw new Error('Index for exports other than tables/memories must be provided');
+    }
+    if (index !== undefined && (typeof index) != 'number') {
+      throw new Error('Index for exports must be a number')
+    }
     this.exports.push({name: name, kind: kind, index: index});
     return this;
   }