From 62e51da588be413918de7ce2ec63536eb66d92ee Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 22 Aug 2024 10:01:36 -0500 Subject: [PATCH] Update handling of wasm features and `*.wast` testing (#9158) * Update handling of wasm features and `*.wast` testing This commit is an attempt at making it easier to manage the set of variables we have in play with `*.wast` testing and wasm features. The Cranelift and Winch backends support different sets of wasm features at this time and historically Cranelift has also had architecture-specific support for wasm features. This is hoped to help simplify management of all of this in the future in addition to making `*.wast` testing more robust. Notable changes here are: * A `Config` no longer tracks a `WasmFeatures` but instead only tracks explicitly disabled/enabled features. The final `WasmFeatures` is then computed given the result of compiler configuration. * Default-enabled features now start from nothing instead of `wasmparser`'s defaults to avoid future breakage there. * Each compiler configuration and/or backend now has a listed set of "this feature may panic" wasm features. These are all disabled by default and if explicitly enabled a `Config::validate` error is returned. * All `*.wast` tests are now run through both Cranelift and Winch. Tests are now asserted to either fail or pass depending on configuration and the whole test will fail if the result doesn't match the expectation. * Many `gc` proposal tests which are now passing are flagged as passing now. Failing `winch` tests are now explicitly listed instead of using a litany of patterns. * Change panicking features for winch * Fix builds without a compiler --- cranelift/wasm/src/code_translator.rs | 10 +- crates/wasmtime/src/compile.rs | 4 +- crates/wasmtime/src/compile/code_builder.rs | 2 +- crates/wasmtime/src/config.rs | 257 +++++++++++++++----- crates/wasmtime/src/engine.rs | 13 +- crates/wasmtime/src/engine/serialization.rs | 4 +- crates/wasmtime/src/runtime/instance.rs | 3 +- crates/wasmtime/src/runtime/module.rs | 2 +- crates/wasmtime/src/runtime/store.rs | 3 +- tests/wast.rs | 228 +++++++++++------ winch/codegen/src/codegen/context.rs | 4 +- winch/codegen/src/codegen/mod.rs | 4 +- winch/codegen/src/isa/x64/abi.rs | 4 +- winch/codegen/src/isa/x64/asm.rs | 2 +- winch/codegen/src/visitor.rs | 13 +- 15 files changed, 386 insertions(+), 167 deletions(-) diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs index 7bb0175637..5506c66728 100644 --- a/cranelift/wasm/src/code_translator.rs +++ b/cranelift/wasm/src/code_translator.rs @@ -2506,7 +2506,9 @@ pub fn translate_operator( } Operator::TryTable { .. } | Operator::ThrowRef => { - unimplemented!("exception operators not yet implemented") + return Err(wasm_unsupported!( + "exception operators are not yet implemented" + )); } Operator::RefEq @@ -2538,7 +2540,7 @@ pub fn translate_operator( | Operator::StructGetU { .. } | Operator::StructSet { .. } | Operator::StructGet { .. } => { - unimplemented!("GC operators not yet implemented") + return Err(wasm_unsupported!("GC operators are not yet implemented")); } Operator::GlobalAtomicGet { .. } @@ -2577,7 +2579,9 @@ pub fn translate_operator( | Operator::ArrayAtomicRmwXchg { .. } | Operator::ArrayAtomicRmwCmpxchg { .. } | Operator::RefI31Shared { .. } => { - unimplemented!("shared-everything-threads not yet implemented") + return Err(wasm_unsupported!( + "shared-everything-threads operators are not yet implemented" + )); } }; Ok(()) diff --git a/crates/wasmtime/src/compile.rs b/crates/wasmtime/src/compile.rs index 1e4ef3d6d5..35877008eb 100644 --- a/crates/wasmtime/src/compile.rs +++ b/crates/wasmtime/src/compile.rs @@ -70,7 +70,7 @@ pub(crate) fn build_artifacts( // validated. Afterwards `types` will have all the type information for // this module. let mut parser = wasmparser::Parser::new(0); - let mut validator = wasmparser::Validator::new_with_features(engine.config().features); + let mut validator = wasmparser::Validator::new_with_features(engine.features()); parser.set_features(*validator.features()); let mut types = ModuleTypesBuilder::new(&validator); let mut translation = ModuleEnvironment::new(tunables, &mut validator, &mut types) @@ -136,7 +136,7 @@ pub(crate) fn build_component_artifacts( let compiler = engine.compiler(); let scope = ScopeVec::new(); - let mut validator = wasmparser::Validator::new_with_features(engine.config().features); + let mut validator = wasmparser::Validator::new_with_features(engine.features()); let mut types = ComponentTypesBuilder::new(&validator); let (component, mut module_translations) = Translator::new(tunables, &mut validator, &mut types, &scope) diff --git a/crates/wasmtime/src/compile/code_builder.rs b/crates/wasmtime/src/compile/code_builder.rs index 0d35886c90..ec8b543292 100644 --- a/crates/wasmtime/src/compile/code_builder.rs +++ b/crates/wasmtime/src/compile/code_builder.rs @@ -308,7 +308,7 @@ impl std::hash::Hash for HashedEngineCompileEnv<'_> { // Hash configuration state read for compilation let config = self.0.config(); self.0.tunables().hash(hasher); - config.features.hash(hasher); + self.0.features().hash(hasher); config.wmemcheck.hash(hasher); // Catch accidental bugs of reusing across crate versions. diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs index a25cfd35ce..d3e731d28f 100644 --- a/crates/wasmtime/src/config.rs +++ b/crates/wasmtime/src/config.rs @@ -1,5 +1,6 @@ use crate::prelude::*; use alloc::sync::Arc; +use bitflags::Flags; use core::fmt; use core::str::FromStr; use hashbrown::{HashMap, HashSet}; @@ -114,7 +115,15 @@ pub struct Config { pub(crate) mem_creator: Option>, pub(crate) allocation_strategy: InstanceAllocationStrategy, pub(crate) max_wasm_stack: usize, - pub(crate) features: WasmFeatures, + /// Explicitly enabled features via `Config::wasm_*` methods. This is a + /// signal that the embedder specifically wants something turned on + /// regardless of the defaults that Wasmtime might otherwise have enabled. + /// + /// Note that this, and `disabled_features` below, start as the empty set of + /// features to only track explicit user requests. + pub(crate) enabled_features: WasmFeatures, + /// Same as `enabled_features`, but for those that are explicitly disabled. + pub(crate) disabled_features: WasmFeatures, pub(crate) wasm_backtrace: bool, pub(crate) wasm_backtrace_details_env_used: bool, pub(crate) native_unwind_info: Option, @@ -234,7 +243,8 @@ impl Config { wasm_backtrace: true, wasm_backtrace_details_env_used: false, native_unwind_info: None, - features: WasmFeatures::default(), + enabled_features: WasmFeatures::empty(), + disabled_features: WasmFeatures::empty(), #[cfg(feature = "async")] async_stack_size: 2 << 20, #[cfg(feature = "async")] @@ -259,30 +269,6 @@ impl Config { ret.cranelift_opt_level(OptLevel::Speed); } - // Conditionally enabled features depending on compile-time crate - // features. Note that if these features are disabled then `Config` has - // no way of re-enabling them. - ret.features - .set(WasmFeatures::REFERENCE_TYPES, cfg!(feature = "gc")); - ret.features - .set(WasmFeatures::THREADS, cfg!(feature = "threads")); - ret.features.set( - WasmFeatures::COMPONENT_MODEL, - cfg!(feature = "component-model"), - ); - - // If GC is disabled at compile time also disable it in features - // forcibly irrespective of `wasmparser` defaults. Note that these also - // aren't yet fully implemented in Wasmtime. - if !cfg!(feature = "gc") { - ret.features.set(WasmFeatures::FUNCTION_REFERENCES, false); - ret.features.set(WasmFeatures::GC, false); - } - - ret.wasm_multi_value(true); - ret.wasm_bulk_memory(true); - ret.wasm_simd(true); - ret.wasm_extended_const(true); ret.wasm_backtrace_details(WasmBacktraceDetails::Environment); ret @@ -701,6 +687,12 @@ impl Config { self } + fn wasm_feature(&mut self, flag: WasmFeatures, enable: bool) -> &mut Self { + self.enabled_features.set(flag, enable); + self.disabled_features.set(flag, !enable); + self + } + /// Configures whether the WebAssembly tail calls proposal will be enabled /// for compilation or not. /// @@ -713,7 +705,7 @@ impl Config { /// /// [WebAssembly tail calls proposal]: https://github.com/WebAssembly/tail-call pub fn wasm_tail_call(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::TAIL_CALL, enable); + self.wasm_feature(WasmFeatures::TAIL_CALL, enable); self } @@ -738,7 +730,7 @@ impl Config { /// /// [WebAssembly custom-page-sizes proposal]: https://github.com/WebAssembly/custom-page-sizes pub fn wasm_custom_page_sizes(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::CUSTOM_PAGE_SIZES, enable); + self.wasm_feature(WasmFeatures::CUSTOM_PAGE_SIZES, enable); self } @@ -761,7 +753,7 @@ impl Config { /// [wasi-threads]: https://github.com/webassembly/wasi-threads #[cfg(feature = "threads")] pub fn wasm_threads(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::THREADS, enable); + self.wasm_feature(WasmFeatures::THREADS, enable); self } @@ -783,7 +775,7 @@ impl Config { /// [proposal]: https://github.com/webassembly/reference-types #[cfg(feature = "gc")] pub fn wasm_reference_types(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::REFERENCE_TYPES, enable); + self.wasm_feature(WasmFeatures::REFERENCE_TYPES, enable); self } @@ -802,7 +794,7 @@ impl Config { /// [proposal]: https://github.com/WebAssembly/function-references #[cfg(feature = "gc")] pub fn wasm_function_references(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::FUNCTION_REFERENCES, enable); + self.wasm_feature(WasmFeatures::FUNCTION_REFERENCES, enable); self } @@ -823,7 +815,7 @@ impl Config { /// [proposal]: https://github.com/WebAssembly/gc #[cfg(feature = "gc")] pub fn wasm_gc(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::GC, enable); + self.wasm_feature(WasmFeatures::GC, enable); self } @@ -843,7 +835,7 @@ impl Config { /// [proposal]: https://github.com/webassembly/simd /// [relaxed simd proposal]: https://github.com/WebAssembly/relaxed-simd pub fn wasm_simd(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::SIMD, enable); + self.wasm_feature(WasmFeatures::SIMD, enable); self } @@ -870,7 +862,7 @@ impl Config { /// /// [proposal]: https://github.com/webassembly/relaxed-simd pub fn wasm_relaxed_simd(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::RELAXED_SIMD, enable); + self.wasm_feature(WasmFeatures::RELAXED_SIMD, enable); self } @@ -914,7 +906,7 @@ impl Config { /// /// [proposal]: https://github.com/webassembly/bulk-memory-operations pub fn wasm_bulk_memory(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::BULK_MEMORY, enable); + self.wasm_feature(WasmFeatures::BULK_MEMORY, enable); self } @@ -928,7 +920,7 @@ impl Config { /// /// [proposal]: https://github.com/webassembly/multi-value pub fn wasm_multi_value(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::MULTI_VALUE, enable); + self.wasm_feature(WasmFeatures::MULTI_VALUE, enable); self } @@ -942,7 +934,7 @@ impl Config { /// /// [proposal]: https://github.com/webassembly/multi-memory pub fn wasm_multi_memory(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::MULTI_MEMORY, enable); + self.wasm_feature(WasmFeatures::MULTI_MEMORY, enable); self } @@ -957,7 +949,7 @@ impl Config { /// /// [proposal]: https://github.com/webassembly/memory64 pub fn wasm_memory64(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::MEMORY64, enable); + self.wasm_feature(WasmFeatures::MEMORY64, enable); self } @@ -968,7 +960,7 @@ impl Config { /// /// [proposal]: https://github.com/webassembly/extended-const pub fn wasm_extended_const(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::EXTENDED_CONST, enable); + self.wasm_feature(WasmFeatures::EXTENDED_CONST, enable); self } @@ -982,7 +974,7 @@ impl Config { /// [proposal]: https://github.com/webassembly/component-model #[cfg(feature = "component-model")] pub fn wasm_component_model(&mut self, enable: bool) -> &mut Self { - self.features.set(WasmFeatures::COMPONENT_MODEL, enable); + self.wasm_feature(WasmFeatures::COMPONENT_MODEL, enable); self } @@ -993,8 +985,7 @@ impl Config { /// https://github.com/WebAssembly/component-model/issues/370. #[cfg(feature = "component-model")] pub fn wasm_component_model_more_flags(&mut self, enable: bool) -> &mut Self { - self.features - .set(WasmFeatures::COMPONENT_MODEL_MORE_FLAGS, enable); + self.wasm_feature(WasmFeatures::COMPONENT_MODEL_MORE_FLAGS, enable); self } @@ -1004,8 +995,7 @@ impl Config { /// https://github.com/WebAssembly/component-model/pull/368. #[cfg(feature = "component-model")] pub fn wasm_component_model_multiple_returns(&mut self, enable: bool) -> &mut Self { - self.features - .set(WasmFeatures::COMPONENT_MODEL_MULTIPLE_RETURNS, enable); + self.wasm_feature(WasmFeatures::COMPONENT_MODEL_MULTIPLE_RETURNS, enable); self } @@ -1732,24 +1722,164 @@ impl Config { self } - pub(crate) fn validate(&self) -> Result { - if self.features.contains(WasmFeatures::REFERENCE_TYPES) - && !self.features.contains(WasmFeatures::BULK_MEMORY) + /// Returns the set of features that the currently selected compiler backend + /// does not support at all and may panic on. + /// + /// Wasmtime strives to reject unknown modules or unsupported modules with + /// first-class errors instead of panics. Not all compiler backends have the + /// same level of feature support on all platforms as well. This method + /// returns a set of features that the currently selected compiler + /// configuration is known to not support and may panic on. This acts as a + /// first-level filter on incoming wasm modules/configuration to fail-fast + /// instead of panicking later on. + /// + /// Note that if a feature is not listed here it does not mean that the + /// backend fully supports the proposal. Instead that means that the backend + /// doesn't ever panic on the proposal, but errors during compilation may + /// still be returned. This means that features listed here are definitely + /// not supported at all, but features not listed here may still be + /// partially supported. For example at the time of this writing the Winch + /// backend partially supports simd so it's not listed here. Winch doesn't + /// fully support simd but unimplemented instructions just return errors. + fn compiler_panicking_wasm_features(&self) -> WasmFeatures { + #[cfg(any(feature = "cranelift", feature = "winch"))] + match self.compiler_config.strategy { + None | Some(Strategy::Cranelift) => WasmFeatures::empty(), + Some(Strategy::Winch) => { + let mut unsupported = WasmFeatures::GC + | WasmFeatures::FUNCTION_REFERENCES + | WasmFeatures::THREADS + | WasmFeatures::RELAXED_SIMD + | WasmFeatures::TAIL_CALL; + match self.compiler_target().architecture { + target_lexicon::Architecture::Aarch64(_) => { + // no support for simd on aarch64 + unsupported |= WasmFeatures::SIMD; + // technically this is mostly supported in the sense of + // multi-tables work well enough but enough of MVP wasm + // currently panics that this is used here instead to + // disable most spec tests which require reference + // types. + unsupported |= WasmFeatures::REFERENCE_TYPES; + } + + // Winch doesn't support other non-x64 architectures at this + // time either but will return an first-class error for + // them. + _ => {} + } + unsupported + } + Some(Strategy::Auto) => unreachable!(), + } + #[cfg(not(any(feature = "cranelift", feature = "winch")))] + return WasmFeatures::empty(); + } + + /// Calculates the set of features that are enabled for this `Config`. + /// + /// This method internally will start with the an empty set of features to + /// avoid being tied to wasmparser's defaults. Next Wasmtime's set of + /// default features are added to this set, some of which are conditional + /// depending on crate features. Finally explicitly requested features via + /// `wasm_*` methods on `Config` are applied. Everything is then validated + /// later in `Config::validate`. + fn features(&self) -> WasmFeatures { + let mut features = WasmFeatures::empty(); + + // On-by-default features that wasmtime has. Note that these are all + // subject to the criteria at + // https://docs.wasmtime.dev/contributing-implementing-wasm-proposals.html + features |= WasmFeatures::FLOATS; + features |= WasmFeatures::MULTI_VALUE; + features |= WasmFeatures::BULK_MEMORY; + features |= WasmFeatures::SIGN_EXTENSION; + features |= WasmFeatures::MUTABLE_GLOBAL; + features |= WasmFeatures::SATURATING_FLOAT_TO_INT; + features |= WasmFeatures::MULTI_MEMORY; + features |= WasmFeatures::SIMD; + features |= WasmFeatures::RELAXED_SIMD; + features |= WasmFeatures::TAIL_CALL; + features |= WasmFeatures::EXTENDED_CONST; + if cfg!(feature = "gc") { + features |= WasmFeatures::REFERENCE_TYPES; + } + if cfg!(feature = "threads") { + features |= WasmFeatures::THREADS; + } + if cfg!(feature = "component-model") { + features |= WasmFeatures::COMPONENT_MODEL; + } + + // From the default set of proposals remove any that the current + // compiler backend may panic on if the module contains them. + features = features & !self.compiler_panicking_wasm_features(); + + // After wasmtime's defaults are configured then factor in user requests + // and disable/enable features. Note that the enable/disable sets should + // be disjoint. + debug_assert!((self.enabled_features & self.disabled_features).is_empty()); + features &= !self.disabled_features; + features |= self.enabled_features; + + features + } + + fn compiler_target(&self) -> target_lexicon::Triple { + #[cfg(any(feature = "cranelift", feature = "winch"))] + { + let host = target_lexicon::Triple::host(); + + self.compiler_config + .target + .as_ref() + .unwrap_or(&host) + .clone() + } + #[cfg(not(any(feature = "cranelift", feature = "winch")))] + { + target_lexicon::Triple::host() + } + } + + pub(crate) fn validate(&self) -> Result<(Tunables, WasmFeatures)> { + let features = self.features(); + + // First validate that the selected compiler backend and configuration + // supports the set of `features` that are enabled. This will help + // provide more first class errors instead of panics about unsupported + // features and configurations. + let unsupported = features & self.compiler_panicking_wasm_features(); + if !unsupported.is_empty() { + for flag in WasmFeatures::FLAGS.iter() { + if !unsupported.contains(*flag.value()) { + continue; + } + bail!( + "the wasm_{} feature is not supported on this compiler configuration", + flag.name().to_lowercase() + ); + } + + panic!("should have returned an error by now") + } + + if features.contains(WasmFeatures::REFERENCE_TYPES) + && !features.contains(WasmFeatures::BULK_MEMORY) { bail!("feature 'reference_types' requires 'bulk_memory' to be enabled"); } - if self.features.contains(WasmFeatures::THREADS) - && !self.features.contains(WasmFeatures::BULK_MEMORY) + if features.contains(WasmFeatures::THREADS) && !features.contains(WasmFeatures::BULK_MEMORY) { bail!("feature 'threads' requires 'bulk_memory' to be enabled"); } - if self.features.contains(WasmFeatures::FUNCTION_REFERENCES) - && !self.features.contains(WasmFeatures::REFERENCE_TYPES) + if features.contains(WasmFeatures::FUNCTION_REFERENCES) + && !features.contains(WasmFeatures::REFERENCE_TYPES) { bail!("feature 'function_references' requires 'reference_types' to be enabled"); } - if self.features.contains(WasmFeatures::GC) - && !self.features.contains(WasmFeatures::FUNCTION_REFERENCES) + if features.contains(WasmFeatures::GC) + && !features.contains(WasmFeatures::FUNCTION_REFERENCES) { bail!("feature 'gc' requires 'function_references' to be enabled"); } @@ -1818,7 +1948,7 @@ impl Config { bail!("static memory guard size cannot be smaller than dynamic memory guard size"); } - Ok(tunables) + Ok((tunables, features)) } #[cfg(feature = "runtime")] @@ -1877,6 +2007,7 @@ impl Config { pub(crate) fn build_compiler( mut self, tunables: &Tunables, + features: WasmFeatures, ) -> Result<(Self, Box)> { let target = self.compiler_config.target.clone(); @@ -1904,13 +2035,7 @@ impl Config { .settings .insert("probestack_strategy".into(), "inline".into()); - let host = target_lexicon::Triple::host(); - let target = self - .compiler_config - .target - .as_ref() - .unwrap_or(&host) - .clone(); + let target = self.compiler_target(); // On supported targets, we enable stack probing by default. // This is required on Windows because of the way Windows @@ -1949,7 +2074,7 @@ impl Config { .insert("preserve_frame_pointers".into(), "true".into()); // check for incompatible compiler options and set required values - if self.features.contains(WasmFeatures::REFERENCE_TYPES) { + if features.contains(WasmFeatures::REFERENCE_TYPES) { if !self .compiler_config .ensure_setting_unset_or_given("enable_safepoints", "true") @@ -1958,9 +2083,7 @@ impl Config { } } - if self.features.contains(WasmFeatures::RELAXED_SIMD) - && !self.features.contains(WasmFeatures::SIMD) - { + if features.contains(WasmFeatures::RELAXED_SIMD) && !features.contains(WasmFeatures::SIMD) { bail!("cannot disable the simd proposal but enable the relaxed simd proposal"); } @@ -2085,11 +2208,11 @@ impl fmt::Debug for Config { // enabled, and doesn't require maintenance by hand (which has become out // of date in the past), at the cost of possible confusion for why // a flag in this set doesn't have a Config setter. - use bitflags::Flags; + let features = self.features(); for flag in WasmFeatures::FLAGS.iter() { f.field( &format!("wasm_{}", flag.name().to_lowercase()), - &self.features.contains(*flag.value()), + &features.contains(*flag.value()), ); } diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs index 735d21f2dc..8fa1797706 100644 --- a/crates/wasmtime/src/engine.rs +++ b/crates/wasmtime/src/engine.rs @@ -47,6 +47,7 @@ pub struct Engine { struct EngineInner { config: Config, + features: WasmFeatures, tunables: Tunables, #[cfg(any(feature = "cranelift", feature = "winch"))] compiler: Box, @@ -99,10 +100,10 @@ impl Engine { } let config = config.clone(); - let tunables = config.validate()?; + let (tunables, features) = config.validate()?; #[cfg(any(feature = "cranelift", feature = "winch"))] - let (config, compiler) = config.build_compiler(&tunables)?; + let (config, compiler) = config.build_compiler(&tunables, features)?; Ok(Engine { inner: Arc::new(EngineInner { @@ -122,6 +123,7 @@ impl Engine { compatible_with_native_host: OnceLock::new(), config, tunables, + features, }), }) } @@ -132,6 +134,11 @@ impl Engine { &self.inner.config } + #[inline] + pub(crate) fn features(&self) -> WasmFeatures { + self.inner.features + } + pub(crate) fn run_maybe_parallel< A: Send, B: Send, @@ -307,7 +314,7 @@ impl Engine { // like typed function references and GC) are enabled this must be // enabled, otherwise this setting can have any value. "enable_safepoints" => { - if self.config().features.contains(WasmFeatures::REFERENCE_TYPES) { + if self.features().contains(WasmFeatures::REFERENCE_TYPES) { *value == FlagValue::Bool(true) } else { return Ok(()) diff --git a/crates/wasmtime/src/engine/serialization.rs b/crates/wasmtime/src/engine/serialization.rs index 13f6d07f1a..665b7569a7 100644 --- a/crates/wasmtime/src/engine/serialization.rs +++ b/crates/wasmtime/src/engine/serialization.rs @@ -238,7 +238,7 @@ impl Metadata<'_> { saturating_float_to_int: _, sign_extension: _, floats: _, - } = engine.config().features.inflate(); + } = engine.features().inflate(); // These features are not implemented in Wasmtime yet. We match on them // above so that once we do implement support for them, we won't @@ -281,7 +281,7 @@ impl Metadata<'_> { self.check_shared_flags(engine)?; self.check_isa_flags(engine)?; self.check_tunables(&engine.tunables())?; - self.check_features(&engine.config().features)?; + self.check_features(&engine.features())?; Ok(()) } diff --git a/crates/wasmtime/src/runtime/instance.rs b/crates/wasmtime/src/runtime/instance.rs index 022ee6a183..06c3e66ffa 100644 --- a/crates/wasmtime/src/runtime/instance.rs +++ b/crates/wasmtime/src/runtime/instance.rs @@ -341,8 +341,7 @@ impl Instance { compiled_module.module(), store .engine() - .config() - .features + .features() .contains(WasmFeatures::BULK_MEMORY), )?; diff --git a/crates/wasmtime/src/runtime/module.rs b/crates/wasmtime/src/runtime/module.rs index abb96227e0..2b277e254f 100644 --- a/crates/wasmtime/src/runtime/module.rs +++ b/crates/wasmtime/src/runtime/module.rs @@ -513,7 +513,7 @@ impl Module { /// /// [binary]: https://webassembly.github.io/spec/core/binary/index.html pub fn validate(engine: &Engine, binary: &[u8]) -> Result<()> { - let mut validator = Validator::new_with_features(engine.config().features); + let mut validator = Validator::new_with_features(engine.features()); let mut functions = Vec::new(); for payload in Parser::new(0).parse_all(binary) { diff --git a/crates/wasmtime/src/runtime/store.rs b/crates/wasmtime/src/runtime/store.rs index 356fe56100..d1f238fa59 100644 --- a/crates/wasmtime/src/runtime/store.rs +++ b/crates/wasmtime/src/runtime/store.rs @@ -1520,8 +1520,7 @@ impl StoreOpaque { #[cfg(feature = "gc")] fn allocate_gc_store(engine: &Engine) -> Result { let (index, heap) = if engine - .config() - .features + .features() .contains(wasmparser::WasmFeatures::REFERENCE_TYPES) { engine diff --git a/tests/wast.rs b/tests/wast.rs index e4983d7d3e..875303c672 100644 --- a/tests/wast.rs +++ b/tests/wast.rs @@ -1,4 +1,4 @@ -use anyhow::Context; +use anyhow::{bail, Context}; use bstr::ByteSlice; use libtest_mimic::{Arguments, FormatSetting, Trial}; use once_cell::sync::Lazy; @@ -57,23 +57,147 @@ fn add_tests(trials: &mut Vec, path: &Path) { } }, ); - trials.push(trial.with_ignored_flag(ignore(&path, strategy))); + trials.push(trial); } } } } -fn ignore(test: &Path, strategy: Strategy) -> bool { +fn should_fail(test: &Path, strategy: Strategy) -> bool { // Winch only supports x86_64 at this time. if strategy == Strategy::Winch && !cfg!(target_arch = "x86_64") { return true; } + // Disable spec tests for proposals that Winch does not implement yet. + if strategy == Strategy::Winch { + let unsupported = [ + // externref/reference-types related + "memory64/threads.wast", + "misc_testsuite/externref-id-function.wast", + "misc_testsuite/externref-segments.wast", + "misc_testsuite/many_table_gets_lead_to_gc.wast", + "misc_testsuite/mutable_externref_globals.wast", + "misc_testsuite/no-mixup-stack-maps.wast", + "misc_testsuite/no-panic.wast", + "misc_testsuite/simple_ref_is_null.wast", + "misc_testsuite/table_grow_with_funcref.wast", + "spec_testsuite/br_table.wast", + "spec_testsuite/data-invalid.wast", + "spec_testsuite/elem.wast", + "spec_testsuite/global.wast", + "spec_testsuite/ref_func.wast", + "spec_testsuite/ref_is_null.wast", + "spec_testsuite/ref_null.wast", + "spec_testsuite/select.wast", + "spec_testsuite/table_fill.wast", + "spec_testsuite/table_get.wast", + "spec_testsuite/table_grow.wast", + "spec_testsuite/table_set.wast", + "spec_testsuite/table_size.wast", + "spec_testsuite/unreached-invalid.wast", + "extended-const/elem.wast", + "extended-const/global.wast", + // simd-related failures + "annotations/simd_lane.wast", + "memory64/simd.wast", + "misc_testsuite/int-to-float-splat.wast", + "misc_testsuite/issue6562.wast", + "misc_testsuite/simd/almost-extmul.wast", + "misc_testsuite/simd/canonicalize-nan.wast", + "misc_testsuite/simd/cvt-from-uint.wast", + "misc_testsuite/simd/issue4807.wast", + "misc_testsuite/simd/issue6725-no-egraph-panic.wast", + "misc_testsuite/simd/issue_3327_bnot_lowering.wast", + "misc_testsuite/simd/load_splat_out_of_bounds.wast", + "misc_testsuite/simd/replace-lane-preserve.wast", + "misc_testsuite/simd/spillslot-size-fuzzbug.wast", + "misc_testsuite/simd/unaligned-load.wast", + "multi-memory/simd_memory-multi.wast", + "spec_testsuite/simd_align.wast", + "spec_testsuite/simd_bit_shift.wast", + "spec_testsuite/simd_bitwise.wast", + "spec_testsuite/simd_boolean.wast", + "spec_testsuite/simd_const.wast", + "spec_testsuite/simd_conversions.wast", + "spec_testsuite/simd_f32x4.wast", + "spec_testsuite/simd_f32x4_arith.wast", + "spec_testsuite/simd_f32x4_cmp.wast", + "spec_testsuite/simd_f32x4_pmin_pmax.wast", + "spec_testsuite/simd_f32x4_rounding.wast", + "spec_testsuite/simd_f64x2.wast", + "spec_testsuite/simd_f64x2_arith.wast", + "spec_testsuite/simd_f64x2_cmp.wast", + "spec_testsuite/simd_f64x2_pmin_pmax.wast", + "spec_testsuite/simd_f64x2_rounding.wast", + "spec_testsuite/simd_i16x8_arith.wast", + "spec_testsuite/simd_i16x8_arith2.wast", + "spec_testsuite/simd_i16x8_cmp.wast", + "spec_testsuite/simd_i16x8_extadd_pairwise_i8x16.wast", + "spec_testsuite/simd_i16x8_extmul_i8x16.wast", + "spec_testsuite/simd_i16x8_q15mulr_sat_s.wast", + "spec_testsuite/simd_i16x8_sat_arith.wast", + "spec_testsuite/simd_i32x4_arith.wast", + "spec_testsuite/simd_i32x4_arith2.wast", + "spec_testsuite/simd_i32x4_cmp.wast", + "spec_testsuite/simd_i32x4_dot_i16x8.wast", + "spec_testsuite/simd_i32x4_extadd_pairwise_i16x8.wast", + "spec_testsuite/simd_i32x4_extmul_i16x8.wast", + "spec_testsuite/simd_i32x4_trunc_sat_f32x4.wast", + "spec_testsuite/simd_i32x4_trunc_sat_f64x2.wast", + "spec_testsuite/simd_i64x2_arith.wast", + "spec_testsuite/simd_i64x2_arith2.wast", + "spec_testsuite/simd_i64x2_cmp.wast", + "spec_testsuite/simd_i64x2_extmul_i32x4.wast", + "spec_testsuite/simd_i8x16_arith.wast", + "spec_testsuite/simd_i8x16_arith2.wast", + "spec_testsuite/simd_i8x16_cmp.wast", + "spec_testsuite/simd_i8x16_sat_arith.wast", + "spec_testsuite/simd_int_to_int_extend.wast", + "spec_testsuite/simd_lane.wast", + "spec_testsuite/simd_load.wast", + "spec_testsuite/simd_load16_lane.wast", + "spec_testsuite/simd_load32_lane.wast", + "spec_testsuite/simd_load64_lane.wast", + "spec_testsuite/simd_load8_lane.wast", + "spec_testsuite/simd_load_extend.wast", + "spec_testsuite/simd_load_splat.wast", + "spec_testsuite/simd_load_zero.wast", + "spec_testsuite/simd_splat.wast", + "spec_testsuite/simd_store16_lane.wast", + "spec_testsuite/simd_store32_lane.wast", + "spec_testsuite/simd_store64_lane.wast", + "spec_testsuite/simd_store8_lane.wast", + ]; + + if unsupported.iter().any(|part| test.ends_with(part)) { + return true; + } + + // A few proposals that winch has no support for. + let unsupported_proposals = [ + "function-references", + "gc", + "tail-call", + "relaxed-simd", + "threads", + ]; + if let Some(parent) = test.parent() { + if unsupported_proposals + .iter() + .any(|part| parent.ends_with(part)) + { + return true; + } + } + } + for part in test.iter() { // Not implemented in Wasmtime yet if part == "exception-handling" { - return true; + return !test.ends_with("binary.wast"); } + // Wasmtime doesn't implement the table64 extension yet. if part == "memory64" { if [ @@ -95,48 +219,6 @@ fn ignore(test: &Path, strategy: Strategy) -> bool { } } - // Disable spec tests for proposals that Winch does not implement yet. - if strategy == Strategy::Winch { - let part = part.to_str().unwrap(); - let unsupported = [ - // wasm proposals that Winch doesn't support, - "references", - "tail-call", - "gc", - "threads", - "multi-memory", - "relaxed-simd", - "function-references", - // tests in misc_testsuite that Winch doesn't support - "no-panic.wast", - "externref-id-function.wast", - "int-to-float-splat.wast", - "issue6562.wast", - "many_table_gets_lead_to_gc.wast", - "mutable_externref_globals.wast", - "no-mixup-stack-maps.wast", - "simple_ref_is_null.wast", - "table_grow_with_funcref.wast", - // Tests in the spec test suite Winch doesn't support - "threads.wast", - "br_table.wast", - "global.wast", - "table_fill.wast", - "table_get.wast", - "table_set.wast", - "table_grow.wast", - "table_size.wast", - "elem.wast", - "select.wast", - "unreached-invalid.wast", - "linking.wast", - ]; - - if unsupported.contains(&part) || part.starts_with("simd") || part.starts_with("ref_") { - return true; - } - } - // Implementation of the GC proposal is a work-in-progress, this is // a list of all currently known-to-fail tests. if part == "gc" { @@ -147,41 +229,20 @@ fn ignore(test: &Path, strategy: Strategy) -> bool { "array_init_elem.wast", "array.wast", "binary_gc.wast", - "binary.wast", "br_on_cast_fail.wast", "br_on_cast.wast", - "br_on_non_null.wast", - "br_on_null.wast", - "br_table.wast", - "call_ref.wast", - "data.wast", - "elem.wast", "extern.wast", - "func.wast", - "global.wast", - "if.wast", - "linking.wast", - "local_get.wast", - "local_init.wast", - "ref_as_non_null.wast", "ref_cast.wast", "ref_eq.wast", - "ref_is_null.wast", - "ref_null.wast", "ref_test.wast", - "ref.wast", "return_call_indirect.wast", - "return_call_ref.wast", "return_call.wast", - "select.wast", "struct.wast", "table_sub.wast", - "table.wast", "type_canon.wast", "type_equivalence.wast", "type-rec.wast", "type-subtyping.wast", - "unreached-invalid.wast", "unreached_valid.wast", "i31.wast", ] @@ -197,6 +258,7 @@ fn ignore(test: &Path, strategy: Strategy) -> bool { // function which actually executes the `wast` test suite given the `strategy` // to compile it. fn run_wast(wast: &Path, strategy: Strategy, pooling: bool) -> anyhow::Result<()> { + let should_fail = should_fail(wast, strategy); let wast_bytes = std::fs::read(wast).with_context(|| format!("failed to read `{}`", wast.display()))?; @@ -340,28 +402,38 @@ fn run_wast(wast: &Path, strategy: Strategy, pooling: bool) -> anyhow::Result<() None }; - let mut engines = vec![(Engine::new(&cfg)?, "default")]; + let mut engines = vec![(Engine::new(&cfg), "default")]; // For tests that use relaxed-simd test both the default engine and the // guaranteed-deterministic engine to ensure that both the 'native' // semantics of the instructions plus the canonical semantics work. if relaxed_simd { engines.push(( - Engine::new(cfg.relaxed_simd_deterministic(true))?, + Engine::new(cfg.relaxed_simd_deterministic(true)), "deterministic", )); } for (engine, desc) in engines { - let store = Store::new(&engine, ()); - let mut wast_context = WastContext::new(store); - wast_context.register_spectest(&SpectestConfig { - use_shared_memory, - suppress_prints: true, - })?; - wast_context - .run_buffer(wast.to_str().unwrap(), &wast_bytes) - .with_context(|| format!("failed to run spec test with {desc} engine"))?; + let result = engine.and_then(|engine| { + let store = Store::new(&engine, ()); + let mut wast_context = WastContext::new(store); + wast_context.register_spectest(&SpectestConfig { + use_shared_memory, + suppress_prints: true, + })?; + wast_context + .run_buffer(wast.to_str().unwrap(), &wast_bytes) + .with_context(|| format!("failed to run spec test with {desc} engine")) + }); + + if should_fail { + if result.is_ok() { + bail!("this test is flagged as should-fail but it succeeded") + } + } else { + result?; + } } Ok(()) diff --git a/winch/codegen/src/codegen/context.rs b/winch/codegen/src/codegen/context.rs index c7e34fb4d7..f61b7749a0 100644 --- a/winch/codegen/src/codegen/context.rs +++ b/winch/codegen/src/codegen/context.rs @@ -73,7 +73,9 @@ impl<'a> CodeGenContext<'a> { // All of our supported architectures use the float registers for vector operations. V128 => self.reg_for_class(RegClass::Float, masm), Ref(rt) => match rt.heap_type { - WasmHeapType::Func => self.reg_for_class(RegClass::Int, masm), + WasmHeapType::Func | WasmHeapType::Extern => { + self.reg_for_class(RegClass::Int, masm) + } ht => unimplemented!("Support for WasmHeapType: {ht}"), }, } diff --git a/winch/codegen/src/codegen/mod.rs b/winch/codegen/src/codegen/mod.rs index 6d052982e4..0edcd7fc41 100644 --- a/winch/codegen/src/codegen/mod.rs +++ b/winch/codegen/src/codegen/mod.rs @@ -419,7 +419,9 @@ where match &ty { I32 | I64 | F32 | F64 | V128 => self.masm.store(src.into(), addr, ty.into()), Ref(rt) => match rt.heap_type { - WasmHeapType::Func => self.masm.store_ptr(src.into(), addr), + WasmHeapType::Func | WasmHeapType::Extern => { + self.masm.store_ptr(src.into(), addr) + } ht => unimplemented!("Support for WasmHeapType: {ht}"), }, } diff --git a/winch/codegen/src/isa/x64/abi.rs b/winch/codegen/src/isa/x64/abi.rs index 02829f28d1..294e484faf 100644 --- a/winch/codegen/src/isa/x64/abi.rs +++ b/winch/codegen/src/isa/x64/abi.rs @@ -170,7 +170,7 @@ impl ABI for X64ABI { fn sizeof(ty: &WasmValType) -> u8 { match ty { WasmValType::Ref(rt) => match rt.heap_type { - WasmHeapType::Func => Self::word_bytes(), + WasmHeapType::Func | WasmHeapType::Extern => Self::word_bytes(), ht => unimplemented!("Support for WasmHeapType: {ht}"), }, WasmValType::F64 | WasmValType::I64 => Self::word_bytes(), @@ -190,7 +190,7 @@ impl X64ABI { ) -> (ABIOperand, u32) { let (reg, ty) = match wasm_arg { ty @ WasmValType::Ref(rt) => match rt.heap_type { - WasmHeapType::Func => ( + WasmHeapType::Func | WasmHeapType::Extern => ( Self::int_reg_for(index_env.next_gpr(), call_conv, params_or_returns), ty, ), diff --git a/winch/codegen/src/isa/x64/asm.rs b/winch/codegen/src/isa/x64/asm.rs index 19d9328fc9..79eab2a807 100644 --- a/winch/codegen/src/isa/x64/asm.rs +++ b/winch/codegen/src/isa/x64/asm.rs @@ -490,7 +490,7 @@ impl Assembler { OperandSize::S32 => types::F32, OperandSize::S64 => types::F64, // Move the entire 128 bits via movdqa. - OperandSize::S128 => types::I128, + OperandSize::S128 => types::I32X4, OperandSize::S8 | OperandSize::S16 => unreachable!(), }; diff --git a/winch/codegen/src/visitor.rs b/winch/codegen/src/visitor.rs index c36f9324f9..e313300af1 100644 --- a/winch/codegen/src/visitor.rs +++ b/winch/codegen/src/visitor.rs @@ -1338,6 +1338,10 @@ where I32 | I64 | F32 | F64 | V128 => context.stack.push(Val::local(index, slot.ty)), Ref(rt) => match rt.heap_type { WasmHeapType::Func => context.stack.push(Val::local(index, slot.ty)), + WasmHeapType::Extern => { + self.found_unsupported_instruction = + Some("unsupported local.get of externref local"); + } ht => unimplemented!("Support for WasmHeapType: {ht}"), }, } @@ -1430,7 +1434,13 @@ where } _ => unimplemented!("Support for eager table init"), }, - t => unimplemented!("Support for WasmHeapType: {t}"), + WasmHeapType::Extern => { + self.found_unsupported_instruction = + Some("unsupported table.get of externref table"); + } + t => { + unimplemented!("Support for WasmHeapType: {t}") + } } } @@ -2150,6 +2160,7 @@ impl From for OperandSize { // to be updated in such a way that the calculation of the // OperandSize will depend on the target's pointer size. WasmHeapType::Func => OperandSize::S64, + WasmHeapType::Extern => OperandSize::S64, t => unimplemented!("Support for WasmHeapType: {t}"), } }