[wasm-gc] Implement function subtyping

Changes:
- Implement function subtyping in wasm-subtyping.cc.
- Add Signature::Build(), which takes initializer lists for the return
  and parameter types.
- Only throw kTrapFuncSigMismatch in call_indirect, change that trap's
  message.
- Add a missing "return 0" in function-body-decoder-impl.h
- Fix a faulty check in wasm-objects.cc.
- Improve some comments.
- Write tests. Improve readability of subtyping-unittest.

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

src/codegen/signature.h

@@ -104,6 +104,14 @@ class Signature : public ZoneObject {
     T* buffer_;
   };
 
+  static Signature<T>* Build(Zone* zone, std::initializer_list<T> returns,
+                             std::initializer_list<T> params) {
+    Builder builder(zone, returns.size(), params.size());
+    for (T ret : returns) builder.AddReturn(ret);
+    for (T param : params) builder.AddParam(param);
+    return builder.Build();
+  }
+
   static constexpr size_t kReturnCountOffset = 0;
   static constexpr size_t kParameterCountOffset =
       kReturnCountOffset + kSizetSize;

src/common/message-template.h

@@ -564,7 +564,7 @@ namespace internal {
   T(WasmTrapRemByZero, "remainder by zero")                                    \
   T(WasmTrapFloatUnrepresentable, "float unrepresentable in integer range")    \
   T(WasmTrapTableOutOfBounds, "table index is out of bounds")                  \
-  T(WasmTrapFuncSigMismatch, "function signature mismatch")                    \
+  T(WasmTrapFuncSigMismatch, "null function or function signature mismatch")   \
   T(WasmTrapMultiReturnLengthMismatch, "multi-return length mismatch")         \
   T(WasmTrapJSTypeError, "type incompatibility when transforming from/to JS")  \
   T(WasmTrapDataSegmentDropped, "data segment has been dropped")               \

src/compiler/wasm-compiler.cc

@@ -3074,12 +3074,13 @@ Node* WasmGraphBuilder::BuildIndirectCall(uint32_t table_index,
 
   const wasm::ValueType table_type = env_->module->tables[table_index].type;
   // Check that the table entry is not null and that the type of the function is
-  // a subtype of the function type declared at the call site. In the absence of
-  // function subtyping, the latter can only happen if the table type is (ref
-  // null? func). Also, subtyping reduces to normalized signature equality
-  // checking.
-  // TODO(7748): Expand this with function subtyping once we have that.
+  // **identical with** the function type declared at the call site (no
+  // subtyping of functions is allowed).
+  // Note: Since null entries are identified by having ift_sig_id (-1), we only
+  // need one comparison.
+  // TODO(9495): Change this if we should do full function subtyping instead.
   const bool needs_signature_check =
+      FLAG_experimental_wasm_gc ||
       table_type.is_reference_to(wasm::HeapType::kFunc) ||
       table_type.is_nullable();
   if (needs_signature_check) {
@@ -3088,19 +3089,10 @@ Node* WasmGraphBuilder::BuildIndirectCall(uint32_t table_index,
 
     Node* loaded_sig = gasm_->LoadFromObject(MachineType::Int32(), ift_sig_ids,
                                              int32_scaled_key);
-
-    if (table_type.is_reference_to(wasm::HeapType::kFunc)) {
-      int32_t expected_sig_id = env_->module->canonicalized_type_ids[sig_index];
-      Node* sig_match =
-          gasm_->Word32Equal(loaded_sig, Int32Constant(expected_sig_id));
-      TrapIfFalse(wasm::kTrapFuncSigMismatch, sig_match, position);
-    } else {
-      // If the table entries are nullable, we still have to check that the
-      // entry is initialized.
-      Node* function_is_null =
-          gasm_->Word32Equal(loaded_sig, Int32Constant(-1));
-      TrapIfTrue(wasm::kTrapNullDereference, function_is_null, position);
-    }
+    int32_t expected_sig_id = env_->module->canonicalized_type_ids[sig_index];
+    Node* sig_match =
+        gasm_->Word32Equal(loaded_sig, Int32Constant(expected_sig_id));
+    TrapIfFalse(wasm::kTrapFuncSigMismatch, sig_match, position);
   }
 
   Node* key_intptr = BuildChangeUint32ToUintPtr(key);

src/wasm/baseline/liftoff-compiler.cc

@@ -5549,7 +5549,6 @@ class LiftoffCompiler {
     __ Load(LiftoffRegister(scratch), table, index, 0, LoadType::kI32Load,
             pinned);
 
-    // TODO(9495): Do not always compare signatures, same as wasm-compiler.cc.
     // Compare against expected signature.
     __ LoadConstant(LiftoffRegister(tmp_const), WasmValue(canonical_sig_num));
 

src/wasm/function-body-decoder-impl.h

@@ -1442,6 +1442,7 @@ class WasmDecoder : public Decoder {
   }
 
   inline bool Validate(const byte* pc, CallIndirectImmediate<validate>& imm) {
+    // Validate immediate table index.
     if (!VALIDATE(imm.table_index < module_->tables.size())) {
       DecodeError(pc, "call_indirect: table index immediate out of bounds");
       return false;
@@ -1453,10 +1454,13 @@ class WasmDecoder : public Decoder {
           imm.table_index);
       return false;
     }
+
+    // Validate immediate signature index.
     if (!Complete(imm)) {
       DecodeError(pc, "invalid signature index: #%u", imm.sig_index);
       return false;
     }
+
     // Check that the dynamic signature for this call is a subtype of the static
     // type of the table the function is defined in.
     ValueType immediate_type = ValueType::Ref(imm.sig_index, kNonNullable);
@@ -1465,6 +1469,7 @@ class WasmDecoder : public Decoder {
                   "call_indirect: Immediate signature #%u is not a subtype of "
                   "immediate table #%u",
                   imm.sig_index, imm.table_index);
+      return false;
     }
     return true;
   }
@@ -4359,7 +4364,6 @@ class WasmFullDecoder : public WasmDecoder<validate> {
               CALL_INTERFACE_IF_OK_AND_REACHABLE(Drop);
               CALL_INTERFACE_IF_OK_AND_REACHABLE(AssertNull, obj, &value);
             } else {
-              // TODO(manoskouk): Change the trap label.
               CALL_INTERFACE_IF_OK_AND_REACHABLE(Trap,
                                                  TrapReason::kTrapIllegalCast);
               EndControl();

src/wasm/wasm-objects.cc

@@ -454,21 +454,19 @@ void WasmTableObject::Set(Isolate* isolate, Handle<WasmTableObject> table,
     case wasm::HeapType::kEq:
     case wasm::HeapType::kData:
     case wasm::HeapType::kI31:
-      // TODO(7748): Implement once we have a story for struct/arrays/i31ref in
-      // JS.
-      UNIMPLEMENTED();
+      // TODO(7748): Implement once we have struct/arrays/i31ref tables.
+      UNREACHABLE();
     case wasm::HeapType::kBottom:
       UNREACHABLE();
     default:
       DCHECK(!table->instance().IsUndefined());
-      if (WasmInstanceObject::cast(table->instance())
-              .module()
-              ->has_signature(entry_index)) {
-        SetFunctionTableEntry(isolate, table, entries, entry_index, entry);
-        return;
-      }
-      // TODO(7748): Implement once we have a story for struct/arrays in JS.
-      UNIMPLEMENTED();
+      // TODO(7748): Relax this once we have struct/array/i31ref tables.
+      DCHECK_EQ(WasmInstanceObject::cast(table->instance())
+                    .module()
+                    ->type_kinds[table->type().ref_index()],
+                wasm::kWasmFunctionTypeCode);
+      SetFunctionTableEntry(isolate, table, entries, entry_index, entry);
+      return;
   }
 }
 
@@ -2256,7 +2254,8 @@ bool TypecheckJSObject(Isolate* isolate, const WasmModule* module,
 
             if (WasmJSFunction::IsWasmJSFunction(*value)) {
               // Since a WasmJSFunction cannot refer to indexed types (definable
-              // only in a module), we do not need to use EquivalentTypes().
+              // only in a module), we do not need full function subtyping.
+              // TODO(manoskouk): Change this if wasm types can be exported.
               if (!WasmJSFunction::cast(*value).MatchesSignature(
                       module->signature(expected.ref_index()))) {
                 *error_message =
@@ -2268,11 +2267,12 @@ bool TypecheckJSObject(Isolate* isolate, const WasmModule* module,
             }
 
             if (WasmCapiFunction::IsWasmCapiFunction(*value)) {
+              // Since a WasmCapiFunction cannot refer to indexed types
+              // (definable only in a module), we do not need full function
+              // subtyping.
+              // TODO(manoskouk): Change this if wasm types can be exported.
               if (!WasmCapiFunction::cast(*value).MatchesSignature(
                       module->signature(expected.ref_index()))) {
-                // Since a WasmCapiFunction cannot refer to indexed types
-                // (definable in a module), we don't need to invoke
-                // IsEquivalentType();
                 *error_message =
                     "assigned WasmCapiFunction has to be a subtype of the "
                     "expected type";

src/wasm/wasm-subtyping.cc

@@ -258,14 +258,46 @@ bool ArrayIsSubtypeOf(uint32_t subtype_index, uint32_t supertype_index,
   }
 }
 
-// TODO(7748): Expand this with function subtyping when it is introduced.
 bool FunctionIsSubtypeOf(uint32_t subtype_index, uint32_t supertype_index,
                          const WasmModule* sub_module,
                          const WasmModule* super_module) {
-  return FunctionEquivalentIndices(subtype_index, supertype_index, sub_module,
-                                   super_module);
-}
+  if (!FLAG_experimental_wasm_gc) {
+    return FunctionEquivalentIndices(subtype_index, supertype_index, sub_module,
+                                     super_module);
+  }
+  const FunctionSig* sub_func = sub_module->types[subtype_index].function_sig;
+  const FunctionSig* super_func =
+      super_module->types[supertype_index].function_sig;
+
+  if (sub_func->parameter_count() != super_func->parameter_count() ||
+      sub_func->return_count() != super_func->return_count()) {
+    return false;
+  }
 
+  TypeJudgementCache::instance()->cache_subtype(subtype_index, supertype_index,
+                                                sub_module, super_module);
+
+  for (uint32_t i = 0; i < sub_func->parameter_count(); i++) {
+    // Contravariance for params.
+    if (!IsSubtypeOf(super_func->parameters()[i], sub_func->parameters()[i],
+                     super_module, sub_module)) {
+      TypeJudgementCache::instance()->uncache_subtype(
+          subtype_index, supertype_index, sub_module, super_module);
+      return false;
+    }
+  }
+  for (uint32_t i = 0; i < sub_func->return_count(); i++) {
+    // Covariance for returns.
+    if (!IsSubtypeOf(sub_func->returns()[i], super_func->returns()[i],
+                     sub_module, super_module)) {
+      TypeJudgementCache::instance()->uncache_subtype(
+          subtype_index, supertype_index, sub_module, super_module);
+      return false;
+    }
+  }
+
+  return true;
+}
 }  // namespace
 
 V8_NOINLINE V8_EXPORT_PRIVATE bool IsSubtypeOfImpl(

src/wasm/wasm-subtyping.h

@@ -60,8 +60,10 @@ V8_NOINLINE bool EquivalentTypes(ValueType type1, ValueType type2,
 // - Struct subtyping: Subtype must have at least as many fields as supertype,
 //   covariance for immutable fields, equivalence for mutable fields.
 // - Array subtyping (mutable only) is the equivalence relation.
-// - Function subtyping is the equivalence relation (note: this rule might
-//   change in the future to include type variance).
+// - Function subtyping depends on the enabled wasm features: if
+//   --experimental-wasm-gc is enabled, then subtyping is computed
+//   contravariantly for parameter types and covariantly for return types.
+//   Otherwise, the subtyping relation is the equivalence relation.
 V8_INLINE bool IsSubtypeOf(ValueType subtype, ValueType supertype,
                            const WasmModule* sub_module,
                            const WasmModule* super_module) {

test/cctest/wasm/test-gc.cc

@@ -43,8 +43,8 @@ class WasmGCTester {
             &v8::internal::FLAG_liftoff_only,
             execution_tier == TestExecutionTier::kLiftoff ? true : false),
         flag_tierup(&v8::internal::FLAG_wasm_tier_up, false),
-        zone(&allocator, ZONE_NAME),
-        builder_(&zone),
+        zone_(&allocator, ZONE_NAME),
+        builder_(&zone_),
         isolate_(CcTest::InitIsolateOnce()),
         scope(isolate_),
         thrower(isolate_, "Test wasm GC") {
@@ -82,7 +82,7 @@ class WasmGCTester {
   }
 
   byte DefineStruct(std::initializer_list<F> fields) {
-    StructType::Builder type_builder(&zone,
+    StructType::Builder type_builder(&zone_,
                                      static_cast<uint32_t>(fields.size()));
     for (F field : fields) {
       type_builder.AddField(field.first, field.second);
@@ -91,7 +91,8 @@ class WasmGCTester {
   }
 
   byte DefineArray(ValueType element_type, bool mutability) {
-    return builder_.AddArrayType(zone.New<ArrayType>(element_type, mutability));
+    return builder_.AddArrayType(
+        zone_.New<ArrayType>(element_type, mutability));
   }
 
   byte DefineSignature(FunctionSig* sig) { return builder_.AddSignature(sig); }
@@ -101,7 +102,7 @@ class WasmGCTester {
   }
 
   void CompileModule() {
-    ZoneBuffer buffer(&zone);
+    ZoneBuffer buffer(&zone_);
     builder_.WriteTo(&buffer);
     MaybeHandle<WasmInstanceObject> maybe_instance =
         testing::CompileAndInstantiateForTesting(
@@ -174,6 +175,7 @@ class WasmGCTester {
   Handle<WasmInstanceObject> instance() { return instance_; }
   Isolate* isolate() { return isolate_; }
   WasmModuleBuilder* builder() { return &builder_; }
+  Zone* zone() { return &zone_; }
 
   TestSignatures sigs;
 
@@ -186,7 +188,7 @@ class WasmGCTester {
   const FlagScope<bool> flag_tierup;
 
   v8::internal::AccountingAllocator allocator;
-  Zone zone;
+  Zone zone_;
   WasmModuleBuilder builder_;
 
   Isolate* const isolate_;
@@ -1484,19 +1486,101 @@ WASM_COMPILED_EXEC_TEST(GlobalInitReferencingGlobal) {
   tester.CheckResult(func, 42);
 }
 
-WASM_COMPILED_EXEC_TEST(IndirectNullSetManually) {
+WASM_COMPILED_EXEC_TEST(GCTables) {
   WasmGCTester tester(execution_tier);
-  byte sig_index = tester.DefineSignature(tester.sigs.i_i());
-  tester.DefineTable(ValueType::Ref(sig_index, kNullable), 1, 1);
-  byte func_index = tester.DefineFunction(
-      tester.sigs.i_i(), {},
-      {WASM_TABLE_SET(0, WASM_I32V(0), WASM_REF_NULL(sig_index)),
-       WASM_CALL_INDIRECT(sig_index, WASM_I32V(0), WASM_LOCAL_GET(0)),
-       kExprEnd});
+
+  byte super_struct = tester.DefineStruct({F(kWasmI32, false)});
+  byte sub_struct =
+      tester.DefineStruct({F(kWasmI32, false), F(kWasmI32, true)});
+  FunctionSig* super_sig =
+      FunctionSig::Build(tester.zone(), {kWasmI32}, {optref(sub_struct)});
+  byte super_sig_index = tester.DefineSignature(super_sig);
+  FunctionSig* sub_sig =
+      FunctionSig::Build(tester.zone(), {kWasmI32}, {optref(super_struct)});
+  byte sub_sig_index = tester.DefineSignature(sub_sig);
+
+  tester.DefineTable(optref(super_sig_index), 10, 10);
+
+  byte super_func = tester.DefineFunction(
+      super_sig, {},
+      {WASM_I32_ADD(WASM_STRUCT_GET(sub_struct, 0, WASM_LOCAL_GET(0)),
+                    WASM_STRUCT_GET(sub_struct, 1, WASM_LOCAL_GET(0))),
+       WASM_END});
+
+  byte sub_func = tester.DefineFunction(
+      sub_sig, {},
+      {WASM_STRUCT_GET(super_struct, 0, WASM_LOCAL_GET(0)), WASM_END});
+
+  byte setup_func = tester.DefineFunction(
+      tester.sigs.i_v(), {},
+      {WASM_TABLE_SET(0, WASM_I32V(0), WASM_REF_NULL(super_sig_index)),
+       WASM_TABLE_SET(0, WASM_I32V(1), WASM_REF_FUNC(super_func)),
+       WASM_TABLE_SET(0, WASM_I32V(2), WASM_REF_FUNC(sub_func)), WASM_I32V(0),
+       WASM_END});
+
+  byte super_struct_producer = tester.DefineFunction(
+      FunctionSig::Build(tester.zone(), {ref(super_struct)}, {}), {},
+      {WASM_STRUCT_NEW_WITH_RTT(super_struct, WASM_I32V(-5),
+                                WASM_RTT_CANON(super_struct)),
+       WASM_END});
+  byte sub_struct_producer = tester.DefineFunction(
+      FunctionSig::Build(tester.zone(), {ref(sub_struct)}, {}), {},
+      {WASM_STRUCT_NEW_WITH_RTT(sub_struct, WASM_I32V(7), WASM_I32V(11),
+                                WASM_RTT_CANON(sub_struct)),
+       WASM_END});
+
+  // Calling a null entry should trap.
+  byte call_null = tester.DefineFunction(
+      tester.sigs.i_v(), {},
+      {WASM_CALL_INDIRECT(super_sig_index,
+                          WASM_CALL_FUNCTION0(sub_struct_producer),
+                          WASM_I32V(0)),
+       WASM_END});
+  // Calling with a signature identical to the type of the table should work,
+  // provided the entry has the same signature.
+  byte call_same_type = tester.DefineFunction(
+      tester.sigs.i_v(), {},
+      {WASM_CALL_INDIRECT(super_sig_index,
+                          WASM_CALL_FUNCTION0(sub_struct_producer),
+                          WASM_I32V(1)),
+       WASM_END});
+  // Calling with a signature that is a subtype of the type of the table should
+  // work, provided the entry has the same signature.
+  byte call_subtype = tester.DefineFunction(
+      tester.sigs.i_v(), {},
+      {WASM_CALL_INDIRECT(sub_sig_index,
+                          WASM_CALL_FUNCTION0(super_struct_producer),
+                          WASM_I32V(2)),
+       WASM_END});
+  // Calling with a signature that is mismatched to that of the entry should
+  // trap.
+  byte call_type_mismatch = tester.DefineFunction(
+      tester.sigs.i_v(), {},
+      {WASM_CALL_INDIRECT(super_sig_index,
+                          WASM_CALL_FUNCTION0(sub_struct_producer),
+                          WASM_I32V(2)),
+       WASM_END});
+  // Getting a table element and then calling it with call_ref should work.
+  byte table_get_and_call_ref = tester.DefineFunction(
+      tester.sigs.i_v(), {},
+      {WASM_CALL_REF(WASM_TABLE_GET(0, WASM_I32V(2)),
+                     WASM_CALL_FUNCTION0(sub_struct_producer)),
+       WASM_END});
+
+  // Only here so these functions count as "declared".
+  tester.AddGlobal(optref(super_sig_index), false,
+                   WasmInitExpr::RefFuncConst(super_func));
+  tester.AddGlobal(optref(sub_sig_index), false,
+                   WasmInitExpr::RefFuncConst(sub_func));
 
   tester.CompileModule();
 
-  tester.CheckHasThrown(func_index, 42);
+  tester.CheckResult(setup_func, 0);
+  tester.CheckHasThrown(call_null);
+  tester.CheckResult(call_same_type, 18);
+  tester.CheckResult(call_subtype, -5);
+  tester.CheckHasThrown(call_type_mismatch);
+  tester.CheckResult(table_get_and_call_ref, 7);
 }
 
 WASM_COMPILED_EXEC_TEST(JsAccess) {

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

@@ -831,7 +831,7 @@ let kTrapMsgs = [
   'remainder by zero',
   'float unrepresentable in integer range',
   'table index is out of bounds',
-  'function signature mismatch',
+  'null function or function signature mismatch',
   'operation does not support unaligned accesses',
   'data segment has been dropped',
   'element segment has been dropped',

test/unittests/wasm/function-body-decoder-unittest.cc

@@ -1914,6 +1914,50 @@ TEST_F(FunctionBodyDecoderTest, IndirectCallsWithMismatchedSigs2) {
       "call_indirect: immediate table #1 is not of a function type");
 }
 
+TEST_F(FunctionBodyDecoderTest, TablesWithFunctionSubtyping) {
+  WASM_FEATURE_SCOPE(reftypes);
+  WASM_FEATURE_SCOPE(typed_funcref);
+  WASM_FEATURE_SCOPE(gc);
+  EXPERIMENTAL_FLAG_SCOPE(gc);
+
+  byte empty_struct = builder.AddStruct({});
+  byte super_struct = builder.AddStruct({F(kWasmI32, false)});
+  byte sub_struct = builder.AddStruct({F(kWasmI32, false), F(kWasmF64, false)});
+
+  byte table_type = builder.AddSignature(
+      FunctionSig::Build(zone(), {ValueType::Ref(super_struct, kNullable)},
+                         {ValueType::Ref(sub_struct, kNullable)}));
+  byte table_supertype = builder.AddSignature(
+      FunctionSig::Build(zone(), {ValueType::Ref(empty_struct, kNullable)},
+                         {ValueType::Ref(sub_struct, kNullable)}));
+  auto function_sig =
+      FunctionSig::Build(zone(), {ValueType::Ref(sub_struct, kNullable)},
+                         {ValueType::Ref(super_struct, kNullable)});
+  byte function_type = builder.AddSignature(function_sig);
+
+  byte function = builder.AddFunction(function_sig);
+
+  byte table = builder.InitializeTable(ValueType::Ref(table_type, kNullable));
+
+  // We can call-indirect from a typed function table with an immediate type
+  // that is a subtype of the table type.
+  ExpectValidates(
+      FunctionSig::Build(zone(), {ValueType::Ref(sub_struct, kNullable)}, {}),
+      {WASM_CALL_INDIRECT_TABLE(
+          table, function_type,
+          WASM_STRUCT_NEW_DEFAULT(super_struct, WASM_RTT_CANON(super_struct)),
+          WASM_ZERO)});
+
+  // table.set's subtyping works as expected.
+  ExpectValidates(sigs.v_i(), {WASM_TABLE_SET(0, WASM_LOCAL_GET(0),
+                                              WASM_REF_FUNC(function))});
+  // table.get's subtyping works as expected.
+  ExpectValidates(
+      FunctionSig::Build(zone(), {ValueType::Ref(table_supertype, kNullable)},
+                         {kWasmI32}),
+      {WASM_TABLE_GET(0, WASM_LOCAL_GET(0))});
+}
+
 TEST_F(FunctionBodyDecoderTest, IndirectCallsWithoutTableCrash) {
   const FunctionSig* sig = sigs.i_i();