Skip to content

Commit

Permalink
Fix field type matching for subtyping and mutability (#9724)
Browse files Browse the repository at this point in the history 
Similar to bytecodealliance/wasm-tools#1934 but for
`wasmtime::FieldType`.
  • Loading branch information
@fitzgen
fitzgen authored Dec 3, 2024
1 parent db95666 commit 27ffc5e
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 51 deletions.
5 changes: 5 additions & 0 deletions crates/wasmtime/src/runtime/type_registry.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1235,11 +1235,16 @@ impl TypeRegistry {
}

/// Is type `sub` a subtype of `sup`?
#[inline]
pub fn is_subtype(&self, sub: VMSharedTypeIndex, sup: VMSharedTypeIndex) -> bool {
if sub == sup {
return true;
}

self.is_subtype_slow(sub, sup)
}

fn is_subtype_slow(&self, sub: VMSharedTypeIndex, sup: VMSharedTypeIndex) -> bool {
// Do the O(1) subtype checking trick:
//
// In a type system with single inheritance, the subtyping relationships
Expand Down
46 changes: 29 additions & 17 deletions crates/wasmtime/src/runtime/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1362,7 +1362,14 @@ impl StorageType {
/// Panics if either type is associated with a different engine from the
/// other.
pub fn eq(a: &Self, b: &Self) -> bool {
a.matches(b) && b.matches(a)
match (a, b) {
(StorageType::I8, StorageType::I8) => true,
(StorageType::I8, _) => false,
(StorageType::I16, StorageType::I16) => true,
(StorageType::I16, _) => false,
(StorageType::ValType(a), StorageType::ValType(b)) => ValType::eq(a, b),
(StorageType::ValType(_), _) => false,
}
}

pub(crate) fn comes_from_same_engine(&self, engine: &Engine) -> bool {
Expand Down Expand Up @@ -1457,8 +1464,21 @@ impl FieldType {
/// Panics if either type is associated with a different engine from the
/// other.
pub fn matches(&self, other: &Self) -> bool {
(other.mutability == Mutability::Var || self.mutability == Mutability::Const)
&& self.element_type.matches(&other.element_type)
// Our storage type must match `other`'s storage type and either
//
// 1. Both field types are immutable, or
//
// 2. Both field types are mutable and `other`'s storage type must match
// ours, i.e. the storage types are exactly the same.
use Mutability as M;
match (self.mutability, other.mutability) {
// Case 1
(M::Const, M::Const) => self.element_type.matches(&other.element_type),
// Case 2
(M::Var, M::Var) => StorageType::eq(&self.element_type, &other.element_type),
// Does not match.
_ => false,
}
}

/// Is field type `a` precisely equal to field type `b`?
Expand Down Expand Up @@ -1676,13 +1696,9 @@ impl StructType {
pub fn matches(&self, other: &StructType) -> bool {
assert!(self.comes_from_same_engine(other.engine()));

// Avoid matching on structure for subtyping checks when we have
// precisely the same type.
if self.type_index() == other.type_index() {
return true;
}

Self::fields_match(self.fields(), other.fields())
self.engine()
.signatures()
.is_subtype(self.type_index(), other.type_index())
}

fn fields_match(
Expand Down Expand Up @@ -1940,13 +1956,9 @@ impl ArrayType {
pub fn matches(&self, other: &ArrayType) -> bool {
assert!(self.comes_from_same_engine(other.engine()));

// Avoid matching on structure for subtyping checks when we have
// precisely the same type.
if self.type_index() == other.type_index() {
return true;
}

self.field_type().matches(&other.field_type())
self.engine()
.signatures()
.is_subtype(self.type_index(), other.type_index())
}

/// Is array type `a` precisely equal to array type `b`?
Expand Down
121 changes: 87 additions & 34 deletions tests/all/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@ fn field(heap_ty: HeapType) -> FieldType {
)
}

fn imm_field(heap_ty: HeapType) -> FieldType {
FieldType::new(
Mutability::Const,
StorageType::ValType(RefType::new(true, heap_ty).into()),
)
}

fn valty(heap_ty: HeapType) -> ValType {
ValType::Ref(RefType::new(true, heap_ty))
}
Expand Down Expand Up @@ -81,25 +88,57 @@ fn basic_struct_types() -> Result<()> {
fn struct_type_matches() -> Result<()> {
let engine = Engine::default();

let super_ty = StructType::new(&engine, [field(HeapType::Func)])?;
let super_ty = StructType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
None,
[imm_field(HeapType::Func)],
)?;

// Depth.
let sub_ty = StructType::new(&engine, [field(HeapType::NoFunc)])?;
let sub_ty = StructType::with_finality_and_supertype(
&engine,
Finality::Final,
Some(&super_ty),
[imm_field(HeapType::NoFunc)],
)?;
assert!(sub_ty.matches(&super_ty));
let not_sub_ty = StructType::new(&engine, [imm_field(HeapType::NoFunc)])?;
assert!(!not_sub_ty.matches(&super_ty));

// Width.
let sub_ty = StructType::new(&engine, [field(HeapType::Func), field(HeapType::Extern)])?;
let sub_ty = StructType::with_finality_and_supertype(
&engine,
Finality::Final,
Some(&super_ty),
[imm_field(HeapType::Func), imm_field(HeapType::Extern)],
)?;
assert!(sub_ty.matches(&super_ty));
let not_sub_ty = StructType::new(
&engine,
[imm_field(HeapType::Func), imm_field(HeapType::Extern)],
)?;
assert!(!not_sub_ty.matches(&super_ty));

// Depth and width.
let sub_ty = StructType::new(&engine, [field(HeapType::NoFunc), field(HeapType::Extern)])?;
let sub_ty = StructType::with_finality_and_supertype(
&engine,
Finality::Final,
Some(&super_ty),
[imm_field(HeapType::NoFunc), imm_field(HeapType::Extern)],
)?;
assert!(sub_ty.matches(&super_ty));
let not_sub_ty = StructType::new(
&engine,
[imm_field(HeapType::NoFunc), imm_field(HeapType::Extern)],
)?;
assert!(!not_sub_ty.matches(&super_ty));

// Not depth.
// Unrelated structs.
let not_sub_ty = StructType::new(&engine, [imm_field(HeapType::Extern)])?;
assert!(!not_sub_ty.matches(&super_ty));
let not_sub_ty = StructType::new(&engine, [field(HeapType::Extern)])?;
assert!(!not_sub_ty.matches(&super_ty));

// Not width.
let not_sub_ty = StructType::new(&engine, [])?;
assert!(!not_sub_ty.matches(&super_ty));

Expand All @@ -109,29 +148,41 @@ fn struct_type_matches() -> Result<()> {
#[test]
fn struct_subtyping_fields_must_match() -> Result<()> {
let engine = Engine::default();

let a = StructType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
None,
[field(HeapType::Any)],
[imm_field(HeapType::Any)],
)?;

for (expected, fields) in [
// Missing field.
(false, vec![]),
// Non-matching field.
(false, vec![field(HeapType::Extern)]),
// Exact match is okay.
(true, vec![field(HeapType::Any)]),
// Subtype of the field is okay.
(true, vec![field(HeapType::Eq)]),
// Extra fields are okay.
(true, vec![field(HeapType::Any), field(HeapType::Extern)]),
for (msg, expected, fields) in [
("Missing field", false, vec![]),
(
"Non-matching field",
false,
vec![imm_field(HeapType::Extern)],
),
("Wrong mutability field", false, vec![field(HeapType::Any)]),
("Exact match is okay", true, vec![imm_field(HeapType::Any)]),
(
"Subtype of the field is okay",
true,
vec![imm_field(HeapType::Eq)],
),
(
"Extra fields are okay",
true,
vec![imm_field(HeapType::Any), imm_field(HeapType::Extern)],
),
] {
let actual =
StructType::with_finality_and_supertype(&engine, Finality::NonFinal, Some(&a), fields)
.is_ok();
assert_eq!(expected, actual);
assert_eq!(
expected, actual,
"expected valid? {expected}; actually valid? {actual}; {msg}"
);
}

Ok(())
Expand Down Expand Up @@ -263,16 +314,18 @@ fn array_subtyping_field_must_match() -> Result<()> {
&engine,
Finality::NonFinal,
None,
field(HeapType::Any),
imm_field(HeapType::Any),
)?;

for (expected, field) in [
// Non-matching field.
(false, field(HeapType::Extern)),
(false, imm_field(HeapType::Extern)),
// Wrong mutability field.
(false, field(HeapType::Any)),
// Exact match is okay.
(true, field(HeapType::Any)),
(true, imm_field(HeapType::Any)),
// Subtype of the field is okay.
(true, field(HeapType::Eq)),
(true, imm_field(HeapType::Eq)),
] {
let actual =
ArrayType::with_finality_and_supertype(&engine, Finality::NonFinal, Some(&a), field)
Expand Down Expand Up @@ -322,61 +375,61 @@ fn array_subtyping() -> Result<()> {
&engine,
Finality::NonFinal,
None,
field(HeapType::Any),
imm_field(HeapType::Any),
)?;
let a = ArrayType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
Some(&base),
field(HeapType::Any),
imm_field(HeapType::Any),
)?;
let b = ArrayType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
Some(&base),
field(HeapType::Eq),
imm_field(HeapType::Eq),
)?;
let c = ArrayType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
Some(&a),
field(HeapType::Any),
imm_field(HeapType::Any),
)?;
let d = ArrayType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
Some(&a),
field(HeapType::Eq),
imm_field(HeapType::Eq),
)?;
let e = ArrayType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
Some(&c),
field(HeapType::Any),
imm_field(HeapType::Any),
)?;
let f = ArrayType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
Some(&e),
field(HeapType::Any),
imm_field(HeapType::Any),
)?;
let g = ArrayType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
None,
field(HeapType::Eq),
imm_field(HeapType::Eq),
)?;
let h = ArrayType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
Some(&g),
field(HeapType::Eq),
imm_field(HeapType::Eq),
)?;
let i = ArrayType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
Some(&h),
field(HeapType::Eq),
imm_field(HeapType::Eq),
)?;

for (expected, sub_name, sub, sup_name, sup) in [
Expand Down

0 comments on commit 27ffc5e

Please sign in to comment.