diff --git a/crates/wasm-compose/src/encoding.rs b/crates/wasm-compose/src/encoding.rs index 48045609e3..37d32c00ff 100644 --- a/crates/wasm-compose/src/encoding.rs +++ b/crates/wasm-compose/src/encoding.rs @@ -285,7 +285,6 @@ impl<'a> TypeEncoder<'a> { wasmparser::HeapType::Func => HeapType::Func, wasmparser::HeapType::Extern => HeapType::Extern, wasmparser::HeapType::TypedFunc(i) => HeapType::TypedFunc(i.into()), - wasmparser::HeapType::Bot => unreachable!(), }, } } diff --git a/crates/wasm-mutate/src/module.rs b/crates/wasm-mutate/src/module.rs index 7f72999dff..1d25e676c0 100644 --- a/crates/wasm-mutate/src/module.rs +++ b/crates/wasm-mutate/src/module.rs @@ -88,7 +88,6 @@ pub fn map_ref_type(tpe: wasmparser::RefType) -> Result { wasmparser::HeapType::Func => HeapType::Func, wasmparser::HeapType::Extern => HeapType::Extern, wasmparser::HeapType::TypedFunc(i) => HeapType::TypedFunc(i.into()), - wasmparser::HeapType::Bot => unreachable!(), }, }) } diff --git a/crates/wasm-mutate/src/mutators/translate.rs b/crates/wasm-mutate/src/mutators/translate.rs index 5ed1dfb027..0705d6199f 100644 --- a/crates/wasm-mutate/src/mutators/translate.rs +++ b/crates/wasm-mutate/src/mutators/translate.rs @@ -205,7 +205,6 @@ pub fn heapty(t: &mut dyn Translator, ty: &wasmparser::HeapType) -> Result { Ok(HeapType::TypedFunc(t.remap(Item::Type, (*i).into())?)) } - wasmparser::HeapType::Bot => unreachable!(), } } diff --git a/crates/wasm-smith/src/core.rs b/crates/wasm-smith/src/core.rs index 4c3a1f3121..1d3702179c 100644 --- a/crates/wasm-smith/src/core.rs +++ b/crates/wasm-smith/src/core.rs @@ -1644,7 +1644,6 @@ fn convert_reftype(ty: wasmparser::RefType) -> RefType { wasmparser::HeapType::Func => wasm_encoder::HeapType::Func, wasmparser::HeapType::Extern => wasm_encoder::HeapType::Extern, wasmparser::HeapType::TypedFunc(i) => wasm_encoder::HeapType::TypedFunc(i.into()), - wasmparser::HeapType::Bot => unreachable!(), }, } } diff --git a/crates/wasmparser/src/readers/core/types.rs b/crates/wasmparser/src/readers/core/types.rs index b86383ba3c..4358e2670c 100644 --- a/crates/wasmparser/src/readers/core/types.rs +++ b/crates/wasmparser/src/readers/core/types.rs @@ -101,8 +101,6 @@ pub enum HeapType { Func, /// From reference types Extern, - /// Special bottom heap type - Bot, } impl ValType { diff --git a/crates/wasmparser/src/validator.rs b/crates/wasmparser/src/validator.rs index 89eb21f27d..4ce0972469 100644 --- a/crates/wasmparser/src/validator.rs +++ b/crates/wasmparser/src/validator.rs @@ -1504,9 +1504,4 @@ mod tests { Ok(()) } - - #[test] - fn valtype_is_small() { - assert_eq!(std::mem::size_of::(), 4); - } } diff --git a/crates/wasmparser/src/validator/core.rs b/crates/wasmparser/src/validator/core.rs index d089131e40..9c873f4e70 100644 --- a/crates/wasmparser/src/validator/core.rs +++ b/crates/wasmparser/src/validator/core.rs @@ -810,7 +810,6 @@ impl Module { // Just check that the index is valid self.func_type_at(type_index.into(), types, offset)?; } - HeapType::Bot => (), } Ok(()) } @@ -860,7 +859,6 @@ impl Module { self.eq_fns(n1, n2, types) } (HeapType::TypedFunc(_), HeapType::Func) => true, - (HeapType::Bot, _) => true, (_, _) => ty1 == ty2, } }; diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs index 89a6c80913..f7be658c75 100644 --- a/crates/wasmparser/src/validator/operators.rs +++ b/crates/wasmparser/src/validator/operators.rs @@ -38,12 +38,12 @@ pub(crate) struct OperatorValidator { pub(crate) features: WasmFeatures, // Temporary storage used during the validation of `br_table`. - br_table_tmp: Vec>, + br_table_tmp: Vec, /// The `control` list is the list of blocks that we're currently in. control: Vec, /// The `operands` is the current type stack. - operands: Vec>, + operands: Vec, /// When local_inits is modified, the relevant index is recorded here to be /// undone when control pops inits: Vec, @@ -140,15 +140,39 @@ struct OperatorValidatorTemp<'validator, 'resources, T> { #[derive(Default)] pub struct OperatorValidatorAllocations { - br_table_tmp: Vec>, + br_table_tmp: Vec, control: Vec, - operands: Vec>, + operands: Vec, local_inits: Vec, inits: Vec, locals_first: Vec, locals_all: Vec<(u32, ValType)>, } +/// Type storage within the validator. +/// +/// This is used to manage the operand stack and notably isn't just `ValType` to +/// handle unreachable code and the "bottom" type. +#[derive(Copy, Clone)] +enum MaybeType { + Bot, + HeapBot, + Type(ValType), +} + +// The validator is pretty performance-sensitive and `MaybeType` is the main +// unit of storage, so assert that it doesn't exceed 4 bytes which is the +// current expected size. +const _: () = { + assert!(std::mem::size_of::() == 4); +}; + +impl From for MaybeType { + fn from(ty: ValType) -> MaybeType { + MaybeType::Type(ty) + } +} + impl OperatorValidator { fn new(features: &WasmFeatures, allocs: OperatorValidatorAllocations) -> Self { let OperatorValidatorAllocations { @@ -278,7 +302,10 @@ impl OperatorValidator { /// /// A `depth` of 0 will refer to the last operand on the stack. pub fn peek_operand_at(&self, depth: usize) -> Option> { - self.operands.iter().rev().nth(depth).copied() + Some(match self.operands.iter().rev().nth(depth)? { + MaybeType::Type(t) => Some(*t), + MaybeType::Bot | MaybeType::HeapBot => None, + }) } /// Returns the number of frames on the control flow stack. @@ -367,7 +394,7 @@ impl<'resources, R: WasmModuleResources> OperatorValidatorTemp<'_, 'resources, R /// Otherwise the push operation always succeeds. fn push_operand(&mut self, ty: T) -> Result<()> where - T: Into>, + T: Into, { let maybe_ty = ty.into(); self.operands.push(maybe_ty); @@ -392,7 +419,7 @@ impl<'resources, R: WasmModuleResources> OperatorValidatorTemp<'_, 'resources, R /// matches `expected`. If `None` is returned then it means that `None` was /// expected and a type was successfully popped, but its exact type is /// indeterminate because the current block is unreachable. - fn pop_operand(&mut self, expected: Option) -> Result> { + fn pop_operand(&mut self, expected: Option) -> Result { // This method is one of the hottest methods in the validator so to // improve codegen this method contains a fast-path success case where // if the top operand on the stack is as expected it's returned @@ -404,15 +431,15 @@ impl<'resources, R: WasmModuleResources> OperatorValidatorTemp<'_, 'resources, R // matched against the state of the world to see if we could actually // pop it. If we shouldn't have popped it then it's passed to the slow // path to get pushed back onto the stack. - let popped = if let Some(actual_ty) = self.operands.pop() { - if actual_ty == expected { + let popped = if let Some(MaybeType::Type(actual_ty)) = self.operands.pop() { + if Some(actual_ty) == expected { if let Some(control) = self.control.last() { if self.operands.len() >= control.height { - return Ok(actual_ty); + return Ok(MaybeType::Type(actual_ty)); } } } - Some(actual_ty) + Some(MaybeType::Type(actual_ty)) } else { None }; @@ -427,15 +454,15 @@ impl<'resources, R: WasmModuleResources> OperatorValidatorTemp<'_, 'resources, R fn _pop_operand( &mut self, expected: Option, - popped: Option>, - ) -> Result> { + popped: Option, + ) -> Result { self.operands.extend(popped); 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 { - None + MaybeType::Bot } else { if self.operands.len() == control.height { let desc = match expected { @@ -450,27 +477,48 @@ impl<'resources, R: WasmModuleResources> OperatorValidatorTemp<'_, 'resources, R self.operands.pop().unwrap() } }; - if let (Some(actual_ty), Some(expected_ty)) = (actual, expected) { - if !self.resources.matches(actual_ty, expected_ty) { - bail!( - self.offset, - "type mismatch: expected {}, found {}", - ty_to_str(expected_ty), - ty_to_str(actual_ty) - ); + 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 `matches` predicate to test if a found type matches + // the expectation. + (MaybeType::Type(actual), expected) => { + if !self.resources.matches(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, + ) => { + bail!( + self.offset, + "type mismatche: expected {}, found heap type", + ty_to_str(expected) + ) + } } } Ok(actual) } - fn pop_ref(&mut self) -> Result { + fn pop_ref(&mut self) -> Result> { match self.pop_operand(None)? { - None => Ok(RefType { - nullable: false, - heap_type: HeapType::Bot, - }), - Some(ValType::Ref(rt)) => Ok(rt), - Some(ty) => bail!( + MaybeType::Bot | MaybeType::HeapBot => Ok(None), + MaybeType::Type(ValType::Ref(rt)) => Ok(Some(rt)), + MaybeType::Type(ty) => bail!( self.offset, "type mismatch: expected ref but found {}", ty_to_str(ty) @@ -967,14 +1015,6 @@ pub fn ty_to_str(ty: ValType) -> &'static str { nullable: true, heap_type: HeapType::TypedFunc(_), }) => "(ref null $type)", - ValType::Ref(RefType { - nullable: true, - heap_type: HeapType::Bot, - }) => "(ref null bot)", - ValType::Ref(RefType { - nullable: false, - heap_type: HeapType::Bot, - }) => "(ref bot)", } } @@ -1255,23 +1295,25 @@ where Ok(()) } fn visit_call_ref(&mut self, hty: HeapType) -> Self::Output { - let rt = self.pop_ref()?; - let expected = RefType { - nullable: true, - heap_type: hty, - }; - if !self - .resources - .matches(ValType::Ref(rt), ValType::Ref(expected)) - { - bail!( - self.offset, - "type mismatch: funcref on stack does not match specified type", - ); + // If `None` is popped then that means a "bottom" type was popped which + // is always considered equivalent to the `hty` tag. + if let Some(rt) = self.pop_ref()? { + let expected = RefType { + nullable: true, + heap_type: hty, + }; + if !self + .resources + .matches(ValType::Ref(rt), ValType::Ref(expected)) + { + bail!( + self.offset, + "type mismatch: funcref on stack does not match specified type", + ); + } } match hty { HeapType::TypedFunc(type_index) => self.check_call_ty(type_index.into())?, - HeapType::Bot => (), _ => bail!( self.offset, "type mismatch: instruction requires function reference type", @@ -1311,30 +1353,37 @@ where self.pop_operand(Some(ValType::I32))?; let ty1 = self.pop_operand(None)?; let ty2 = self.pop_operand(None)?; - fn is_num(ty: Option) -> bool { - matches!( - ty, - Some(ValType::I32) - | Some(ValType::I64) - | Some(ValType::F32) - | Some(ValType::F64) - | Some(ValType::V128) - | None - ) - } - if !is_num(ty1) || !is_num(ty2) { - bail!( - self.offset, - "type mismatch: select only takes integral types" - ) - } - if ty1 != ty2 && ty1.is_some() && ty2.is_some() { - bail!( - self.offset, - "type mismatch: select operands have different types" - ); - } - self.push_operand(ty1.or(ty2))?; + + let ty = match (ty1, ty2) { + // All heap-related types aren't allowed with the `select` + // instruction + (MaybeType::HeapBot, _) + | (_, MaybeType::HeapBot) + | (MaybeType::Type(ValType::Ref(_)), _) + | (_, MaybeType::Type(ValType::Ref(_))) => { + bail!( + self.offset, + "type mismatch: select only takes integral types" + ) + } + + // If one operand is the "bottom" type then whatever the other + // operand is is the result of the `select` + (MaybeType::Bot, t) | (t, MaybeType::Bot) => t, + + // Otherwise these are two integral types and they must match for + // `select` to typecheck. + (t @ MaybeType::Type(t1), MaybeType::Type(t2)) => { + if t1 != t2 { + bail!( + self.offset, + "type mismatch: select operands have different types" + ); + } + t + } + }; + self.push_operand(ty)?; Ok(()) } fn visit_typed_select(&mut self, ty: ValType) -> Self::Output { @@ -2199,15 +2248,24 @@ where } fn visit_ref_as_non_null(&mut self) -> Self::Output { - let rt = self.pop_ref()?; - self.push_operand(ValType::Ref(RefType { - nullable: false, - heap_type: rt.heap_type, - }))?; + let ty = match self.pop_ref()? { + Some(ty) => MaybeType::Type(ValType::Ref(RefType { + nullable: false, + heap_type: ty.heap_type, + })), + None => MaybeType::HeapBot, + }; + self.push_operand(ty)?; Ok(()) } fn visit_br_on_null(&mut self, relative_depth: u32) -> Self::Output { - let rt = self.pop_ref()?; + let ty = match self.pop_ref()? { + None => MaybeType::HeapBot, + Some(ty) => MaybeType::Type(ValType::Ref(RefType { + nullable: false, + heap_type: ty.heap_type, + })), + }; let (ft, kind) = self.jump(relative_depth)?; for ty in self.label_types(ft, kind)?.rev() { self.pop_operand(Some(ty))?; @@ -2215,41 +2273,39 @@ where for ty in self.label_types(ft, kind)? { self.push_operand(ty)?; } - self.push_operand(ValType::Ref(RefType { - nullable: false, - heap_type: rt.heap_type, - }))?; + self.push_operand(ty)?; Ok(()) } fn visit_br_on_non_null(&mut self, relative_depth: u32) -> Self::Output { - let RefType { heap_type, .. } = self.pop_ref()?; - let rt0 = ValType::Ref(RefType { - nullable: false, - heap_type: heap_type, - }); + let ty = self.pop_ref()?; let (ft, kind) = self.jump(relative_depth)?; let mut lts = self.label_types(ft, kind)?; - match lts.next_back() { - None => bail!( + match (lts.next_back(), ty) { + (None, _) => bail!( self.offset, - "type mismatch: expected {} but stack has nothing", - ty_to_str(rt0) + "type mismatch: br_on_non_null target has no label types", ), - Some(rt1 @ ValType::Ref(_)) => { - if !self.resources.matches(rt0, rt1) { + (Some(ValType::Ref(_)), None) => {} + (Some(rt1 @ ValType::Ref(_)), Some(rt0)) => { + // Switch rt0, our popped type, to a non-nullable type and + // perform the match because if the branch is taken it's a + // non-null value. + let ty = RefType { + nullable: false, + heap_type: rt0.heap_type, + }; + if !self.resources.matches(ty.into(), rt1) { bail!( self.offset, "type mismatch: expected {} but found {}", - ty_to_str(rt0), + ty_to_str(rt0.into()), ty_to_str(rt1) ) } } - Some(ty) => bail!( + (Some(_), _) => bail!( self.offset, - "type mismatch: expected {} but found {}", - ty_to_str(rt0), - ty_to_str(ty) + "type mismatch: br_on_non_null target does not end with heap type", ), } for ty in self.label_types(ft, kind)?.rev().skip(1) { diff --git a/crates/wasmprinter/src/lib.rs b/crates/wasmprinter/src/lib.rs index d5d8e3dc2a..9083fbaabf 100644 --- a/crates/wasmprinter/src/lib.rs +++ b/crates/wasmprinter/src/lib.rs @@ -713,7 +713,6 @@ impl Printer { HeapType::Func => self.result.push_str("func"), HeapType::Extern => self.result.push_str("extern"), HeapType::TypedFunc(i) => self.result.push_str(&format!("{}", u32::from(i))), - HeapType::Bot => self.result.push_str("bot"), } Ok(()) } diff --git a/crates/wit-component/src/gc.rs b/crates/wit-component/src/gc.rs index 7768a00032..35e51b948a 100644 --- a/crates/wit-component/src/gc.rs +++ b/crates/wit-component/src/gc.rs @@ -424,7 +424,6 @@ impl<'a> Module<'a> { match ty { HeapType::Func | HeapType::Extern => {} HeapType::TypedFunc(i) => self.ty(i.into()), - HeapType::Bot => unreachable!(), } } @@ -1012,7 +1011,6 @@ impl Encoder { wasmparser::HeapType::TypedFunc(idx) => { wasm_encoder::HeapType::TypedFunc(self.types.remap(idx.into()).try_into().unwrap()) } - wasmparser::HeapType::Bot => unimplemented!(), } } }