Skip to content

Commit

Permalink
threads: add polymorphism over shared
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
abrown committed Aug 14, 2024
1 parent 39f6170 commit b7d1e6f
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 77 deletions.
10 changes: 10 additions & 0 deletions crates/wasmparser/src/resources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
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 @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
5 changes: 4 additions & 1 deletion crates/wasmparser/src/validator/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ impl<T: WasmModuleResources> FuncValidator<T> {
mod tests {
use super::*;
use crate::types::CoreTypeId;
use crate::HeapType;
use crate::{HeapType, RefType};

struct EmptyResources(crate::SubType);

Expand Down Expand Up @@ -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!()
}
Expand Down
185 changes: 109 additions & 76 deletions crates/wasmparser/src/validator/operators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,6 @@ impl From<RefType> for MaybeType {
}
}

impl MaybeType {
fn as_type(&self) -> Option<ValType> {
match *self {
Self::Type(ty) => Some(ty),
Self::Bot | Self::HeapBot => None,
}
}
}

impl OperatorValidator {
fn new(features: &WasmFeatures, allocs: OperatorValidatorAllocations) -> Self {
let OperatorValidatorAllocations {
Expand Down Expand Up @@ -556,12 +547,20 @@ where
popped: Option<MaybeType>,
) -> Result<MaybeType> {
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<ValType>) -> Result<MaybeType> {
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 {
Expand All @@ -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<Option<RefType>> {
match self.pop_operand(None)? {
let ty = self.pop_operand(None)?;
self._as_ref_type(ty)
}

fn _as_ref_type(&self, ty: MaybeType) -> Result<Option<RefType>> {
match ty {
MaybeType::Bot | MaybeType::HeapBot => Ok(None),
MaybeType::Type(ValType::Ref(rt)) => Ok(Some(rt)),
MaybeType::Type(ty) => bail!(
Expand All @@ -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<Option<RefType>> {
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<ValType> {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
)
)

0 comments on commit b7d1e6f

Please sign in to comment.