From b7d1e6f8fe7d7631a0629fdc2d30a53e74bcb579 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 13 Aug 2024 17:16:20 -0700 Subject: [PATCH] threads: add polymorphism over `shared` Certain instructions accept both shared and unshared references: `ref.eq`, `i31.get_s`, `i31.get_u`, `array.len`, `any.convert_extern`, and `extern.convert_any`. Validation needs to handle these special cases to pass Binaryen's `polymorphism.wast` test. This change refactors the `pop_operand` family of functions with several helpers to add `pop_maybe_shared_ref`; `pop_maybe_shared_ref` lets the validator pass an unshared `RefType` expectation but adapts it to being shared if that is what is on the stack. --- crates/wasmparser/src/resources.rs | 10 + crates/wasmparser/src/validator/core.rs | 8 + crates/wasmparser/src/validator/func.rs | 5 +- crates/wasmparser/src/validator/operators.rs | 185 +++++++++++------- .../polymorphism.wast/0.print | 46 +++++ 5 files changed, 177 insertions(+), 77 deletions(-) create mode 100644 tests/snapshots/local/shared-everything-threads/polymorphism.wast/0.print diff --git a/crates/wasmparser/src/resources.rs b/crates/wasmparser/src/resources.rs index 729fc285cb..b9c98bf36a 100644 --- a/crates/wasmparser/src/resources.rs +++ b/crates/wasmparser/src/resources.rs @@ -67,6 +67,9 @@ pub trait WasmModuleResources { /// Is `a` a subtype of `b`? fn is_subtype(&self, a: ValType, b: ValType) -> bool; + /// Is the given reference type `shared`? + fn is_shared_ref_type(&self, ty: RefType) -> bool; + /// Check and canonicalize a value type. /// /// This will validate that `t` is valid under the `features` provided and @@ -161,6 +164,9 @@ where fn is_subtype(&self, a: ValType, b: ValType) -> bool { T::is_subtype(self, a, b) } + fn is_shared_ref_type(&self, ty: RefType) -> bool { + T::is_shared_ref_type(self, ty) + } fn element_count(&self) -> u32 { T::element_count(self) } @@ -220,6 +226,10 @@ where T::is_subtype(self, a, b) } + fn is_shared_ref_type(&self, ty: RefType) -> bool { + T::is_shared_ref_type(self, ty) + } + fn element_count(&self) -> u32 { T::element_count(self) } diff --git a/crates/wasmparser/src/validator/core.rs b/crates/wasmparser/src/validator/core.rs index 003dbe5916..82a52e964b 100644 --- a/crates/wasmparser/src/validator/core.rs +++ b/crates/wasmparser/src/validator/core.rs @@ -1322,6 +1322,10 @@ impl WasmModuleResources for OperatorValidatorResources<'_> { self.types.valtype_is_subtype(a, b) } + fn is_shared_ref_type(&self, ty: RefType) -> bool { + self.types.reftype_is_shared(ty) + } + fn element_count(&self) -> u32 { self.module.element_types.len() as u32 } @@ -1394,6 +1398,10 @@ impl WasmModuleResources for ValidatorResources { self.0.snapshot.as_ref().unwrap().valtype_is_subtype(a, b) } + fn is_shared_ref_type(&self, ty: RefType) -> bool { + self.0.snapshot.as_ref().unwrap().reftype_is_shared(ty) + } + fn element_count(&self) -> u32 { self.0.element_types.len() as u32 } diff --git a/crates/wasmparser/src/validator/func.rs b/crates/wasmparser/src/validator/func.rs index 9d393585a7..a188bbc5dc 100644 --- a/crates/wasmparser/src/validator/func.rs +++ b/crates/wasmparser/src/validator/func.rs @@ -237,7 +237,7 @@ impl FuncValidator { mod tests { use super::*; use crate::types::CoreTypeId; - use crate::HeapType; + use crate::{HeapType, RefType}; struct EmptyResources(crate::SubType); @@ -288,6 +288,9 @@ mod tests { fn is_subtype(&self, _t1: ValType, _t2: ValType) -> bool { todo!() } + fn is_shared_ref_type(&self, _ty: RefType) -> bool { + todo!() + } fn element_count(&self) -> u32 { todo!() } diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs index abaf409a39..7dce89c9f8 100644 --- a/crates/wasmparser/src/validator/operators.rs +++ b/crates/wasmparser/src/validator/operators.rs @@ -199,15 +199,6 @@ impl From for MaybeType { } } -impl MaybeType { - fn as_type(&self) -> Option { - match *self { - Self::Type(ty) => Some(ty), - Self::Bot | Self::HeapBot => None, - } - } -} - impl OperatorValidator { fn new(features: &WasmFeatures, allocs: OperatorValidatorAllocations) -> Self { let OperatorValidatorAllocations { @@ -556,12 +547,20 @@ where popped: Option, ) -> Result { self.operands.extend(popped); + let actual = self._calculate_operand_type(expected)?; + if let Some(expected) = expected { + self._check_operand_type(expected, actual)?; + } + Ok(actual) + } + + fn _calculate_operand_type(&mut self, expected: Option) -> Result { let control = match self.control.last() { Some(c) => c, None => return Err(self.err_beyond_end(self.offset)), }; - let actual = if self.operands.len() == control.height && control.unreachable { - MaybeType::Bot + if self.operands.len() == control.height && control.unreachable { + Ok(MaybeType::Bot) } else { if self.operands.len() == control.height { let desc = match expected { @@ -573,48 +572,58 @@ where "type mismatch: expected {desc} but nothing on stack" ) } else { - self.operands.pop().unwrap() + Ok(self + .operands + .pop() + .expect("must have an operand on the stack")) } - }; - if let Some(expected) = expected { - match (actual, expected) { - // The bottom type matches all expectations - (MaybeType::Bot, _) - // The "heap bottom" type only matches other references types, - // but not any integer types. - | (MaybeType::HeapBot, ValType::Ref(_)) => {} - - // Use the `is_subtype` predicate to test if a found type matches - // the expectation. - (MaybeType::Type(actual), expected) => { - if !self.resources.is_subtype(actual, expected) { - bail!( - self.offset, - "type mismatch: expected {}, found {}", - ty_to_str(expected), - ty_to_str(actual) - ); - } - } + } + } - // A "heap bottom" type cannot match any numeric types. - ( - MaybeType::HeapBot, - ValType::I32 | ValType::I64 | ValType::F32 | ValType::F64 | ValType::V128, - ) => { + fn _check_operand_type(&self, expected: ValType, actual: MaybeType) -> Result<()> { + match (actual, expected) { + // The bottom type matches all expectations + (MaybeType::Bot, _) + // The "heap bottom" type only matches other references types, + // but not any integer types. + | (MaybeType::HeapBot, ValType::Ref(_)) => {} + + // Use the `is_subtype` predicate to test if a found type matches + // the expectation. + (MaybeType::Type(actual), expected) => { + if !self.resources.is_subtype(actual, expected) { bail!( self.offset, - "type mismatch: expected {}, found heap type", - ty_to_str(expected) - ) + "type mismatch: expected {}, found {}", + ty_to_str(expected), + ty_to_str(actual) + ); } } + + // A "heap bottom" type cannot match any numeric types. + ( + MaybeType::HeapBot, + ValType::I32 | ValType::I64 | ValType::F32 | ValType::F64 | ValType::V128, + ) => { + bail!( + self.offset, + "type mismatch: expected {}, found heap type", + ty_to_str(expected) + ) + } } - Ok(actual) + Ok(()) } + /// Pop a reference type from the operand stack. fn pop_ref(&mut self) -> Result> { - match self.pop_operand(None)? { + let ty = self.pop_operand(None)?; + self._as_ref_type(ty) + } + + fn _as_ref_type(&self, ty: MaybeType) -> Result> { + match ty { MaybeType::Bot | MaybeType::HeapBot => Ok(None), MaybeType::Type(ValType::Ref(rt)) => Ok(Some(rt)), MaybeType::Type(ty) => bail!( @@ -625,6 +634,29 @@ where } } + /// Pop a reference type from the operand stack, checking if it matches + /// `expected` or the shared version of `expected`. This function will panic + /// if `expected` is shared or a concrete type. + fn pop_maybe_shared_ref(&mut self, expected: RefType) -> Result> { + assert!(!self.resources.is_shared_ref_type(expected)); + let actual = self._calculate_operand_type(Some(expected.into()))?; + let actual_ref = match self._as_ref_type(actual)? { + Some(rt) => rt, + None => return Ok(None), + }; + // Change our expectation based on whether we're dealing with an actual + // shared or unshared type. + let expected = if self.resources.is_shared_ref_type(actual_ref) { + expected + .shared() + .expect("this only expects abstract heap types") + } else { + expected + }; + self._check_operand_type(expected.into(), actual)?; + Ok(Some(actual_ref)) + } + /// Fetches the type for the local at `idx`, returning an error if it's out /// of bounds. fn local(&self, idx: u32) -> Result { @@ -2715,26 +2747,17 @@ where Ok(()) } fn visit_ref_eq(&mut self) -> Self::Output { - match (self.pop_ref()?, self.pop_ref()?) { - (Some(a), Some(b)) => { - const EQREF: RefType = RefType::EQ.nullable(); - let shared_eqref = EQREF.shared().expect("eq is abstract"); - if a != EQREF && a != shared_eqref { - bail!( - self.offset, - "type mismatch: expected `{}` or `{}`", - EQREF, - shared_eqref - ); - } - if a != b { - bail!( - self.offset, - "type mismatch: expected `ref.eq` types to match" - ); - } - } - _ => {} + let a = self + .pop_maybe_shared_ref(RefType::EQ.nullable())? + .expect("an actual ref type"); + let b = self + .pop_maybe_shared_ref(RefType::EQ.nullable())? + .expect("an actual ref type"); + if self.resources.is_shared_ref_type(a) != self.resources.is_shared_ref_type(b) { + bail!( + self.offset, + "type mismatch: expected `ref.eq` types to match `shared`-ness" + ); } self.push_operand(ValType::I32) } @@ -4235,7 +4258,7 @@ where Ok(()) } fn visit_array_len(&mut self) -> Self::Output { - self.pop_operand(Some(RefType::ARRAY.nullable().into()))?; + self.pop_maybe_shared_ref(RefType::ARRAY.nullable())?; self.push_operand(ValType::I32) } fn visit_array_fill(&mut self, array_type_index: u32) -> Self::Output { @@ -4417,24 +4440,34 @@ where Ok(()) } fn visit_any_convert_extern(&mut self) -> Self::Output { - let extern_ref = self.pop_operand(Some(RefType::EXTERNREF.into()))?; - let is_nullable = extern_ref - .as_type() - .map_or(false, |ty| ty.as_reference_type().unwrap().is_nullable()); + let extern_ref = self.pop_maybe_shared_ref(RefType::EXTERNREF)?; + let (is_nullable, shared) = if let Some(extern_ref) = extern_ref { + ( + extern_ref.is_nullable(), + self.resources.is_shared_ref_type(extern_ref), + ) + } else { + (false, false) + }; let heap_type = HeapType::Abstract { - shared: false, // TODO: handle shared--see https://github.com/WebAssembly/shared-everything-threads/issues/65. + shared, ty: AbstractHeapType::Any, }; let any_ref = RefType::new(is_nullable, heap_type).unwrap(); self.push_operand(any_ref) } fn visit_extern_convert_any(&mut self) -> Self::Output { - let any_ref = self.pop_operand(Some(RefType::ANY.nullable().into()))?; - let is_nullable = any_ref - .as_type() - .map_or(false, |ty| ty.as_reference_type().unwrap().is_nullable()); + let any_ref = self.pop_maybe_shared_ref(RefType::ANY.nullable())?; + let (is_nullable, shared) = if let Some(any_ref) = any_ref { + ( + any_ref.is_nullable(), + self.resources.is_shared_ref_type(any_ref), + ) + } else { + (false, false) + }; let heap_type = HeapType::Abstract { - shared: false, // TODO: handle shared--see https://github.com/WebAssembly/shared-everything-threads/issues/65. + shared, ty: AbstractHeapType::Extern, }; let extern_ref = RefType::new(is_nullable, heap_type).unwrap(); @@ -4550,11 +4583,11 @@ where )) } fn visit_i31_get_s(&mut self) -> Self::Output { - self.pop_operand(Some(ValType::Ref(RefType::I31REF)))?; + self.pop_maybe_shared_ref(RefType::I31REF)?; self.push_operand(ValType::I32) } fn visit_i31_get_u(&mut self) -> Self::Output { - self.pop_operand(Some(ValType::Ref(RefType::I31REF)))?; + self.pop_maybe_shared_ref(RefType::I31REF)?; self.push_operand(ValType::I32) } fn visit_try(&mut self, mut ty: BlockType) -> Self::Output { diff --git a/tests/snapshots/local/shared-everything-threads/polymorphism.wast/0.print b/tests/snapshots/local/shared-everything-threads/polymorphism.wast/0.print new file mode 100644 index 0000000000..b8b3e7a1ae --- /dev/null +++ b/tests/snapshots/local/shared-everything-threads/polymorphism.wast/0.print @@ -0,0 +1,46 @@ +(module + (type (;0;) (func)) + (type (;1;) (func (param (ref null (shared i31))))) + (type (;2;) (func (param (ref null (shared array))))) + (type (;3;) (func (param (ref null (shared extern))) (result (ref null (shared any))))) + (type (;4;) (func (param (ref (shared extern))) (result (ref (shared any))))) + (type (;5;) (func (param (ref null (shared any))) (result (ref null (shared extern))))) + (type (;6;) (func (param (ref (shared any))) (result (ref (shared extern))))) + (func (;0;) (type 0) + ref.null (shared none) + ref.null (shared none) + ref.eq + drop + ) + (func (;1;) (type 1) (param (ref null (shared i31))) + local.get 0 + i31.get_s + drop + ) + (func (;2;) (type 1) (param (ref null (shared i31))) + local.get 0 + i31.get_u + drop + ) + (func (;3;) (type 2) (param (ref null (shared array))) + local.get 0 + array.len + drop + ) + (func (;4;) (type 3) (param (ref null (shared extern))) (result (ref null (shared any))) + local.get 0 + any.convert_extern + ) + (func (;5;) (type 4) (param (ref (shared extern))) (result (ref (shared any))) + local.get 0 + any.convert_extern + ) + (func (;6;) (type 5) (param (ref null (shared any))) (result (ref null (shared extern))) + local.get 0 + extern.convert_any + ) + (func (;7;) (type 6) (param (ref (shared any))) (result (ref (shared extern))) + local.get 0 + extern.convert_any + ) +)