Skip to content

Commit

Permalink
Move component value support behind a feature flag (#1131)
Browse files Browse the repository at this point in the history
* Move component `value` support behind a feature flag

Support for the `value` type in the component model is somewhat
best-effort right now in implementations like Wasmtime's. Additionally
there's open questions in the upstream specification about what to do
with `value`, especially around `post-return` and ABI details. For now
this adds a separate feature gate for the `value` type to have
first-class errors about its usage in the binary encoding rather than
possible panics in Wasmtime, for example.

* Update fuzzers

* Fix benchmark build
  • Loading branch information
alexcrichton authored Jul 17, 2023
1 parent b19ce51 commit cea2220
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 20 deletions.
1 change: 1 addition & 0 deletions crates/wasm-shrink/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ impl ShrinkRun {
component_model: false,
function_references: false,
gc: false,
component_model_values: false,

floats: true,
memory_control: true,
Expand Down
1 change: 1 addition & 0 deletions crates/wasm-smith/tests/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ fn parser_features_from_config(config: &impl Config) -> WasmFeatures {
function_references: false,
memory_control: false,
gc: false,
component_model_values: false,
}
}

Expand Down
1 change: 1 addition & 0 deletions crates/wasmparser/benches/benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ fn define_benchmarks(c: &mut Criterion) {
function_references: true,
memory_control: true,
gc: true,
component_model_values: true,
})
}

Expand Down
21 changes: 13 additions & 8 deletions crates/wasmparser/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ pub struct WasmFeatures {
pub memory_control: bool,
/// The WebAssembly gc proposal
pub gc: bool,
/// Support for the `value` type in the component model proposal.
pub component_model_values: bool,
}

impl WasmFeatures {
Expand Down Expand Up @@ -336,6 +338,7 @@ impl Default for WasmFeatures {
function_references: false,
memory_control: false,
gc: false,
component_model_values: false,

// On-by-default features (phase 4 or greater).
mutable_global: true,
Expand Down Expand Up @@ -1062,11 +1065,11 @@ impl Validator {
current.instances.reserve(count as usize);
Ok(())
},
|components, types, _, instance, offset| {
|components, types, features, instance, offset| {
components
.last_mut()
.unwrap()
.add_instance(instance, types, offset)
.add_instance(instance, features, types, offset)
},
)
}
Expand All @@ -1082,8 +1085,8 @@ impl Validator {
section,
"alias",
|_, _, _, _| Ok(()), // maximums checked via `add_alias`
|components, types, _, alias, offset| -> Result<(), BinaryReaderError> {
ComponentState::add_alias(components, alias, types, offset)
|components, types, features, alias, offset| -> Result<(), BinaryReaderError> {
ComponentState::add_alias(components, alias, features, types, offset)
},
)
}
Expand Down Expand Up @@ -1181,6 +1184,7 @@ impl Validator {
f.func_index,
&f.arguments,
f.results,
&self.features,
&self.types,
range.start,
)
Expand All @@ -1197,11 +1201,11 @@ impl Validator {
section,
"import",
|_, _, _, _| Ok(()), // add_import will check limits
|components, types, _, import, offset| {
|components, types, features, import, offset| {
components
.last_mut()
.unwrap()
.add_import(import, types, offset)
.add_import(import, features, types, offset)
},
)
}
Expand All @@ -1228,12 +1232,13 @@ impl Validator {
current.exports.reserve(count as usize);
Ok(())
},
|components, types, _, export, offset| {
|components, types, features, export, offset| {
let current = components.last_mut().unwrap();
let ty = current.export_to_entity_type(&export, types, offset)?;
let ty = current.export_to_entity_type(&export, features, types, offset)?;
current.add_export(
export.name,
ty,
features,
types,
offset,
false, /* checked above */
Expand Down
68 changes: 56 additions & 12 deletions crates/wasmparser/src/validator/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,15 @@ impl ComponentState {
pub fn add_import(
&mut self,
import: crate::ComponentImport,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
) -> Result<()> {
let mut entity = self.check_type_ref(&import.ty, types, offset)?;
let mut entity = self.check_type_ref(&import.ty, features, types, offset)?;
self.add_entity(
&mut entity,
Some((import.name.as_str(), ExternKind::Import)),
features,
types,
offset,
)?;
Expand All @@ -437,6 +439,7 @@ impl ComponentState {
&mut self,
ty: &mut ComponentEntityType,
name_and_kind: Option<(&str, ExternKind)>,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
) -> Result<()> {
Expand Down Expand Up @@ -464,6 +467,7 @@ impl ComponentState {
(self.function_count(), MAX_WASM_FUNCTIONS, "functions")
}
ComponentEntityType::Value(ty) => {
self.check_value_support(features, offset)?;
let value_used = match kind {
Some(ExternKind::Import) | None => false,
Some(ExternKind::Export) => true,
Expand Down Expand Up @@ -868,6 +872,7 @@ impl ComponentState {
&mut self,
name: ComponentExternName<'_>,
mut ty: ComponentEntityType,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
check_limit: bool,
Expand All @@ -878,6 +883,7 @@ impl ComponentState {
self.add_entity(
&mut ty,
Some((name.as_str(), ExternKind::Export)),
features,
types,
offset,
)?;
Expand Down Expand Up @@ -1037,16 +1043,23 @@ impl ComponentState {
pub fn add_instance(
&mut self,
instance: crate::ComponentInstance,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
) -> Result<()> {
let instance = match instance {
crate::ComponentInstance::Instantiate {
component_index,
args,
} => self.instantiate_component(component_index, args.into_vec(), types, offset)?,
} => self.instantiate_component(
component_index,
args.into_vec(),
features,
types,
offset,
)?,
crate::ComponentInstance::FromExports(exports) => {
self.instantiate_exports(exports.into_vec(), types, offset)?
self.instantiate_exports(exports.into_vec(), features, types, offset)?
}
};

Expand All @@ -1058,6 +1071,7 @@ impl ComponentState {
pub fn add_alias(
components: &mut [Self],
alias: crate::ComponentAlias,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
) -> Result<()> {
Expand All @@ -1070,6 +1084,7 @@ impl ComponentState {
instance_index,
kind,
name,
features,
types,
offset,
),
Expand Down Expand Up @@ -1106,9 +1121,16 @@ impl ComponentState {
func_index: u32,
args: &[u32],
results: u32,
features: &WasmFeatures,
types: &TypeList,
offset: usize,
) -> Result<()> {
if !features.component_model_values {
bail!(
offset,
"support for component model `value`s is not enabled"
);
}
if self.has_start {
return Err(BinaryReaderError::new(
"component cannot have more than one start function",
Expand Down Expand Up @@ -1281,6 +1303,7 @@ impl ComponentState {
fn check_type_ref(
&mut self,
ty: &ComponentTypeRef,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
) -> Result<ComponentEntityType> {
Expand All @@ -1302,6 +1325,7 @@ impl ComponentState {
ComponentEntityType::Func(id)
}
ComponentTypeRef::Value(ty) => {
self.check_value_support(features, offset)?;
let ty = match ty {
crate::ComponentValType::Primitive(ty) => ComponentValType::Primitive(*ty),
crate::ComponentValType::Type(index) => {
Expand Down Expand Up @@ -1348,6 +1372,7 @@ impl ComponentState {
pub fn export_to_entity_type(
&mut self,
export: &crate::ComponentExport,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
) -> Result<ComponentEntityType> {
Expand All @@ -1359,6 +1384,7 @@ impl ComponentState {
ComponentEntityType::Func(self.function_at(export.index, offset)?)
}
ComponentExternalKind::Value => {
self.check_value_support(features, offset)?;
ComponentEntityType::Value(*self.value_at(export.index, offset)?)
}
ComponentExternalKind::Type => {
Expand All @@ -1378,7 +1404,7 @@ impl ComponentState {
};

let ascribed = match &export.ty {
Some(ty) => self.check_type_ref(ty, types, offset)?,
Some(ty) => self.check_type_ref(ty, features, types, offset)?,
None => return Ok(actual),
};

Expand Down Expand Up @@ -1466,17 +1492,17 @@ impl ComponentState {
}
crate::ComponentTypeDeclaration::Export { name, ty } => {
let current = components.last_mut().unwrap();
let ty = current.check_type_ref(&ty, types, offset)?;
current.add_export(name, ty, types, offset, true)?;
let ty = current.check_type_ref(&ty, features, types, offset)?;
current.add_export(name, ty, features, types, offset, true)?;
}
crate::ComponentTypeDeclaration::Import(import) => {
components
.last_mut()
.unwrap()
.add_import(import, types, offset)?;
.add_import(import, features, types, offset)?;
}
crate::ComponentTypeDeclaration::Alias(alias) => {
Self::add_alias(components, alias, types, offset)?;
Self::add_alias(components, alias, features, types, offset)?;
}
};
}
Expand All @@ -1503,11 +1529,11 @@ impl ComponentState {
}
crate::InstanceTypeDeclaration::Export { name, ty } => {
let current = components.last_mut().unwrap();
let ty = current.check_type_ref(&ty, types, offset)?;
current.add_export(name, ty, types, offset, true)?;
let ty = current.check_type_ref(&ty, features, types, offset)?;
current.add_export(name, ty, features, types, offset, true)?;
}
crate::InstanceTypeDeclaration::Alias(alias) => {
Self::add_alias(components, alias, types, offset)?;
Self::add_alias(components, alias, features, types, offset)?;
}
};
}
Expand Down Expand Up @@ -1690,6 +1716,7 @@ impl ComponentState {
&mut self,
component_index: u32,
component_args: Vec<crate::ComponentInstantiationArg>,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
) -> Result<TypeId> {
Expand All @@ -1712,6 +1739,7 @@ impl ComponentState {
ComponentEntityType::Func(self.function_at(component_arg.index, offset)?)
}
ComponentExternalKind::Value => {
self.check_value_support(features, offset)?;
ComponentEntityType::Value(*self.value_at(component_arg.index, offset)?)
}
ComponentExternalKind::Type => {
Expand Down Expand Up @@ -1957,6 +1985,7 @@ impl ComponentState {
fn instantiate_exports(
&mut self,
exports: Vec<crate::ComponentExport>,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
) -> Result<TypeId> {
Expand Down Expand Up @@ -2003,6 +2032,7 @@ impl ComponentState {
ComponentEntityType::Func(self.function_at(export.index, offset)?)
}
ComponentExternalKind::Value => {
self.check_value_support(features, offset)?;
ComponentEntityType::Value(*self.value_at(export.index, offset)?)
}
ComponentExternalKind::Type => {
Expand Down Expand Up @@ -2229,9 +2259,13 @@ impl ComponentState {
instance_index: u32,
kind: ComponentExternalKind,
name: &str,
features: &WasmFeatures,
types: &mut TypeAlloc,
offset: usize,
) -> Result<()> {
if let ComponentExternalKind::Value = kind {
self.check_value_support(features, offset)?;
}
let mut ty = match types[self.instance_at(instance_index, offset)?]
.unwrap_component_instance()
.exports
Expand Down Expand Up @@ -2266,7 +2300,7 @@ impl ComponentState {
);
}

self.add_entity(&mut ty, None, types, offset)?;
self.add_entity(&mut ty, None, features, types, offset)?;
Ok(())
}

Expand Down Expand Up @@ -2869,6 +2903,16 @@ impl ComponentState {

Ok(ty)
}

fn check_value_support(&self, features: &WasmFeatures, offset: usize) -> Result<()> {
if !features.component_model_values {
bail!(
offset,
"support for component model `value`s is not enabled"
);
}
Ok(())
}
}

impl KebabNameContext {
Expand Down
1 change: 1 addition & 0 deletions fuzz/fuzz_targets/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ fuzz_target!(|data: &[u8]| {
memory_control: (byte3 & 0b0000_0001) != 0,
function_references: (byte3 & 0b0000_0010) != 0,
gc: (byte3 & 0b0000_0100) != 0,
component_model_values: (byte3 & 0b0000_1000) != 0,
});
let use_maybe_invalid = byte3 & 0b0000_1000 != 0;

Expand Down
1 change: 1 addition & 0 deletions src/bin/wasm-tools/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ fn parse_features(arg: &str) -> Result<WasmFeatures> {
("multi-value", |f| &mut f.multi_value),
("tail-call", |f| &mut f.tail_call),
("component-model", |f| &mut f.component_model),
("component-model-values", |f| &mut f.component_model_values),
("multi-memory", |f| &mut f.multi_memory),
("exception-handling", |f| &mut f.exceptions),
("memory64", |f| &mut f.memory64),
Expand Down
Loading

0 comments on commit cea2220

Please sign in to comment.