From 1d033340e16b91d794d33ecac9b83557742835ca Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Fri, 23 Sep 2022 08:11:00 -0700 Subject: [PATCH] wasmparser: make `TypeId` hash differently for aliased types. (#775) * Fix warnings in `wasmparser`. * wasmparser: add type index to `TypeId`. This commit adds the original type index (if there is one) to `TypeId`. The intention for doing so is making `TypeId` `Hash` and `PartialEq` implementations take into account type aliases. * wasmparser: add a test for `TypeId` aliasing. --- crates/wasmparser/benches/benchmark.rs | 10 +---- crates/wasmparser/src/readers/core/types.rs | 2 +- crates/wasmparser/src/validator.rs | 41 ++++++++++++++++++++ crates/wasmparser/src/validator/component.rs | 34 +++++++++++++++- crates/wasmparser/src/validator/core.rs | 2 + crates/wasmparser/src/validator/operators.rs | 2 +- crates/wasmparser/src/validator/types.rs | 8 ++++ 7 files changed, 87 insertions(+), 12 deletions(-) diff --git a/crates/wasmparser/benches/benchmark.rs b/crates/wasmparser/benches/benchmark.rs index ad82213cae..7da82f9ec2 100644 --- a/crates/wasmparser/benches/benchmark.rs +++ b/crates/wasmparser/benches/benchmark.rs @@ -1,16 +1,10 @@ -#[macro_use] -extern crate criterion; - use anyhow::Result; -use criterion::Criterion; +use criterion::{criterion_group, criterion_main, Criterion}; use once_cell::unsync::Lazy; use std::fs; use std::path::Path; use std::path::PathBuf; -use wasmparser::{ - BlockType, BrTable, DataKind, ElementKind, Ieee32, Ieee64, MemArg, Parser, Payload, ValType, - Validator, VisitOperator, WasmFeatures, V128, -}; +use wasmparser::{DataKind, ElementKind, Parser, Payload, Validator, VisitOperator, WasmFeatures}; /// A benchmark input. pub struct BenchmarkInput { diff --git a/crates/wasmparser/src/readers/core/types.rs b/crates/wasmparser/src/readers/core/types.rs index 974c197321..cb6d2e80af 100644 --- a/crates/wasmparser/src/readers/core/types.rs +++ b/crates/wasmparser/src/readers/core/types.rs @@ -58,7 +58,7 @@ pub enum Type { pub struct FuncType { /// The combined parameters and result types. params_results: Box<[ValType]>, - /// The number of paramter types. + /// The number of parameter types. len_params: usize, } diff --git a/crates/wasmparser/src/validator.rs b/crates/wasmparser/src/validator.rs index 923890d30b..724cb96608 100644 --- a/crates/wasmparser/src/validator.rs +++ b/crates/wasmparser/src/validator.rs @@ -1426,4 +1426,45 @@ mod tests { Ok(()) } + + #[test] + fn test_type_id_aliasing() -> Result<()> { + let bytes = wat::parse_str( + r#" + (component + (type $T (list string)) + (alias outer 0 $T (type $A1)) + (alias outer 0 $T (type $A2)) + ) + "#, + )?; + + let mut validator = Validator::new_with_features(WasmFeatures { + component_model: true, + ..Default::default() + }); + + let types = validator.validate_all(&bytes)?; + + let t_id = types.id_from_type_index(0, false).unwrap(); + let a1_id = types.id_from_type_index(1, false).unwrap(); + let a2_id = types.id_from_type_index(2, false).unwrap(); + + // The ids should all be different + assert!(t_id != a1_id); + assert!(t_id != a2_id); + assert!(a1_id != a2_id); + + // However, they should all point to the same type + assert!(std::ptr::eq( + types.type_from_id(t_id).unwrap(), + types.type_from_id(a1_id).unwrap() + )); + assert!(std::ptr::eq( + types.type_from_id(t_id).unwrap(), + types.type_from_id(a2_id).unwrap() + )); + + Ok(()) + } } diff --git a/crates/wasmparser/src/validator/component.rs b/crates/wasmparser/src/validator/component.rs index 42c282cae2..b2acb467bf 100644 --- a/crates/wasmparser/src/validator/component.rs +++ b/crates/wasmparser/src/validator/component.rs @@ -87,6 +87,8 @@ impl ComponentState { current.core_types.push(TypeId { type_size: ty.type_size(), index: types.len(), + type_index: Some(current.core_types.len()), + is_core: true, }); types.push(ty); @@ -113,6 +115,8 @@ impl ComponentState { self.core_modules.push(TypeId { type_size: ty.type_size(), index: types.len(), + type_index: None, + is_core: true, }); types.push(ty); @@ -182,6 +186,8 @@ impl ComponentState { current.types.push(TypeId { type_size: ty.type_size(), index: types.len(), + type_index: Some(current.types.len()), + is_core: false, }); types.push(ty); @@ -327,6 +333,8 @@ impl ComponentState { self.core_funcs.push(TypeId { type_size: lowered_ty.type_size(), index: types.len(), + type_index: None, + is_core: true, }); types.push(lowered_ty); @@ -344,6 +352,8 @@ impl ComponentState { self.components.push(TypeId { type_size: ty.type_size(), index: types.len(), + type_index: None, + is_core: false, }); types.push(ty); @@ -1009,6 +1019,8 @@ impl ComponentState { let id = TypeId { type_size: ty.type_size(), index: types.len(), + type_index: None, + is_core: true, }; types.push(ty); @@ -1144,6 +1156,8 @@ impl ComponentState { let id = TypeId { type_size: ty.type_size(), index: types.len(), + type_index: None, + is_core: false, }; types.push(ty); @@ -1245,6 +1259,8 @@ impl ComponentState { let id = TypeId { type_size: ty.type_size(), index: types.len(), + type_index: None, + is_core: false, }; types.push(ty); @@ -1331,6 +1347,8 @@ impl ComponentState { let id = TypeId { type_size: ty.type_size(), index: types.len(), + type_index: None, + is_core: true, }; types.push(ty); @@ -1544,7 +1562,13 @@ impl ComponentState { let current = components.last_mut().unwrap(); check_max(current.type_count(), 1, MAX_WASM_TYPES, "types", offset)?; - current.core_types.push(ty); + current.core_types.push(TypeId { + type_size: ty.type_size, + index: ty.index, + type_index: Some(current.core_types.len()), + is_core: true, + }); + Ok(()) } @@ -1555,7 +1579,13 @@ impl ComponentState { let current = components.last_mut().unwrap(); check_max(current.type_count(), 1, MAX_WASM_TYPES, "types", offset)?; - current.types.push(ty); + current.types.push(TypeId { + type_size: ty.type_size, + index: ty.index, + type_index: Some(current.types.len()), + is_core: false, + }); + Ok(()) } diff --git a/crates/wasmparser/src/validator/core.rs b/crates/wasmparser/src/validator/core.rs index 933ea49555..956e50ee05 100644 --- a/crates/wasmparser/src/validator/core.rs +++ b/crates/wasmparser/src/validator/core.rs @@ -481,6 +481,8 @@ impl Module { self.types.push(TypeId { type_size: ty.type_size(), index: types.len(), + type_index: Some(self.types.len()), + is_core: true, }); types.push(ty); Ok(()) diff --git a/crates/wasmparser/src/validator/operators.rs b/crates/wasmparser/src/validator/operators.rs index d2d6e40831..3d9b481a06 100644 --- a/crates/wasmparser/src/validator/operators.rs +++ b/crates/wasmparser/src/validator/operators.rs @@ -324,7 +324,7 @@ impl Deref for OperatorValidatorTemp<'_, '_, R> { impl DerefMut for OperatorValidatorTemp<'_, '_, R> { fn deref_mut(&mut self) -> &mut OperatorValidator { - &mut self.inner + self.inner } } diff --git a/crates/wasmparser/src/validator/types.rs b/crates/wasmparser/src/validator/types.rs index d007f71131..a20d399ae6 100644 --- a/crates/wasmparser/src/validator/types.rs +++ b/crates/wasmparser/src/validator/types.rs @@ -141,6 +141,14 @@ pub struct TypeId { pub(crate) type_size: usize, /// The index into the global list of types. pub(crate) index: usize, + /// The original type index. + /// + /// This will be `None` for implicitly defined types, e.g. types for + /// modules definitions, component definitions, instantiations, and function + /// lowerings. + pub(crate) type_index: Option, + /// Whether or not the type is a core type. + pub(crate) is_core: bool, } /// A unified type definition for validating WebAssembly modules and components.