Skip to content

Commit

Permalink
Fix ref.{test,cast} to check against top types (bytecodealliance#1336)
Browse files Browse the repository at this point in the history
The spec says that `ref.{test,cast} rt` are valid when `rt'` is on the stack and
`rt <: rt'`. We were checking this before, but not allowing `rt'` to upcast to
any of its super types. The solution is to immediately upcast it to its top
type, and then check `rt <: top(rt')`.

This gets the `type-subtyping.wast` test fully passing.
  • Loading branch information
fitzgen authored Dec 13, 2023
1 parent e67d2bf commit 0f9476a
Show file tree
Hide file tree
Showing 49 changed files with 1,006 additions and 5 deletions.
10 changes: 10 additions & 0 deletions crates/wasmparser/src/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,9 @@ pub trait WasmModuleResources {
offset: usize,
) -> Result<(), BinaryReaderError>;

/// Get the top type for the given heap type.
fn top_type(&self, heap_type: &HeapType) -> HeapType;

/// Returns the number of elements.
fn element_count(&self) -> u32;

Expand Down Expand Up @@ -312,6 +315,9 @@ where
fn check_heap_type(&self, t: &mut HeapType, offset: usize) -> Result<(), BinaryReaderError> {
T::check_heap_type(self, t, offset)
}
fn top_type(&self, heap_type: &HeapType) -> HeapType {
T::top_type(self, heap_type)
}
fn element_type_at(&self, at: u32) -> Option<RefType> {
T::element_type_at(self, at)
}
Expand Down Expand Up @@ -368,6 +374,10 @@ where
T::check_heap_type(self, t, offset)
}

fn top_type(&self, heap_type: &HeapType) -> HeapType {
T::top_type(self, heap_type)
}

fn element_type_at(&self, at: u32) -> Option<RefType> {
T::element_type_at(self, at)
}
Expand Down
8 changes: 8 additions & 0 deletions crates/wasmparser/src/validator/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,10 @@ impl WasmModuleResources for OperatorValidatorResources<'_> {
self.module.check_heap_type(t, offset)
}

fn top_type(&self, heap_type: &HeapType) -> HeapType {
self.types.top_type(heap_type)
}

fn element_type_at(&self, at: u32) -> Option<RefType> {
self.module.element_types.get(at as usize).cloned()
}
Expand Down Expand Up @@ -1268,6 +1272,10 @@ impl WasmModuleResources for ValidatorResources {
self.0.check_heap_type(t, offset)
}

fn top_type(&self, heap_type: &HeapType) -> HeapType {
self.0.snapshot.as_ref().unwrap().top_type(heap_type)
}

fn element_type_at(&self, at: u32) -> Option<RefType> {
self.0.element_types.get(at as usize).cloned()
}
Expand Down
3 changes: 3 additions & 0 deletions crates/wasmparser/src/validator/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ mod tests {
fn check_heap_type(&self, _t: &mut HeapType, _offset: usize) -> Result<()> {
Ok(())
}
fn top_type(&self, _heap_type: &HeapType) -> HeapType {
todo!()
}
fn element_type_at(&self, _at: u32) -> Option<crate::RefType> {
todo!()
}
Expand Down
19 changes: 15 additions & 4 deletions crates/wasmparser/src/validator/operators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,12 @@ where

/// Common helper for `ref.test` and `ref.cast` downcasting/checking
/// instructions. Returns the given `heap_type` as a `ValType`.
fn check_downcast(&mut self, nullable: bool, mut heap_type: HeapType) -> Result<ValType> {
fn check_downcast(
&mut self,
nullable: bool,
mut heap_type: HeapType,
inst_name: &str,
) -> Result<ValType> {
self.resources
.check_heap_type(&mut heap_type, self.offset)?;

Expand All @@ -989,11 +994,17 @@ where
),
Some(ty) => ty,
};
let sup_ty = RefType::new(
sup_ty.is_nullable(),
self.resources.top_type(&sup_ty.heap_type()),
)
.unwrap();

if !self.resources.is_subtype(sub_ty, sup_ty.into()) {
bail!(
self.offset,
"ref.test's heap type must be a sub type of the type on the stack"
"{inst_name}'s heap type must be a sub type of the type on the stack: \
{sub_ty} is not a sub type of {sup_ty}"
);
}

Expand All @@ -1003,14 +1014,14 @@ where
/// Common helper for both nullable and non-nullable variants of `ref.test`
/// instructions.
fn check_ref_test(&mut self, nullable: bool, heap_type: HeapType) -> Result<()> {
self.check_downcast(nullable, heap_type)?;
self.check_downcast(nullable, heap_type, "ref.test")?;
self.push_operand(ValType::I32)
}

/// Common helper for both nullable and non-nullable variants of `ref.cast`
/// instructions.
fn check_ref_cast(&mut self, nullable: bool, heap_type: HeapType) -> Result<()> {
let sub_ty = self.check_downcast(nullable, heap_type)?;
let sub_ty = self.check_downcast(nullable, heap_type, "ref.cast")?;
self.push_operand(sub_ty)
}

Expand Down
21 changes: 21 additions & 0 deletions crates/wasmparser/src/validator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2727,6 +2727,27 @@ impl TypeList {
}
}

/// Get the top type of the given heap type.
///
/// Concrete types must have had their indices canonicalized to core type
/// ids, otherwise this method will panic.
pub fn top_type(&self, heap_type: &HeapType) -> HeapType {
match *heap_type {
HeapType::Concrete(idx) => match self[idx.as_core_type_id().unwrap()].composite_type {
CompositeType::Func(_) => HeapType::Func,
CompositeType::Array(_) | CompositeType::Struct(_) => HeapType::Any,
},
HeapType::Func | HeapType::NoFunc => HeapType::Func,
HeapType::Extern | HeapType::NoExtern => HeapType::Extern,
HeapType::Any
| HeapType::Eq
| HeapType::Struct
| HeapType::Array
| HeapType::I31
| HeapType::None => HeapType::Any,
}
}

fn checkpoint(&self) -> TypeListCheckpoint {
let TypeList {
alias_mappings: _,
Expand Down
1 change: 0 additions & 1 deletion tests/roundtrip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ fn skip_validation(test: &Path) -> bool {
"proposals/gc/ref_eq.wast",
"proposals/gc/ref_test.wast",
"proposals/gc/struct.wast",
"proposals/gc/type-subtyping.wast",
"exnref/exnref.wast",
"exnref/throw_ref.wast",
"exnref/try_table.wast",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
(module
(type $e0 (;0;) (sub (array i32)))
(type $e1 (;1;) (sub $e0 (;0;) (array i32)))
(type $e2 (;2;) (sub (array anyref)))
(type $e3 (;3;) (sub (array (ref null 0))))
(type $e4 (;4;) (sub (array (ref 1))))
(type $m1 (;5;) (sub (array (mut i32))))
(type $m2 (;6;) (sub $m1 (;5;) (array (mut i32))))
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
(module
(type $e0 (;0;) (sub (struct)))
(type $e1 (;1;) (sub $e0 (;0;) (struct)))
(type $e2 (;2;) (sub $e1 (;1;) (struct (field i32))))
(type $e3 (;3;) (sub $e2 (;2;) (struct (field i32) (field (ref null 0)))))
(type $e4 (;4;) (sub $e3 (;3;) (struct (field i32) (field (ref 0)) (field (mut i64)))))
(type $e5 (;5;) (sub $e4 (;4;) (struct (field i32) (field (ref 1)) (field (mut i64)))))
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
(module
(rec
(type $f1 (;0;) (sub (func)))
(type (;1;) (struct (field (ref 0))))
)
(rec
(type $f2 (;2;) (sub (func)))
(type (;3;) (struct (field (ref 2))))
)
(rec
(type $g (;4;) (sub $f1 (;0;) (func)))
(type (;5;) (struct))
)
(func $g (;0;) (type $g))
(global (;0;) (ref 0) ref.func $g)
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
(module
(rec
(type $f1 (;0;) (sub (func)))
(type $s1 (;1;) (sub (struct (field (ref 0)))))
)
(rec
(type $f2 (;2;) (sub (func)))
(type $s2 (;3;) (sub (struct (field (ref 2)))))
)
(rec
(type $g1 (;4;) (sub $f1 (;0;) (func)))
(type (;5;) (sub $s1 (;1;) (struct (field (ref 0)) (field (ref 0)) (field (ref 2)) (field (ref 2)) (field (ref 4)))))
)
(rec
(type $g2 (;6;) (sub $f2 (;2;) (func)))
(type (;7;) (sub $s2 (;3;) (struct (field (ref 0)) (field (ref 2)) (field (ref 0)) (field (ref 2)) (field (ref 6)))))
)
(rec
(type $h (;8;) (sub $g2 (;6;) (func)))
(type (;9;) (struct))
)
(func $h (;0;) (type $h))
(global (;0;) (ref 0) ref.func $h)
(global (;1;) (ref 4) ref.func $h)
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
(module
(rec
(type $f11 (;0;) (sub (func (result (ref func)))))
(type $f12 (;1;) (sub $f11 (;0;) (func (result (ref 0)))))
)
(rec
(type $f21 (;2;) (sub (func (result (ref func)))))
(type $f22 (;3;) (sub $f21 (;2;) (func (result (ref 2)))))
)
(func $f11 (;0;) (type $f11) (result (ref func))
unreachable
)
(func $f12 (;1;) (type $f12) (result (ref 0))
unreachable
)
(global (;0;) (ref 0) ref.func $f11)
(global (;1;) (ref 2) ref.func $f11)
(global (;2;) (ref 1) ref.func $f12)
(global (;3;) (ref 3) ref.func $f12)
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
(module
(rec
(type $f11 (;0;) (sub (func (result (ref func)))))
(type $f12 (;1;) (sub $f11 (;0;) (func (result (ref 0)))))
)
(rec
(type $f21 (;2;) (sub (func (result (ref func)))))
(type $f22 (;3;) (sub $f21 (;2;) (func (result (ref 2)))))
)
(rec
(type $g11 (;4;) (sub $f11 (;0;) (func (result (ref func)))))
(type $g12 (;5;) (sub $g11 (;4;) (func (result (ref 4)))))
)
(rec
(type $g21 (;6;) (sub $f21 (;2;) (func (result (ref func)))))
(type $g22 (;7;) (sub $g21 (;6;) (func (result (ref 6)))))
)
(func $g11 (;0;) (type $g11) (result (ref func))
unreachable
)
(func $g12 (;1;) (type $g12) (result (ref 4))
unreachable
)
(global (;0;) (ref 0) ref.func $g11)
(global (;1;) (ref 2) ref.func $g11)
(global (;2;) (ref 0) ref.func $g12)
(global (;3;) (ref 2) ref.func $g12)
(global (;4;) (ref 4) ref.func $g11)
(global (;5;) (ref 6) ref.func $g11)
(global (;6;) (ref 5) ref.func $g12)
(global (;7;) (ref 7) ref.func $g12)
)
124 changes: 124 additions & 0 deletions tests/snapshots/testsuite/proposals/gc/type-subtyping.wast/17.print
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
(module
(type $t0 (;0;) (sub (func (result funcref))))
(rec
(type $t1 (;1;) (sub $t0 (;0;) (func (result (ref null 1)))))
)
(rec
(type $t2 (;2;) (sub $t1 (;1;) (func (result (ref null 2)))))
)
(type (;3;) (func))
(func $f0 (;0;) (type $t0) (result funcref)
ref.null func
)
(func $f1 (;1;) (type $t1) (result (ref null 1))
ref.null 1
)
(func $f2 (;2;) (type $t2) (result (ref null 2))
ref.null 2
)
(func (;3;) (type 3)
block (result funcref) ;; label = @1
i32.const 0
call_indirect (type $t0)
end
block (result funcref) ;; label = @1
i32.const 1
call_indirect (type $t0)
end
block (result funcref) ;; label = @1
i32.const 2
call_indirect (type $t0)
end
block (result (ref null 1)) ;; label = @1
i32.const 1
call_indirect (type $t1)
end
block (result (ref null 1)) ;; label = @1
i32.const 2
call_indirect (type $t1)
end
block (result (ref null 2)) ;; label = @1
i32.const 2
call_indirect (type $t2)
end
block (result (ref null 0)) ;; label = @1
i32.const 0
table.get 0
ref.cast (ref 0)
end
block (result (ref null 0)) ;; label = @1
i32.const 1
table.get 0
ref.cast (ref 0)
end
block (result (ref null 0)) ;; label = @1
i32.const 2
table.get 0
ref.cast (ref 0)
end
block (result (ref null 1)) ;; label = @1
i32.const 1
table.get 0
ref.cast (ref 1)
end
block (result (ref null 1)) ;; label = @1
i32.const 2
table.get 0
ref.cast (ref 1)
end
block (result (ref null 2)) ;; label = @1
i32.const 2
table.get 0
ref.cast (ref 2)
end
br 0 (;@0;)
)
(func (;4;) (type 3)
block (result (ref null 1)) ;; label = @1
i32.const 0
call_indirect (type $t1)
end
br 0 (;@0;)
)
(func (;5;) (type 3)
block (result (ref null 1)) ;; label = @1
i32.const 0
call_indirect (type $t2)
end
br 0 (;@0;)
)
(func (;6;) (type 3)
block (result (ref null 1)) ;; label = @1
i32.const 1
call_indirect (type $t2)
end
br 0 (;@0;)
)
(func (;7;) (type 3)
i32.const 0
table.get 0
ref.cast (ref 1)
br 0 (;@0;)
)
(func (;8;) (type 3)
i32.const 0
table.get 0
ref.cast (ref 2)
br 0 (;@0;)
)
(func (;9;) (type 3)
i32.const 1
table.get 0
ref.cast (ref 2)
br 0 (;@0;)
)
(table (;0;) 3 3 funcref)
(export "run" (func 3))
(export "fail1" (func 4))
(export "fail2" (func 5))
(export "fail3" (func 6))
(export "fail4" (func 7))
(export "fail5" (func 8))
(export "fail6" (func 9))
(elem (;0;) (i32.const 0) func $f0 $f1 $f2)
)
Loading

0 comments on commit 0f9476a

Please sign in to comment.