From 48c3551179299151b044b2c161566465497c0407 Mon Sep 17 00:00:00 2001
From: Jakob Kummerow <[email protected]>
Date: Tue, 23 Sep 2025 15:39:41 +0200
Subject: [PATCH] [wasm-custom-desc] Fix subtyping

This updates the restrictions on subtyping of descriptors, in
anticipation of upcoming spec changes:
- descriptors and described types must have matching subtyping
- ref.func for imported functions returns inexact types (for
  now; long-term solution TBD)

And it fixes our implementation:
- `ProcessBranchOnTarget` erroneously still thought it could
  compute reachability based on static types when Custom
  Descriptors are in play
- `JSToWasmObject` was missing support for exact types
- `ref.get_desc` must not return an exact type when the actual
  type on the stack is exact, but a non-trivial subtype of the
  instruction's type immediate.

Bug: 403372470
Fixed: 446113731, 446113732, 446122633, 446124892, 446124893
Change-Id: Ic79ab08d906a2e21e66b76e9d96eebb4ebb7a8e5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6973586
Commit-Queue: Jakob Kummerow <[email protected]>
Reviewed-by: Matthias Liedtke <[email protected]>
Cr-Commit-Position: refs/heads/main@{#102697}
---

diff --git a/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.cc b/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.cc
index c8d6693..f6905e6 100644
--- a/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.cc
+++ b/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.cc
@@ -445,7 +445,10 @@
       } else {
         DCHECK_EQ(branch.if_false, &target);
         if (wasm::IsSubtypeOf(GetResolvedType(check.object()), check.config.to,
-                              module_)) {
+                              module_) &&
+            // When checking for a particular custom descriptor, static types
+            // cannot predict the outcome.
+            !(IsCastToCustomDescriptor(module_, check.config))) {
           // The type check always succeeds, the target is impossible to be
           // reached.
           DCHECK_EQ(target.PredecessorCount(), 1);
diff --git a/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.h b/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.h
index a571fe4..2ba8fbe 100644
--- a/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.h
+++ b/src/compiler/turboshaft/wasm-gc-typed-optimization-reducer.h
@@ -18,6 +18,13 @@
 
 namespace v8::internal::compiler::turboshaft {
 
+inline bool IsCastToCustomDescriptor(const wasm::WasmModule* module,
+                                     WasmTypeCheckConfig config) {
+  return config.to.has_index() &&
+         module->type(config.to.ref_index()).has_descriptor() &&
+         config.exactness == compiler::kExactMatchOnly;
+}
+
 // The WasmGCTypedOptimizationReducer infers type information based on the input
 // graph and reduces type checks and casts based on that information.
 //
@@ -179,7 +186,7 @@
       bool to_nullable = cast_op.config.to.is_nullable();
       if (wasm::IsHeapSubtypeOf(type.heap_type(), cast_op.config.to.heap_type(),
                                 module_) &&
-          !IsCastToCustomDescriptor(cast_op.config)) {
+          !IsCastToCustomDescriptor(module_, cast_op.config)) {
         if (to_nullable || type.is_non_nullable()) {
           // The inferred type is already as specific as the cast target, the
           // cast is guaranteed to always succeed and can therefore be removed.
@@ -247,7 +254,7 @@
                                 type_check.config.to.heap_type(), module_) &&
           // When checking for a particular custom descriptor, static types
           // cannot guarantee success.
-          !(IsCastToCustomDescriptor(type_check.config))) {
+          !(IsCastToCustomDescriptor(module_, type_check.config))) {
         if (to_nullable || type.is_non_nullable()) {
           // The inferred type is guaranteed to be a subtype of the checked
           // type.
@@ -435,12 +442,6 @@
   }
 
  private:
-  bool IsCastToCustomDescriptor(WasmTypeCheckConfig config) {
-    return config.to.has_index() &&
-           module_->type(config.to.ref_index()).has_descriptor() &&
-           config.exactness == compiler::kExactMatchOnly;
-  }
-
   Graph& graph_ = __ modifiable_input_graph();
   const wasm::WasmModule* module_ = __ data() -> wasm_module();
   WasmGCTypeAnalyzer analyzer_{__ data(), graph_, __ phase_zone()};
diff --git a/src/wasm/canonical-types.cc b/src/wasm/canonical-types.cc
index 359ca5e..50120e0 100644
--- a/src/wasm/canonical-types.cc
+++ b/src/wasm/canonical-types.cc
@@ -243,15 +243,19 @@
 }
 
 bool TypeCanonicalizer::IsCanonicalSubtype(CanonicalTypeIndex sub_index,
-                                           CanonicalTypeIndex super_index) {
+                                           CanonicalValueType super_type) {
+  DCHECK(super_type.has_index());
   // Fast path without synchronization:
-  if (sub_index == super_index) return true;
+  if (sub_index == super_type.ref_index()) return true;
+  // If the supertype is exact, then only the equality case above is
+  // successful.
+  if (super_type.is_exact()) return false;
 
   // Multiple threads could try to register and access recursive groups
   // concurrently.
   // TODO(manoskouk): Investigate if we can improve this synchronization.
   base::MutexGuard mutex_guard(&mutex_);
-  return IsCanonicalSubtype_Locked(sub_index, super_index);
+  return IsCanonicalSubtype_Locked(sub_index, super_type.ref_index());
 }
 bool TypeCanonicalizer::IsCanonicalSubtype_Locked(
     CanonicalTypeIndex sub_index, CanonicalTypeIndex super_index) const {
@@ -266,16 +270,6 @@
   return false;
 }
 
-bool TypeCanonicalizer::IsCanonicalSubtype(ModuleTypeIndex sub_index,
-                                           ModuleTypeIndex super_index,
-                                           const WasmModule* sub_module,
-                                           const WasmModule* super_module) {
-  CanonicalTypeIndex canonical_super =
-      super_module->canonical_type_id(super_index);
-  CanonicalTypeIndex canonical_sub = sub_module->canonical_type_id(sub_index);
-  return IsCanonicalSubtype(canonical_sub, canonical_super);
-}
-
 bool TypeCanonicalizer::IsHeapSubtype(CanonicalTypeIndex sub,
                                       CanonicalTypeIndex super) const {
   DCHECK_NE(sub, super);
diff --git a/src/wasm/canonical-types.h b/src/wasm/canonical-types.h
index b992652..5301224 100644
--- a/src/wasm/canonical-types.h
+++ b/src/wasm/canonical-types.h
@@ -85,17 +85,12 @@
   V8_EXPORT_PRIVATE const CanonicalArrayType* LookupArray(
       CanonicalTypeIndex index) const;
 
-  // Returns if {canonical_sub_index} is a canonical subtype of
-  // {canonical_super_index}.
+  // Returns if {sub_index} is a canonical subtype of {super_type}, which must
+  // be an indexed type. Interprets {sub_index} as (exact sub_index), which is
+  // appropriate for checking the actual type of a thing against a required
+  // type.
   V8_EXPORT_PRIVATE bool IsCanonicalSubtype(CanonicalTypeIndex sub_index,
-                                            CanonicalTypeIndex super_index);
-
-  // Returns if the type at {sub_index} in {sub_module} is a subtype of the
-  // type at {super_index} in {super_module} after canonicalization.
-  V8_EXPORT_PRIVATE bool IsCanonicalSubtype(ModuleTypeIndex sub_index,
-                                            ModuleTypeIndex super_index,
-                                            const WasmModule* sub_module,
-                                            const WasmModule* super_module);
+                                            CanonicalValueType super_type);
 
   // Deletes recursive groups. Used by fuzzers to avoid accumulating memory, and
   // used by specific tests e.g. for serialization / deserialization.
diff --git a/src/wasm/constant-expression-interface.cc b/src/wasm/constant-expression-interface.cc
index 0bd4c138..757b4a1 100644
--- a/src/wasm/constant-expression-interface.cc
+++ b/src/wasm/constant-expression-interface.cc
@@ -124,8 +124,13 @@
   bool function_is_shared = module_->type(sig_index).is_shared;
   CanonicalValueType type =
       CanonicalValueType::Ref(module_->canonical_type_id(sig_index),
-                              function_is_shared, RefTypeKind::kFunction)
-          .AsExactIfEnabled(decoder->enabled_);
+                              function_is_shared, RefTypeKind::kFunction);
+  // Imported functions can be subtypes of their static import type,
+  // for non-imported functions we can return an exact type.
+  if (decoder->enabled_.has_custom_descriptors() &&
+      function_index >= module_->num_imported_functions) {
+    type = type.AsExact();
+  }
   DirectHandle<WasmFuncRef> func_ref =
       WasmTrustedInstanceData::GetOrCreateFuncRef(
           isolate_,
diff --git a/src/wasm/function-body-decoder-impl.h b/src/wasm/function-body-decoder-impl.h
index 6aa071e..5a6a2ec 100644
--- a/src/wasm/function-body-decoder-impl.h
+++ b/src/wasm/function-body-decoder-impl.h
@@ -4243,9 +4243,17 @@
     if (!this->ValidateFunction(this->pc_ + 1, imm)) return 0;
     ModuleTypeIndex index = this->module_->functions[imm.index].sig_index;
     const TypeDefinition& type_def = this->module_->type(index);
-    Value* value =
-        Push(ValueType::Ref(index, type_def.is_shared, RefTypeKind::kFunction)
-                 .AsExactIfEnabled(this->enabled_));
+    ValueType result_type =
+        ValueType::Ref(index, type_def.is_shared, RefTypeKind::kFunction);
+    // For imported functions, we must return an inexact type, because
+    // importing checks subtyping, i.e. for function types f1 <: f2, it is

diff --git a/test/mjsunit/wasm/custom-descriptors-validity.js b/test/mjsunit/wasm/custom-descriptors-validity.js
index 9b3118f2..204d0a5 100644
--- a/test/mjsunit/wasm/custom-descriptors-validity.js
+++ b/test/mjsunit/wasm/custom-descriptors-validity.js
@@ -119,7 +119,7 @@
   builder.addStruct({describes: 2, supertype: 1});   // 3
 });
 
-CheckValid((builder) => {
+CheckInvalid(/type 4 has invalid explicit supertype 2/, (builder) => {
   builder.addStruct({final: false});  // 0
 }, (builder) => {
   builder.addStruct({descriptor: 2});  // 1