Skip to content

Commit

Permalink
Share more core type infrastructure in components (#1765)
Browse files Browse the repository at this point in the history 
This commit chiefly fixes the validator panic found in #1763. This is
done by sharing more infrastructure when validating types between the
core module validator and the component validator. This sharing is done
by extracting a new trait and moving methods from the core module to the
trait and then implementing the trait for both component and core
contexts.

When writing tests this additionally discovered that name resolution on
core types wasn't happening within components. The fix there was the
same as the core module validator which was to extract shared bits to a
trait and then implement the trait in both locations.

Closes #1763
  • Loading branch information
@alexcrichton
alexcrichton authored Sep 9, 2024
1 parent b1766fb commit 50f63d4
Show file tree
Hide file tree
Showing 12 changed files with 472 additions and 288 deletions.
48 changes: 34 additions & 14 deletions crates/wasmparser/src/validator/component.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use super::{
check_max,
core::Module,
core::{InternRecGroup, Module},
types::{
AliasableResourceId, ComponentCoreInstanceTypeId, ComponentDefinedTypeId,
ComponentFuncType, ComponentFuncTypeId, ComponentInstanceType, ComponentInstanceTypeId,
Expand Down Expand Up @@ -261,12 +261,18 @@ impl ComponentState {
offset: usize,
check_limit: bool,
) -> Result<()> {
let id = match ty {
let current = components.last_mut().unwrap();
if check_limit {
check_max(current.type_count(), 1, MAX_WASM_TYPES, "types", offset)?;
}
match ty {
crate::CoreType::Sub(sub) => {
let (_is_new, group_id) =
types.intern_canonical_rec_group(RecGroup::implicit(offset, sub));
let id = types[group_id].start;
ComponentCoreTypeId::Sub(id)
current.canonicalize_and_intern_rec_group(
features,
types,
RecGroup::implicit(offset, sub),
offset,
)?;
}
crate::CoreType::Module(decls) => {
let mod_ty = Self::create_module_type(
Expand All @@ -276,16 +282,11 @@ impl ComponentState {
types,
offset,
)?;
let id = types.push_ty(mod_ty);
ComponentCoreTypeId::Module(id)
let id = ComponentCoreTypeId::Module(types.push_ty(mod_ty));
components.last_mut().unwrap().core_types.push(id);
}
};

let current = components.last_mut().unwrap();
if check_limit {
check_max(current.type_count(), 1, MAX_WASM_TYPES, "types", offset)?;
}
current.core_types.push(id);

Ok(())
}

Expand Down Expand Up @@ -3036,6 +3037,25 @@ impl ComponentState {
}
}

impl InternRecGroup for ComponentState {
fn add_type_id(&mut self, id: CoreTypeId) {
self.core_types.push(ComponentCoreTypeId::Sub(id));
}

fn type_id_at(&self, idx: u32, offset: usize) -> Result<CoreTypeId> {
match self.core_type_at(idx, offset)? {
ComponentCoreTypeId::Sub(id) => Ok(id),
ComponentCoreTypeId::Module(_) => {
bail!(offset, "type index {idx} is a module type, not a sub type");
}
}
}

fn types_len(&self) -> u32 {
u32::try_from(self.core_types.len()).unwrap()
}
}

impl ComponentNameContext {
/// Registers that the resource `id` is named `name` within this context.
fn register(&mut self, name: &str, id: AliasableResourceId) {
Expand Down
207 changes: 23 additions & 184 deletions crates/wasmparser/src/validator/core.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@
//!
mod canonical;
pub(crate) use canonical::InternRecGroup;

use self::{arc::MaybeOwned, canonical::canonicalize_and_intern_rec_group};
use self::arc::MaybeOwned;
use super::{
check_max, combine_type_sizes,
operators::{ty_to_str, OperatorValidator, OperatorValidatorAllocations},
types::{CoreTypeId, EntityType, RecGroupId, TypeAlloc, TypeList},
};
use crate::{
limits::*, validator::types::TypeIdentifier, BinaryReaderError, CompositeType, ConstExpr, Data,
DataKind, Element, ElementKind, ExternalKind, FuncType, Global, GlobalType, HeapType,
MemoryType, PackedIndex, RecGroup, RefType, Result, StorageType, SubType, Table, TableInit,
TableType, TagType, TypeRef, UnpackedIndex, ValType, VisitOperator, WasmFeatures,
limits::*, BinaryReaderError, ConstExpr, Data, DataKind, Element, ElementKind, ExternalKind,
FuncType, Global, GlobalType, HeapType, MemoryType, RecGroup, RefType, Result, SubType, Table,
TableInit, TableType, TagType, TypeRef, UnpackedIndex, ValType, VisitOperator, WasmFeatures,
WasmModuleResources,
};
use crate::{prelude::*, CompositeInnerType};
Expand Down Expand Up @@ -552,21 +552,6 @@ pub(crate) struct Module {
}

impl Module {
/// Get the `CoreTypeId` of the type at the given packed index.
pub(crate) fn at_packed_index(
&self,
types: &TypeList,
rec_group: RecGroupId,
index: PackedIndex,
offset: usize,
) -> Result<CoreTypeId> {
match index.unpack() {
UnpackedIndex::Id(id) => Ok(id),
UnpackedIndex::Module(idx) => self.type_id_at(idx, offset),
UnpackedIndex::RecGroup(idx) => types.rec_group_local_id(rec_group, idx, offset),
}
}

pub fn add_types(
&mut self,
rec_group: RecGroup,
Expand All @@ -575,14 +560,6 @@ impl Module {
offset: usize,
check_limit: bool,
) -> Result<()> {
debug_assert!(rec_group.is_explicit_rec_group() || rec_group.types().len() == 1);
if rec_group.is_explicit_rec_group() && !features.gc() {
bail!(
offset,
"rec group usage requires `gc` proposal to be enabled"
);
}

if check_limit {
check_max(
self.types.len(),
Expand All @@ -592,155 +569,7 @@ impl Module {
offset,
)?;
}

let (is_new, rec_group_id) =
canonicalize_and_intern_rec_group(features, types, self, rec_group, offset)?;

let range = &types[rec_group_id];
let start = range.start.index();
let end = range.end.index();

for i in start..end {
let i = u32::try_from(i).unwrap();
let id = CoreTypeId::from_index(i);
debug_assert!(types.get(id).is_some());
self.types.push(id);
if is_new {
self.check_subtype(rec_group_id, id, features, types, offset)?;
}
}

Ok(())
}

fn check_subtype(
&mut self,
rec_group: RecGroupId,
id: CoreTypeId,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
) -> Result<()> {
let ty = &types[id];
if !features.gc() && (!ty.is_final || ty.supertype_idx.is_some()) {
bail!(offset, "gc proposal must be enabled to use subtypes");
}

self.check_composite_type(&ty.composite_type, features, &types, offset)?;

let depth = if let Some(supertype_index) = ty.supertype_idx {
debug_assert!(supertype_index.is_canonical());
let sup_id = self.at_packed_index(types, rec_group, supertype_index, offset)?;
if types[sup_id].is_final {
bail!(offset, "sub type cannot have a final super type");
}
if !types.matches(id, sup_id) {
bail!(offset, "sub type must match super type");
}
let depth = types.get_subtyping_depth(sup_id) + 1;
if usize::from(depth) > crate::limits::MAX_WASM_SUBTYPING_DEPTH {
bail!(
offset,
"sub type hierarchy too deep: found depth {}, cannot exceed depth {}",
depth,
crate::limits::MAX_WASM_SUBTYPING_DEPTH,
);
}
depth
} else {
0
};
types.set_subtyping_depth(id, depth);

Ok(())
}

fn check_composite_type(
&mut self,
ty: &CompositeType,
features: &WasmFeatures,
types: &TypeList,
offset: usize,
) -> Result<()> {
let check = |ty: &ValType, shared: bool| {
features
.check_value_type(*ty)
.map_err(|e| BinaryReaderError::new(e, offset))?;
if shared && !types.valtype_is_shared(*ty) {
return Err(BinaryReaderError::new(
"shared composite type must contain shared types",
offset,
));
// The other cases are fine:
// - both shared or unshared: good to go
// - the func type is unshared, `ty` is shared: though
// odd, we _can_ in fact use shared values in
// unshared composite types (e.g., functions).
}
Ok(())
};
if !features.shared_everything_threads() && ty.shared {
return Err(BinaryReaderError::new(
"shared composite types require the shared-everything-threads proposal",
offset,
));
}
match &ty.inner {
CompositeInnerType::Func(t) => {
for vt in t.params().iter().chain(t.results()) {
check(vt, ty.shared)?;
}
if t.results().len() > 1 && !features.multi_value() {
return Err(BinaryReaderError::new(
"func type returns multiple values but the multi-value feature is not enabled",
offset,
));
}
}
CompositeInnerType::Array(t) => {
if !features.gc() {
bail!(
offset,
"array indexed types not supported without the gc feature",
);
}
if !features.gc_types() {
bail!(
offset,
"cannot define array types when gc types are disabled",
);
}
match &t.0.element_type {
StorageType::I8 | StorageType::I16 => {
// Note: scalar types are always `shared`.
}
StorageType::Val(value_type) => check(value_type, ty.shared)?,
};
}
CompositeInnerType::Struct(t) => {
if !features.gc() {
bail!(
offset,
"struct indexed types not supported without the gc feature",
);
}
if !features.gc_types() {
bail!(
offset,
"cannot define struct types when gc types are disabled",
);
}
for ft in t.fields.iter() {
match &ft.element_type {
StorageType::I8 | StorageType::I16 => {
// Note: scalar types are always `shared`.
}
StorageType::Val(value_type) => check(value_type, ty.shared)?,
}
}
}
}
Ok(())
self.canonicalize_and_intern_rec_group(features, types, rec_group, offset)
}

pub fn add_import(
Expand Down Expand Up @@ -859,13 +688,6 @@ impl Module {
Ok(())
}

pub fn type_id_at(&self, idx: u32, offset: usize) -> Result<CoreTypeId> {
self.types
.get(idx as usize)
.copied()
.ok_or_else(|| format_err!(offset, "unknown type {idx}: type index out of bounds"))
}

fn sub_type_at<'a>(&self, types: &'a TypeList, idx: u32, offset: usize) -> Result<&'a SubType> {
let id = self.type_id_at(idx, offset)?;
Ok(&types[id])
Expand Down Expand Up @@ -1276,6 +1098,23 @@ impl Module {
}
}

impl InternRecGroup for Module {
fn add_type_id(&mut self, id: CoreTypeId) {
self.types.push(id);
}

fn type_id_at(&self, idx: u32, offset: usize) -> Result<CoreTypeId> {
self.types
.get(idx as usize)
.copied()
.ok_or_else(|| format_err!(offset, "unknown type {idx}: type index out of bounds"))
}

fn types_len(&self) -> u32 {
u32::try_from(self.types.len()).unwrap()
}
}

impl Default for Module {
fn default() -> Self {
Self {
Expand Down
Loading

0 comments on commit 50f63d4

Please sign in to comment.