Repository: bytecodealliance/wasm-tools
Pull request #2172 (2025-04-30 21:54:04)

wasmparser(CM+GC): Validate components' core function linking via type canonicalization
Instead of structurally checking parameters and return types, rely on type canonicalization to dedupe types so that we can simply compare ids. This gets instantiation with the "same" function type defined in a different rec group to properly fail to link.

This does not yet handle function subtyping. That will happen in follow up commits.


Patch:
diff --git a/crates/wasmparser/src/readers/core/types.rs b/crates/wasmparser/src/readers/core/types.rs
index 9061ea52..39a62ab2 100644
--- a/crates/wasmparser/src/readers/core/types.rs
+++ b/crates/wasmparser/src/readers/core/types.rs
@@ -548,16 +548,6 @@ impl SubType {
     }
 }
 
-/// Represents a composite type in a WebAssembly module.
-#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
-pub struct CompositeType {
-    /// The type defined inside the composite type.
-    pub inner: CompositeInnerType,
-    /// Is the composite type shared? This is part of the
-    /// shared-everything-threads proposal.
-    pub shared: bool,
-}
-
 /// A [`CompositeType`] can contain one of these types.
 #[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
 pub enum CompositeInnerType {
@@ -571,18 +561,33 @@ pub enum CompositeInnerType {
     Cont(ContType),
 }
 
+impl fmt::Display for CompositeInnerType {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            CompositeInnerType::Func(ty) => write!(f, "{ty}"),
+            CompositeInnerType::Array(ty) => write!(f, "{ty}"),
+            CompositeInnerType::Struct(ty) => write!(f, "{ty}"),
+            CompositeInnerType::Cont(ty) => write!(f, "{ty}"),
+        }
+    }
+}
+
+/// Represents a composite type in a WebAssembly module.
+#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
+pub struct CompositeType {
+    /// The type defined inside the composite type.
+    pub inner: CompositeInnerType,
+    /// Is the composite type shared? This is part of the
+    /// shared-everything-threads proposal.
+    pub shared: bool,
+}
+
 impl fmt::Display for CompositeType {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        use CompositeInnerType::*;
         if self.shared {
             write!(f, "(shared ")?;
         }
-        match self.inner {
-            Array(_) => write!(f, "(array ...)"),
-            Func(_) => write!(f, "(func ...)"),
-            Struct(_) => write!(f, "(struct ...)"),
-            Cont(_) => write!(f, "(cont ...)"),
-        }?;
+        write!(f, "{}", self.inner)?;
         if self.shared {
             write!(f, ")")?;
         }
@@ -642,6 +647,28 @@ impl fmt::Debug for FuncType {
     }
 }
 
+impl fmt::Display for FuncType {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "(func")?;
+        if self.params().len() > 0 {
+            write!(f, " (param")?;
+            for p in self.params() {
+                write!(f, " {p}")?;
+            }
+            write!(f, ")")?;
+        }
+        if self.results().len() > 0 {
+            write!(f, " (result")?;
+            for p in self.results() {
+                write!(f, " {p}")?;
+            }
+            write!(f, ")")?;
+        }
+        write!(f, ")")?;
+        Ok(())
+    }
+}
+
 impl FuncType {
     /// Creates a new [`FuncType`] from the given `params` and `results`.
     pub fn new<P, R>(params: P, results: R) -> Self
@@ -698,35 +725,18 @@ impl FuncType {
     pub(crate) fn results_mut(&mut self) -> &mut [ValType] {
         &mut self.params_results[self.len_params..]
     }
-
-    #[cfg(all(feature = "validate", feature = "component-model"))]
-    pub(crate) fn desc(&self) -> String {
-        use core::fmt::Write;
-
-        let mut s = String::new();
-        s.push_str("[");
-        for (i, param) in self.params().iter().enumerate() {
-            if i > 0 {
-                s.push_str(" ");
-            }
-            write!(s, "{param}").unwrap();
-        }
-        s.push_str("] -> [");
-        for (i, result) in self.results().iter().enumerate() {
-            if i > 0 {
-                s.push_str(" ");
-            }
-            write!(s, "{result}").unwrap();
-        }
-        s.push_str("]");
-        s
-    }
 }
 
 /// Represents a type of an array in a WebAssembly module.
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
 pub struct ArrayType(pub FieldType);
 
+impl fmt::Display for ArrayType {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "(array {})", self.0)
+    }
+}
+
 /// Represents a field type of an array or a struct.
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
 pub struct FieldType {
@@ -736,6 +746,16 @@ pub struct FieldType {
     pub mutable: bool,
 }
 
+impl fmt::Display for FieldType {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if self.mutable {
+            write!(f, "(mut {})", self.element_type)
+        } else {
+            fmt::Display::fmt(&self.element_type, f)
+        }
+    }
+}
+
 impl FieldType {
     /// Maps any `UnpackedIndex` via the specified closure.
     #[cfg(feature = "validate")]
@@ -798,10 +818,27 @@ pub struct StructType {
     pub fields: Box<[FieldType]>,
 }
 
+impl fmt::Display for StructType {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "(struct")?;
+        for field in self.fields.iter() {
+            write!(f, " {field}")?;
+        }
+        write!(f, ")")?;
+        Ok(())
+    }
+}
+
 /// Represents a type of a continuation in a WebAssembly module.
 #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
 pub struct ContType(pub PackedIndex);
 
+impl fmt::Display for ContType {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "(cont {})", self.0)
+    }
+}
+
 impl ContType {
     /// Maps any `UnpackedIndex` via the specified closure.
     #[cfg(feature = "validate")]
diff --git a/crates/wasmparser/src/validator/component_types.rs b/crates/wasmparser/src/validator/component_types.rs
index 1d7f7699..4e78cd1a 100644
--- a/crates/wasmparser/src/validator/component_types.rs
+++ b/crates/wasmparser/src/validator/component_types.rs
@@ -3030,9 +3030,7 @@ impl<'a> SubtypeCx<'a> {
 
     pub(crate) fn entity_type(&self, a: &EntityType, b: &EntityType, offset: usize) -> Result<()> {
         match (a, b) {
-            (EntityType::Func(a), EntityType::Func(b)) => {
-                self.core_func_type(self.a[*a].unwrap_func(), self.b[*b].unwrap_func(), offset)
-            }
+            (EntityType::Func(a), EntityType::Func(b)) => self.core_func_type(*a, *b, offset),
             (EntityType::Func(_), b) => bail!(offset, "expected {}, found func", b.desc()),
             (EntityType::Table(a), EntityType::Table(b)) => Self::table_type(a, b, offset),
             (EntityType::Table(_), b) => bail!(offset, "expected {}, found table", b.desc()),
@@ -3054,9 +3052,7 @@ impl<'a> SubtypeCx<'a> {
                 }
             }
             (EntityType::Global(_), b) => bail!(offset, "expected {}, found global", b.desc()),
-            (EntityType::Tag(a), EntityType::Tag(b)) => {
-                self.core_func_type(self.a[*a].unwrap_func(), self.b[*b].unwrap_func(), offset)
-            }
+            (EntityType::Tag(a), EntityType::Tag(b)) => self.core_func_type(*a, *b, offset),
             (EntityType::Tag(_), b) => bail!(offset, "expected {}, found tag", b.desc()),
         }
     }
@@ -3094,16 +3090,20 @@ impl<'a> SubtypeCx<'a> {
         }
     }
 
-    fn core_func_type(&self, a: &FuncType, b: &FuncType, offset: usize) -> Result<()> {
+    fn core_func_type(&self, a: CoreTypeId, b: CoreTypeId, offset: usize) -> Result<()> {
+        debug_assert!(self.a.get(a).is_some());
+        debug_assert!(self.b.get(b).is_some());
         if a == b {
+            debug_assert!(self.a.get(b).is_some());
+            debug_assert!(self.b.get(a).is_some());
             Ok(())
         } else {
             bail!(
                 offset,
                 "expected: {}\n\
                  found:    {}",
-                b.desc(),
-                a.desc(),
+                self.b[b],
+                self.a[a],
             )
         }
     }
@@ -3353,6 +3353,21 @@ impl<'a> SubtypeArena<'a> {
             list: TypeList::default(),
         }
     }
+
+    fn get<T>(&self, id: T) -> Option<&T::Data>
+    where
+        T: TypeIdentifier,
+    {
+        let index = id.index();
+        if index < T::list(self.types).len() {
+            self.types.get(id)
+        } else {
+            let temp_index = index - T::list(self.types).len();
+            let temp_index = u32::try_from(temp_index).unwrap();
+            let temp_id = T::from_index(temp_index);
+            self.list.get(temp_id)
+        }
+    }
 }
 
 impl<T> Index<T> for SubtypeArena<'_>
@@ -3362,15 +3377,7 @@ where
     type Output = T::Data;
 
     fn index(&self, id: T) -> &T::Data {
-        let index = id.index();
-        if index < T::list(self.types).len() {
-            &self.types[id]
-        } else {
-            let temp_index = index - T::list(self.types).len();
-            let temp_index = u32::try_from(temp_index).unwrap();
-            let temp_id = T::from_index(temp_index);
-            &self.list[temp_id]
-        }
+        self.get(id).unwrap()
     }
 }
 


Test patch:
diff --git a/tests/cli/component-model-async/task-builtins.wast b/tests/cli/component-model-async/task-builtins.wast
index 9d364d5b..1e0d3c5d 100644
--- a/tests/cli/component-model-async/task-builtins.wast
+++ b/tests/cli/component-model-async/task-builtins.wast
@@ -265,14 +265,14 @@
     (core func $f (canon context.get i32 0))
     (core instance $i (instantiate $m (with "" (instance (export "" (func $f))))))
   )
-  "found:    [] -> [i32]")
+  "found:    (func (result i32))")
 (assert_invalid
   (component
     (core module $m (import "" "" (func (param i32) (result i32))))
     (core func $f (canon context.set i32 0))
     (core instance $i (instantiate $m (with "" (instance (export "" (func $f))))))
   )
-  "found:    [i32] -> []")
+  "found:    (func (param i32))")
 (assert_invalid
   (component
     (core func (canon context.get i32 1)))
diff --git a/tests/cli/component-model-gc/core-type.wat b/tests/cli/component-model-gc/core-type.wat
index b5e84874..d3b7276d 100644
--- a/tests/cli/component-model-gc/core-type.wat
+++ b/tests/cli/component-model-gc/core-type.wat
@@ -39,3 +39,46 @@
   )
   "declared core type has `[I64]` result types, but actual lowering has `[I32]` result types"
 )
+
+(component
+  (import "f" (func $f (param "x" u32) (param "y" u32) (result u32)))
+
+  (core rec
+    (type (func))
+    (type $ty (func (param i32 i32) (result i32)))
+  )
+
+  (core func $f (canon lower (func $f) (core-type $ty)))
+
+  (core module $m
+    (rec
+      (type (func))
+      (type $ty (func (param i32 i32) (result i32)))
+    )
+    (import "a" "b" (func (type $ty)))
+  )
+
+  (core instance (instantiate $m (with "a" (instance (export "b" (func $f))))))
+)
+
+;; Satisfying an import with the "same" type but from the wrong rec group should
+;; fail.
+(assert_invalid
+  (component
+    (import "f" (func $f (param "x" u32) (param "y" u32) (result u32)))
+
+    (core type $ty (func (param i32 i32) (result i32)))
+    (core func $f (canon lower (func $f) (core-type $ty)))
+
+    (core module $m
+      (rec
+        (type (func))
+        (type $ty (func (param i32 i32) (result i32)))
+      )
+      (import "a" "b" (func (type $ty)))
+    )
+
+    (core instance (instantiate $m (with "a" (instance (export "b" (func $f))))))
+  )
+  "type mismatch for export `b` of module instantiation argument `a`"
+)
diff --git a/tests/cli/component-model/instantiate.wast b/tests/cli/component-model/instantiate.wast
index 64c68af2..35abcbc6 100644
--- a/tests/cli/component-model/instantiate.wast
+++ b/tests/cli/component-model/instantiate.wast
@@ -285,7 +285,7 @@
     (core instance $i (instantiate $m2))
     (core instance (instantiate $m1 (with "" (instance $i))))
   )
-  "expected: [] -> []")
+  "expected: (func)")
 (assert_invalid
   (component
     (import "m1" (core module $m1 (import "" "" (func))))
@@ -293,7 +293,7 @@
     (core instance $i (instantiate $m2))
     (core instance (instantiate $m1 (with "" (instance $i))))
   )
-  "expected: [] -> []")
+  "expected: (func)")
 (assert_invalid
   (component
     (import "m1" (core module $m1 (import "" "" (global i32))))
diff --git a/tests/cli/gc/invalid.wast b/tests/cli/gc/invalid.wast
index a69b5d4b..f48ce959 100644
--- a/tests/cli/gc/invalid.wast
+++ b/tests/cli/gc/invalid.wast
@@ -36,7 +36,7 @@
   (module
     (type $t (func))
     (func struct.new $t drop))
-  "expected struct type at index 0, found (func ...)")
+  "expected struct type at index 0, found (func)")
 
 (assert_invalid
   (module
@@ -53,7 +53,7 @@
   (module
     (type $t (struct))
     (func array.new $t drop))
-  "expected array type at index 0, found (struct ...)")
+  "expected array type at index 0, found (struct)")
 
 (assert_invalid
   (module
@@ -70,7 +70,7 @@
   (module
     (type $t (struct))
     (func block (type $t) end))
-  "expected func type at index 0, found (struct ...)")
+  "expected func type at index 0, found (struct)")
 
 (assert_invalid
   (module
diff --git a/tests/cli/shared-everything-threads/builtins.wast b/tests/cli/shared-everything-threads/builtins.wast
index 4545f206..20128ac6 100644
--- a/tests/cli/shared-everything-threads/builtins.wast
+++ b/tests/cli/shared-everything-threads/builtins.wast
@@ -24,10 +24,13 @@
   (core func $parallelism (canon thread.available_parallelism))
 
   (core module $m
-    (type $st (shared (func (param $context i32))))
-    (import "" "spawn_ref" (func (param (ref null $st)) (param i32) (result i32)))
-    (import "" "spawn_indirect" (func (param i32) (param i32) (result i32)))
-    (import "" "parallelism" (func (result i32)))
+    (type $spawned_func_ty (shared (func (param $context i32))))
+    (type $spawn_ref_ty (shared (func (param (ref null $spawned_func_ty)) (param i32) (result i32))))
+    (type $spawn_indirect_ty (shared (func (param i32) (param i32) (result i32))))
+    (type $parallelism_ty (shared (func (result i32))))
+    (import "" "spawn_ref" (func (type $spawn_ref_ty)))
+    (import "" "spawn_indirect" (func (type $spawn_indirect_ty)))
+    (import "" "parallelism" (func (type $parallelism_ty)))
   )
 
   (core instance (instantiate $m
diff --git a/tests/snapshots/cli/component-model-async/task-builtins.wast.json b/tests/snapshots/cli/component-model-async/task-builtins.wast.json
index 9d38e0ce..442ca64b 100644
--- a/tests/snapshots/cli/component-model-async/task-builtins.wast.json
+++ b/tests/snapshots/cli/component-model-async/task-builtins.wast.json
@@ -175,14 +175,14 @@
       "line": 263,
       "filename": "task-builtins.26.wasm",
       "module_type": "binary",
-      "text": "found:    [] -> [i32]"
+      "text": "found:    (func (result i32))"
     },
     {
       "type": "assert_invalid",
       "line": 270,
       "filename": "task-builtins.27.wasm",
       "module_type": "binary",
-      "text": "found:    [i32] -> []"
+      "text": "found:    (func (param i32))"
     },
     {
       "type": "assert_invalid",
diff --git a/tests/snapshots/cli/component-model-gc/core-type.wat.json b/tests/snapshots/cli/component-model-gc/core-type.wat.json
index b47108bb..42068df4 100644
--- a/tests/snapshots/cli/component-model-gc/core-type.wat.json
+++ b/tests/snapshots/cli/component-model-gc/core-type.wat.json
@@ -27,6 +27,19 @@
       "filename": "core-type.3.wasm",
       "module_type": "binary",
       "text": "declared core type has `[I64]` result types, but actual lowering has `[I32]` result types"
+    },
+    {
+      "type": "module",
+      "line": 43,
+      "filename": "core-type.4.wasm",
+      "module_type": "binary"
+    },
+    {
+      "type": "assert_invalid",
+      "line": 67,
+      "filename": "core-type.5.wasm",
+      "module_type": "binary",
+      "text": "type mismatch for export `b` of module instantiation argument `a`"
     }
   ]
 }
\ No newline at end of file
diff --git a/tests/snapshots/cli/component-model-gc/core-type.wat/4.print b/tests/snapshots/cli/component-model-gc/core-type.wat/4.print
new file mode 100644
index 00000000..a2de79d0
--- /dev/null
+++ b/tests/snapshots/cli/component-model-gc/core-type.wat/4.print
@@ -0,0 +1,23 @@
+(component
+  (type (;0;) (func (param "x" u32) (param "y" u32) (result u32)))
+  (import "f" (func $f (;0;) (type 0)))
+  (core rec
+    (type (;0;) (func))
+    (type $ty (;1;) (func (param i32 i32) (result i32)))
+  )
+  (core func $f (;0;) (canon lower (func $f) (core-type $ty)))
+  (core module $m (;0;)
+    (rec
+      (type (;0;) (func))
+      (type $ty (;1;) (func (param i32 i32) (result i32)))
+    )
+    (import "a" "b" (func (;0;) (type $ty)))
+  )
+  (core instance (;0;)
+    (export "b" (func $f))
+  )
+  (core instance (;1;) (instantiate $m
+      (with "a" (instance 0))
+    )
+  )
+)
diff --git a/tests/snapshots/cli/component-model/instantiate.wast.json b/tests/snapshots/cli/component-model/instantiate.wast.json
index f832eecd..63d31693 100644
--- a/tests/snapshots/cli/component-model/instantiate.wast.json
+++ b/tests/snapshots/cli/component-model/instantiate.wast.json
@@ -173,14 +173,14 @@
       "line": 282,
       "filename": "instantiate.26.wasm",
       "module_type": "binary",
-      "text": "expected: [] -> []"
+      "text": "expected: (func)"
     },
     {
       "type": "assert_invalid",
       "line": 290,
       "filename": "instantiate.27.wasm",
       "module_type": "binary",
-      "text": "expected: [] -> []"
+      "text": "expected: (func)"
     },
     {
       "type": "assert_invalid",
diff --git a/tests/snapshots/cli/gc/invalid.wast.json b/tests/snapshots/cli/gc/invalid.wast.json
index ed52d571..a1870d13 100644
--- a/tests/snapshots/cli/gc/invalid.wast.json
+++ b/tests/snapshots/cli/gc/invalid.wast.json
@@ -20,7 +20,7 @@
       "line": 36,
       "filename": "invalid.2.wasm",
       "module_type": "binary",
-      "text": "expected struct type at index 0, found (func ...)"
+      "text": "expected struct type at index 0, found (func)"
     },
     {
       "type": "assert_invalid",
@@ -41,7 +41,7 @@
       "line": 53,
       "filename": "invalid.5.wasm",
       "module_type": "binary",
-      "text": "expected array type at index 0, found (struct ...)"
+      "text": "expected array type at index 0, found (struct)"
     },
     {
       "type": "assert_invalid",
@@ -62,7 +62,7 @@
       "line": 70,
       "filename": "invalid.8.wasm",
       "module_type": "binary",
-      "text": "expected func type at index 0, found (struct ...)"
+      "text": "expected func type at index 0, found (struct)"
     },
     {
       "type": "assert_invalid",
diff --git a/tests/snapshots/cli/shared-everything-threads/builtins.wast.json b/tests/snapshots/cli/shared-everything-threads/builtins.wast.json
index a522f753..8ea666b2 100644
--- a/tests/snapshots/cli/shared-everything-threads/builtins.wast.json
+++ b/tests/snapshots/cli/shared-everything-threads/builtins.wast.json
@@ -15,35 +15,35 @@
     },
     {
       "type": "assert_invalid",
-      "line": 43,
+      "line": 46,
       "filename": "builtins.2.wasm",
       "module_type": "binary",
       "text": "spawn type must be shared"
     },
     {
       "type": "assert_invalid",
-      "line": 51,
+      "line": 54,
       "filename": "builtins.3.wasm",
       "module_type": "binary",
       "text": "unknown table 0: table index out of bounds"
     },
     {
       "type": "assert_invalid",
-      "line": 61,
+      "line": 64,
       "filename": "builtins.4.wasm",
       "module_type": "binary",
       "text": "spawn function must take a single `i32` argument (currently)"
     },
     {
       "type": "assert_invalid",
-      "line": 69,
+      "line": 72,
       "filename": "builtins.5.wasm",
       "module_type": "binary",
       "text": "spawn function must take a single `i32` argument (currently)"
     },
     {
       "type": "assert_invalid",
-      "line": 79,
+      "line": 82,
       "filename": "builtins.6.wasm",
       "module_type": "binary",
       "text": "mismatch in the shared flag for tables"
diff --git a/tests/snapshots/cli/shared-everything-threads/builtins.wast/1.print b/tests/snapshots/cli/shared-everything-threads/builtins.wast/1.print
index 1174f32a..628eba34 100644
--- a/tests/snapshots/cli/shared-everything-threads/builtins.wast/1.print
+++ b/tests/snapshots/cli/shared-everything-threads/builtins.wast/1.print
@@ -10,13 +10,13 @@
   (core func $spawn_indirect (;1;) (canon thread.spawn_indirect $start (table 0)))
   (core func $parallelism (;2;) (canon thread.available_parallelism))
   (core module $m (;1;)
-    (type $st (;0;) (shared (func (param i32))))
-    (type (;1;) (func (param (ref null $st) i32) (result i32)))
-    (type (;2;) (func (param i32 i32) (result i32)))
-    (type (;3;) (func (result i32)))
-    (import "" "spawn_ref" (func (;0;) (type 1)))
-    (import "" "spawn_indirect" (func (;1;) (type 2)))
-    (import "" "parallelism" (func (;2;) (type 3)))
+    (type $spawned_func_ty (;0;) (shared (func (param i32))))
+    (type $spawn_ref_ty (;1;) (shared (func (param (ref null $spawned_func_ty) i32) (result i32))))
+    (type $spawn_indirect_ty (;2;) (shared (func (param i32 i32) (result i32))))
+    (type $parallelism_ty (;3;) (shared (func (result i32))))
+    (import "" "spawn_ref" (func (;0;) (type $spawn_ref_ty)))
+    (import "" "spawn_indirect" (func (;1;) (type $spawn_indirect_ty)))
+    (import "" "parallelism" (func (;2;) (type $parallelism_ty)))
   )
   (core instance (;1;)
     (export "spawn_ref" (func $spawn_ref))