Skip to content

Commit c0f3082

Browse files
authored
wasmparser(CM+GC): Validate components' core function linking via type canonicalization (#2172)
* wasmparser:(CM+GC): Validate components' core function linking via 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. * delete some unused snapshot files that got added by accident * Add debug asserts that type lists contain IDs * fix assertion * Move all `desc` methods over to `Display` impls
1 parent 7b08b93 commit c0f3082

File tree

14 files changed

+214
-88
lines changed

14 files changed

+214
-88
lines changed

crates/wasmparser/src/readers/core/types.rs

Lines changed: 77 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -548,16 +548,6 @@ impl SubType {
548548
}
549549
}
550550

551-
/// Represents a composite type in a WebAssembly module.
552-
#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
553-
pub struct CompositeType {
554-
/// The type defined inside the composite type.
555-
pub inner: CompositeInnerType,
556-
/// Is the composite type shared? This is part of the
557-
/// shared-everything-threads proposal.
558-
pub shared: bool,
559-
}
560-
561551
/// A [`CompositeType`] can contain one of these types.
562552
#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
563553
pub enum CompositeInnerType {
@@ -571,18 +561,33 @@ pub enum CompositeInnerType {
571561
Cont(ContType),
572562
}
573563

564+
impl fmt::Display for CompositeInnerType {
565+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
566+
match self {
567+
CompositeInnerType::Func(ty) => write!(f, "{ty}"),
568+
CompositeInnerType::Array(ty) => write!(f, "{ty}"),
569+
CompositeInnerType::Struct(ty) => write!(f, "{ty}"),
570+
CompositeInnerType::Cont(ty) => write!(f, "{ty}"),
571+
}
572+
}
573+
}
574+
575+
/// Represents a composite type in a WebAssembly module.
576+
#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
577+
pub struct CompositeType {
578+
/// The type defined inside the composite type.
579+
pub inner: CompositeInnerType,
580+
/// Is the composite type shared? This is part of the
581+
/// shared-everything-threads proposal.
582+
pub shared: bool,
583+
}
584+
574585
impl fmt::Display for CompositeType {
575586
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
576-
use CompositeInnerType::*;
577587
if self.shared {
578588
write!(f, "(shared ")?;
579589
}
580-
match self.inner {
581-
Array(_) => write!(f, "(array ...)"),
582-
Func(_) => write!(f, "(func ...)"),
583-
Struct(_) => write!(f, "(struct ...)"),
584-
Cont(_) => write!(f, "(cont ...)"),
585-
}?;
590+
write!(f, "{}", self.inner)?;
586591
if self.shared {
587592
write!(f, ")")?;
588593
}
@@ -642,6 +647,28 @@ impl fmt::Debug for FuncType {
642647
}
643648
}
644649

650+
impl fmt::Display for FuncType {
651+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
652+
write!(f, "(func")?;
653+
if self.params().len() > 0 {
654+
write!(f, " (param")?;
655+
for p in self.params() {
656+
write!(f, " {p}")?;
657+
}
658+
write!(f, ")")?;
659+
}
660+
if self.results().len() > 0 {
661+
write!(f, " (result")?;
662+
for p in self.results() {
663+
write!(f, " {p}")?;
664+
}
665+
write!(f, ")")?;
666+
}
667+
write!(f, ")")?;
668+
Ok(())
669+
}
670+
}
671+
645672
impl FuncType {
646673
/// Creates a new [`FuncType`] from the given `params` and `results`.
647674
pub fn new<P, R>(params: P, results: R) -> Self
@@ -698,35 +725,18 @@ impl FuncType {
698725
pub(crate) fn results_mut(&mut self) -> &mut [ValType] {
699726
&mut self.params_results[self.len_params..]
700727
}
701-
702-
#[cfg(all(feature = "validate", feature = "component-model"))]
703-
pub(crate) fn desc(&self) -> String {
704-
use core::fmt::Write;
705-
706-
let mut s = String::new();
707-
s.push_str("[");
708-
for (i, param) in self.params().iter().enumerate() {
709-
if i > 0 {
710-
s.push_str(" ");
711-
}
712-
write!(s, "{param}").unwrap();
713-
}
714-
s.push_str("] -> [");
715-
for (i, result) in self.results().iter().enumerate() {
716-
if i > 0 {
717-
s.push_str(" ");
718-
}
719-
write!(s, "{result}").unwrap();
720-
}
721-
s.push_str("]");
722-
s
723-
}
724728
}
725729

726730
/// Represents a type of an array in a WebAssembly module.
727731
#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
728732
pub struct ArrayType(pub FieldType);
729733

734+
impl fmt::Display for ArrayType {
735+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
736+
write!(f, "(array {})", self.0)
737+
}
738+
}
739+
730740
/// Represents a field type of an array or a struct.
731741
#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
732742
pub struct FieldType {
@@ -736,6 +746,16 @@ pub struct FieldType {
736746
pub mutable: bool,
737747
}
738748

749+
impl fmt::Display for FieldType {
750+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
751+
if self.mutable {
752+
write!(f, "(mut {})", self.element_type)
753+
} else {
754+
fmt::Display::fmt(&self.element_type, f)
755+
}
756+
}
757+
}
758+
739759
impl FieldType {
740760
/// Maps any `UnpackedIndex` via the specified closure.
741761
#[cfg(feature = "validate")]
@@ -798,10 +818,27 @@ pub struct StructType {
798818
pub fields: Box<[FieldType]>,
799819
}
800820

821+
impl fmt::Display for StructType {
822+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
823+
write!(f, "(struct")?;
824+
for field in self.fields.iter() {
825+
write!(f, " {field}")?;
826+
}
827+
write!(f, ")")?;
828+
Ok(())
829+
}
830+
}
831+
801832
/// Represents a type of a continuation in a WebAssembly module.
802833
#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
803834
pub struct ContType(pub PackedIndex);
804835

836+
impl fmt::Display for ContType {
837+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
838+
write!(f, "(cont {})", self.0)
839+
}
840+
}
841+
805842
impl ContType {
806843
/// Maps any `UnpackedIndex` via the specified closure.
807844
#[cfg(feature = "validate")]

crates/wasmparser/src/validator/component_types.rs

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3030,9 +3030,7 @@ impl<'a> SubtypeCx<'a> {
30303030

30313031
pub(crate) fn entity_type(&self, a: &EntityType, b: &EntityType, offset: usize) -> Result<()> {
30323032
match (a, b) {
3033-
(EntityType::Func(a), EntityType::Func(b)) => {
3034-
self.core_func_type(self.a[*a].unwrap_func(), self.b[*b].unwrap_func(), offset)
3035-
}
3033+
(EntityType::Func(a), EntityType::Func(b)) => self.core_func_type(*a, *b, offset),
30363034
(EntityType::Func(_), b) => bail!(offset, "expected {}, found func", b.desc()),
30373035
(EntityType::Table(a), EntityType::Table(b)) => Self::table_type(a, b, offset),
30383036
(EntityType::Table(_), b) => bail!(offset, "expected {}, found table", b.desc()),
@@ -3054,9 +3052,7 @@ impl<'a> SubtypeCx<'a> {
30543052
}
30553053
}
30563054
(EntityType::Global(_), b) => bail!(offset, "expected {}, found global", b.desc()),
3057-
(EntityType::Tag(a), EntityType::Tag(b)) => {
3058-
self.core_func_type(self.a[*a].unwrap_func(), self.b[*b].unwrap_func(), offset)
3059-
}
3055+
(EntityType::Tag(a), EntityType::Tag(b)) => self.core_func_type(*a, *b, offset),
30603056
(EntityType::Tag(_), b) => bail!(offset, "expected {}, found tag", b.desc()),
30613057
}
30623058
}
@@ -3094,16 +3090,20 @@ impl<'a> SubtypeCx<'a> {
30943090
}
30953091
}
30963092

3097-
fn core_func_type(&self, a: &FuncType, b: &FuncType, offset: usize) -> Result<()> {
3093+
fn core_func_type(&self, a: CoreTypeId, b: CoreTypeId, offset: usize) -> Result<()> {
3094+
debug_assert!(self.a.get(a).is_some());
3095+
debug_assert!(self.b.get(b).is_some());
30983096
if a == b {
3097+
debug_assert!(self.a.get(b).is_some());
3098+
debug_assert!(self.b.get(a).is_some());
30993099
Ok(())
31003100
} else {
31013101
bail!(
31023102
offset,
31033103
"expected: {}\n\
31043104
found: {}",
3105-
b.desc(),
3106-
a.desc(),
3105+
self.b[b],
3106+
self.a[a],
31073107
)
31083108
}
31093109
}
@@ -3353,6 +3353,21 @@ impl<'a> SubtypeArena<'a> {
33533353
list: TypeList::default(),
33543354
}
33553355
}
3356+
3357+
fn get<T>(&self, id: T) -> Option<&T::Data>
3358+
where
3359+
T: TypeIdentifier,
3360+
{
3361+
let index = id.index();
3362+
if index < T::list(self.types).len() {
3363+
self.types.get(id)
3364+
} else {
3365+
let temp_index = index - T::list(self.types).len();
3366+
let temp_index = u32::try_from(temp_index).unwrap();
3367+
let temp_id = T::from_index(temp_index);
3368+
self.list.get(temp_id)
3369+
}
3370+
}
33563371
}
33573372

33583373
impl<T> Index<T> for SubtypeArena<'_>
@@ -3362,15 +3377,7 @@ where
33623377
type Output = T::Data;
33633378

33643379
fn index(&self, id: T) -> &T::Data {
3365-
let index = id.index();
3366-
if index < T::list(self.types).len() {
3367-
&self.types[id]
3368-
} else {
3369-
let temp_index = index - T::list(self.types).len();
3370-
let temp_index = u32::try_from(temp_index).unwrap();
3371-
let temp_id = T::from_index(temp_index);
3372-
&self.list[temp_id]
3373-
}
3380+
self.get(id).unwrap()
33743381
}
33753382
}
33763383

tests/cli/component-model-async/task-builtins.wast

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,14 +265,14 @@
265265
(core func $f (canon context.get i32 0))
266266
(core instance $i (instantiate $m (with "" (instance (export "" (func $f))))))
267267
)
268-
"found: [] -> [i32]")
268+
"found: (func (result i32))")
269269
(assert_invalid
270270
(component
271271
(core module $m (import "" "" (func (param i32) (result i32))))
272272
(core func $f (canon context.set i32 0))
273273
(core instance $i (instantiate $m (with "" (instance (export "" (func $f))))))
274274
)
275-
"found: [i32] -> []")
275+
"found: (func (param i32))")
276276
(assert_invalid
277277
(component
278278
(core func (canon context.get i32 1)))

tests/cli/component-model-gc/core-type.wat

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,46 @@
3939
)
4040
"declared core type has `[I64]` result types, but actual lowering has `[I32]` result types"
4141
)
42+
43+
(component
44+
(import "f" (func $f (param "x" u32) (param "y" u32) (result u32)))
45+
46+
(core rec
47+
(type (func))
48+
(type $ty (func (param i32 i32) (result i32)))
49+
)
50+
51+
(core func $f (canon lower (func $f) (core-type $ty)))
52+
53+
(core module $m
54+
(rec
55+
(type (func))
56+
(type $ty (func (param i32 i32) (result i32)))
57+
)
58+
(import "a" "b" (func (type $ty)))
59+
)
60+
61+
(core instance (instantiate $m (with "a" (instance (export "b" (func $f))))))
62+
)
63+
64+
;; Satisfying an import with the "same" type but from the wrong rec group should
65+
;; fail.
66+
(assert_invalid
67+
(component
68+
(import "f" (func $f (param "x" u32) (param "y" u32) (result u32)))
69+
70+
(core type $ty (func (param i32 i32) (result i32)))
71+
(core func $f (canon lower (func $f) (core-type $ty)))
72+
73+
(core module $m
74+
(rec
75+
(type (func))
76+
(type $ty (func (param i32 i32) (result i32)))
77+
)
78+
(import "a" "b" (func (type $ty)))
79+
)
80+
81+
(core instance (instantiate $m (with "a" (instance (export "b" (func $f))))))
82+
)
83+
"type mismatch for export `b` of module instantiation argument `a`"
84+
)

tests/cli/component-model/instantiate.wast

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,15 +285,15 @@
285285
(core instance $i (instantiate $m2))
286286
(core instance (instantiate $m1 (with "" (instance $i))))
287287
)
288-
"expected: [] -> []")
288+
"expected: (func)")
289289
(assert_invalid
290290
(component
291291
(import "m1" (core module $m1 (import "" "" (func))))
292292
(import "m2" (core module $m2 (export "" (func (result i32)))))
293293
(core instance $i (instantiate $m2))
294294
(core instance (instantiate $m1 (with "" (instance $i))))
295295
)
296-
"expected: [] -> []")
296+
"expected: (func)")
297297
(assert_invalid
298298
(component
299299
(import "m1" (core module $m1 (import "" "" (global i32))))

tests/cli/gc/invalid.wast

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
(module
3737
(type $t (func))
3838
(func struct.new $t drop))
39-
"expected struct type at index 0, found (func ...)")
39+
"expected struct type at index 0, found (func)")
4040

4141
(assert_invalid
4242
(module
@@ -53,7 +53,7 @@
5353
(module
5454
(type $t (struct))
5555
(func array.new $t drop))
56-
"expected array type at index 0, found (struct ...)")
56+
"expected array type at index 0, found (struct)")
5757

5858
(assert_invalid
5959
(module
@@ -70,7 +70,7 @@
7070
(module
7171
(type $t (struct))
7272
(func block (type $t) end))
73-
"expected func type at index 0, found (struct ...)")
73+
"expected func type at index 0, found (struct)")
7474

7575
(assert_invalid
7676
(module

tests/cli/shared-everything-threads/builtins.wast

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,13 @@
2424
(core func $parallelism (canon thread.available_parallelism))
2525

2626
(core module $m
27-
(type $st (shared (func (param $context i32))))
28-
(import "" "spawn_ref" (func (param (ref null $st)) (param i32) (result i32)))
29-
(import "" "spawn_indirect" (func (param i32) (param i32) (result i32)))
30-
(import "" "parallelism" (func (result i32)))
27+
(type $spawned_func_ty (shared (func (param $context i32))))
28+
(type $spawn_ref_ty (shared (func (param (ref null $spawned_func_ty)) (param i32) (result i32))))
29+
(type $spawn_indirect_ty (shared (func (param i32) (param i32) (result i32))))
30+
(type $parallelism_ty (shared (func (result i32))))
31+
(import "" "spawn_ref" (func (type $spawn_ref_ty)))
32+
(import "" "spawn_indirect" (func (type $spawn_indirect_ty)))
33+
(import "" "parallelism" (func (type $parallelism_ty)))
3134
)
3235

3336
(core instance (instantiate $m

0 commit comments

Comments
 (0)