From 02adf4f72feb5181b3243622d473830baad6bb32 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 8 Mar 2022 11:20:05 -0800 Subject: [PATCH] Implement runtime checks for compilation settings This commit fills out a few FIXME annotations by implementing run-time checks that when a `Module` is created it has compatible codegen settings for the current host (as `Module` is proof of "this code can run"). This is done by implementing new `Engine`-level methods which validate compiler settings. These settings are validated on `Module::new` as well as when loading serialized modules. Settings are split into two categories, one for "shared" top-level settings and one for ISA-specific settings. Both categories now have allow-lists hardcoded into `Engine` which indicate the acceptable values for each setting (if applicable). ISA-specific settings are checked with the Rust standard library's `std::is_x86_feature_detected!` macro. Other macros for other platforms are not stable at this time but can be added here if necessary. Closes #3897 --- crates/environ/src/compilation.rs | 2 +- crates/wasmtime/src/engine.rs | 208 +++++++++++++++++++- crates/wasmtime/src/module.rs | 15 +- crates/wasmtime/src/module/serialization.rs | 122 ++++-------- tests/all/wast.rs | 5 +- 5 files changed, 253 insertions(+), 99 deletions(-) diff --git a/crates/environ/src/compilation.rs b/crates/environ/src/compilation.rs index d091024e6615..3ec65cb4f2e0 100644 --- a/crates/environ/src/compilation.rs +++ b/crates/environ/src/compilation.rs @@ -235,7 +235,7 @@ pub trait Compiler: Send + Sync { } /// Value of a configured setting for a [`Compiler`] -#[derive(Serialize, Deserialize, Hash, Eq, PartialEq)] +#[derive(Serialize, Deserialize, Hash, Eq, PartialEq, Debug)] pub enum FlagValue { /// Name of the value that has been configured for this setting. Enum(Cow<'static, str>), diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs index 835923d9dacc..6760b9069017 100644 --- a/crates/wasmtime/src/engine.rs +++ b/crates/wasmtime/src/engine.rs @@ -1,12 +1,13 @@ use crate::signatures::SignatureRegistry; use crate::{Config, Trap}; -use anyhow::Result; +use anyhow::{bail, Result}; #[cfg(feature = "parallel-compilation")] use rayon::prelude::*; -use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::Arc; #[cfg(feature = "cache")] use wasmtime_cache::CacheConfig; +use wasmtime_environ::FlagValue; use wasmtime_runtime::{debug_builtins, CompiledModuleIdAllocator, InstanceAllocator}; /// An `Engine` which is a global context for compilation and management of wasm @@ -44,6 +45,7 @@ struct EngineInner { signatures: SignatureRegistry, epoch: AtomicU64, unique_id_allocator: CompiledModuleIdAllocator, + compatible_with_native_host: AtomicBool, } impl Engine { @@ -70,6 +72,7 @@ impl Engine { signatures: registry, epoch: AtomicU64::new(0), unique_id_allocator: CompiledModuleIdAllocator::new(), + compatible_with_native_host: AtomicBool::new(false), }), }) } @@ -217,6 +220,207 @@ impl Engine { .map(|a| f(a)) .collect::, E>>() } + + /// Returns the target triple which this engine is compiling code for + /// and/or running code for. + pub(crate) fn target(&self) -> target_lexicon::Triple { + // If a compiler is configured, use that target. + #[cfg(compiler)] + return self.compiler().triple().clone(); + + // ... otherwise it's the native target + #[cfg(not(compiler))] + return target_lexicon::Triple::host(); + } + + /// Verify that this engine's configuration is compatible with loading + /// modules onto the native host platform. + /// + /// This method is used as part of `Module::new` to ensure that this + /// engine can indeed load modules for the configured compiler (if any). + /// Note that if cranelift is disabled this trivially returns `Ok` because + /// loaded serialized modules are checked separately. + pub(crate) fn check_compatible_with_native_host(&self) -> Result<()> { + #[cfg(compiler)] + { + // Check if we've already performed this check + let flag = &self.inner.compatible_with_native_host; + if flag.load(Ordering::SeqCst) { + return Ok(()); + } + + let compiler = self.compiler(); + + // Check to see that the config's target matches the host + let target = compiler.triple(); + if *target != target_lexicon::Triple::host() { + bail!( + "target '{}' specified in the configuration does not match the host", + target + ); + } + + // Also double-check all compiler settings + for (key, value) in compiler.flags().iter() { + self.check_compatible_with_shared_flag(key, value)?; + } + for (key, value) in compiler.isa_flags().iter() { + self.check_compatible_with_isa_flag(key, value)?; + } + + flag.store(true, Ordering::SeqCst); + } + Ok(()) + } + + /// Checks to see whether the "shared flag", something enabled for + /// individual compilers, is compatible with the native host platform. + /// + /// This is used both when validating an engine's compilation settings are + /// compatible with the host as well as when deserializing modules from + /// disk to ensure they're compatible with the current host. + /// + /// Note that most of the settings here are not configured by users that + /// often. While theoretically possible via `Config` methods the more + /// interesting flags are the ISA ones below. Typically the values here + /// represent global configuration for wasm features. Settings here + /// currently rely on the compiler informing us of all settings, including + /// those disabled. Settings then fall in a few buckets: + /// + /// * Some settings must be enabled, such as `avoid_div_traps`. + /// * Some settings must have a particular value, such as + /// `libcall_call_conv`. + /// * Some settings do not matter as to their value, such as `opt_level`. + pub(crate) fn check_compatible_with_shared_flag( + &self, + flag: &str, + value: &FlagValue, + ) -> Result<()> { + let ok = match flag { + // These settings must all have be enabled, since their value + // can affect the way the generated code performs or behaves at + // runtime. + "avoid_div_traps" => *value == FlagValue::Bool(true), + "unwind_info" => *value == FlagValue::Bool(true), + "libcall_call_conv" => *value == FlagValue::Enum("isa_default".into()), + + // Features wasmtime doesn't use should all be disabled, since + // otherwise if they are enabled it could change the behavior of + // generated code. + "baldrdash_prologue_words" => *value == FlagValue::Num(0), + "enable_llvm_abi_extensions" => *value == FlagValue::Bool(false), + "emit_all_ones_funcaddrs" => *value == FlagValue::Bool(false), + "enable_pinned_reg" => *value == FlagValue::Bool(false), + "enable_probestack" => *value == FlagValue::Bool(false), + "use_colocated_libcalls" => *value == FlagValue::Bool(false), + "use_pinned_reg_as_heap_base" => *value == FlagValue::Bool(false), + + // If reference types are enabled this must be enabled, otherwise + // this setting can have any value. + "enable_safepoints" => { + if self.config().features.reference_types { + *value == FlagValue::Bool(true) + } else { + return Ok(()) + } + } + + // These settings don't affect the interface or functionality of + // the module itself, so their configuration values shouldn't + // matter. + "enable_heap_access_spectre_mitigation" + | "enable_nan_canonicalization" + | "enable_jump_tables" + | "enable_float" + | "enable_simd" + | "enable_verifier" + | "is_pic" + | "machine_code_cfg_info" + | "tls_model" // wasmtime doesn't use tls right now + | "opt_level" // opt level doesn't change semantics + | "probestack_func_adjusts_sp" // probestack above asserted disabled + | "probestack_size_log2" // probestack above asserted disabled + | "regalloc" // shouldn't change semantics + | "enable_atomics" => return Ok(()), + + // Everything else is unknown and needs to be added somewhere to + // this list if encountered. + _ => { + bail!("unknown shared setting {:?} configured to {:?}", flag, value) + } + }; + + if !ok { + bail!( + "setting {:?} is configured to {:?} which is not supported", + flag, + value, + ); + } + Ok(()) + } + + /// Same as `check_compatible_with_native_host` except used for ISA-specific + /// flags. This is used to test whether a configured ISA flag is indeed + /// available on the host platform itself. + pub(crate) fn check_compatible_with_isa_flag( + &self, + flag: &str, + value: &FlagValue, + ) -> Result<()> { + match value { + // ISA flags are used for things like CPU features, so if they're + // disabled then it's compatible with the native host. + FlagValue::Bool(false) => return Ok(()), + + // Fall through below where we test at runtime that features are + // available. + FlagValue::Bool(true) => {} + + // Only `bool` values are supported right now, other settings would + // need more support here. + _ => bail!( + "isa-specific feature {:?} configured to unknown value {:?}", + flag, + value + ), + } + #[cfg(target_arch = "x86_64")] + { + let enabled = match flag { + "has_sse3" => Some(std::is_x86_feature_detected!("sse3")), + "has_ssse3" => Some(std::is_x86_feature_detected!("ssse3")), + "has_sse41" => Some(std::is_x86_feature_detected!("sse4.1")), + "has_sse42" => Some(std::is_x86_feature_detected!("sse4.2")), + "has_popcnt" => Some(std::is_x86_feature_detected!("popcnt")), + "has_avx" => Some(std::is_x86_feature_detected!("avx")), + "has_avx2" => Some(std::is_x86_feature_detected!("avx2")), + "has_bmi1" => Some(std::is_x86_feature_detected!("bmi1")), + "has_bmi2" => Some(std::is_x86_feature_detected!("bmi2")), + "has_avx512bitalg" => Some(std::is_x86_feature_detected!("avx512bitalg")), + "has_avx512dq" => Some(std::is_x86_feature_detected!("avx512dq")), + "has_avx512f" => Some(std::is_x86_feature_detected!("avx512f")), + "has_avx512vl" => Some(std::is_x86_feature_detected!("avx512vl")), + "has_avx512vbmi" => Some(std::is_x86_feature_detected!("avx512vbmi")), + "has_lzcnt" => Some(std::is_x86_feature_detected!("lzcnt")), + + // fall through to the very bottom to indicate that support is + // not enabled to test whether this feature is enabled on the + // host. + _ => None, + }; + if let Some(false) = enabled { + bail!( + "compilation setting {:?} is enabled but not available on the host", + flag + ); + } + } + bail!( + "cannot test if target-specific flag {:?} is available at runtime", + flag + ); + } } impl Default for Engine { diff --git a/crates/wasmtime/src/module.rs b/crates/wasmtime/src/module.rs index d31c3a47147a..01520f9eeef4 100644 --- a/crates/wasmtime/src/module.rs +++ b/crates/wasmtime/src/module.rs @@ -295,18 +295,9 @@ impl Module { #[cfg(compiler)] #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs pub fn from_binary(engine: &Engine, binary: &[u8]) -> Result { - // Check to see that the config's target matches the host - let target = engine.compiler().triple(); - if *target != target_lexicon::Triple::host() { - bail!( - "target '{}' specified in the configuration does not match the host", - target - ); - } - - // FIXME: we may want to validate that the ISA flags in the config match those that - // would be inferred for the host, otherwise the JIT might produce unrunnable code - // for the features the host's CPU actually has. + engine + .check_compatible_with_native_host() + .context("compilation settings are not compatible with the native host")?; cfg_if::cfg_if! { if #[cfg(feature = "cache")] { diff --git a/crates/wasmtime/src/module/serialization.rs b/crates/wasmtime/src/module/serialization.rs index 93c1a49b6714..1f3ceb1c5f1c 100644 --- a/crates/wasmtime/src/module/serialization.rs +++ b/crates/wasmtime/src/module/serialization.rs @@ -58,7 +58,7 @@ use std::convert::TryFrom; use std::path::Path; use std::str::FromStr; use std::sync::Arc; -use wasmtime_environ::{Compiler, FlagValue, Tunables}; +use wasmtime_environ::{FlagValue, Tunables}; use wasmtime_jit::{subslice_range, CompiledModule, CompiledModuleInfo, TypeTables}; use wasmtime_runtime::MmapVec; @@ -306,25 +306,11 @@ impl<'a> SerializedModule<'a> { TypeTables, Vec, )> { - // Verify that the module we're loading matches the triple that `engine` - // is configured for. If compilation is disabled within engine then the - // assumed triple is the host itself. - #[cfg(compiler)] - let engine_triple = engine.compiler().triple(); - #[cfg(not(compiler))] - let engine_triple = &target_lexicon::Triple::host(); - self.check_triple(engine_triple)?; - - // FIXME: Similar to `Module::from_binary` it should likely be validated - // here that when `cfg(not(compiler))` is true the isa/shared flags - // enabled for this precompiled module are compatible with the host - // itself, which `engine` is assumed to be running code for. - #[cfg(compiler)] - { - let compiler = engine.compiler(); - self.check_shared_flags(compiler)?; - self.check_isa_flags(compiler)?; - } + // Verify that the compilation settings in the engine match the + // compilation settings of the module that's being loaded. + self.check_triple(engine)?; + self.check_shared_flags(engine)?; + self.check_isa_flags(engine)?; self.check_tunables(&engine.config().tunables)?; self.check_features(&engine.config().features)?; @@ -501,80 +487,43 @@ impl<'a> SerializedModule<'a> { } } - fn check_triple(&self, other: &target_lexicon::Triple) -> Result<()> { - let triple = + fn check_triple(&self, engine: &Engine) -> Result<()> { + let engine_target = engine.target(); + let module_target = target_lexicon::Triple::from_str(&self.metadata.target).map_err(|e| anyhow!(e))?; - if triple.architecture != other.architecture { + if module_target.architecture != engine_target.architecture { bail!( "Module was compiled for architecture '{}'", - triple.architecture + module_target.architecture ); } - if triple.operating_system != other.operating_system { + if module_target.operating_system != engine_target.operating_system { bail!( "Module was compiled for operating system '{}'", - triple.operating_system + module_target.operating_system ); } Ok(()) } - fn check_shared_flags(&mut self, compiler: &dyn Compiler) -> Result<()> { - let mut shared_flags = std::mem::take(&mut self.metadata.shared_flags); - for (name, host) in compiler.flags() { - match shared_flags.remove(&name) { - Some(v) => { - if v != host { - bail!("Module was compiled with a different '{}' setting: expected '{}' but host has '{}'", name, v, host); - } - } - None => bail!("Module was compiled without setting '{}'", name), - } + fn check_shared_flags(&mut self, engine: &Engine) -> Result<()> { + for (name, val) in self.metadata.shared_flags.iter() { + engine + .check_compatible_with_shared_flag(name, val) + .context("compilation settings of module incompatible with native host")?; } - - for (name, _) in shared_flags { - bail!( - "Module was compiled with setting '{}' but it is not present for the host", - name - ); - } - Ok(()) } - fn check_isa_flags(&mut self, compiler: &dyn Compiler) -> Result<()> { - let mut isa_flags = std::mem::take(&mut self.metadata.isa_flags); - for (name, host) in compiler.isa_flags() { - match isa_flags.remove(&name) { - Some(v) => match (&v, &host) { - (FlagValue::Bool(v), FlagValue::Bool(host)) => { - // ISA flags represent CPU features; for boolean values, only - // treat it as an error if the module was compiled with the setting enabled - // but the host does not have it enabled. - if *v && !*host { - bail!("Module was compiled with setting '{}' enabled but the host does not support it", name); - } - } - _ => { - if v != host { - bail!("Module was compiled with a different '{}' setting: expected '{}' but host has '{}'", name, v, host); - } - } - }, - None => bail!("Module was compiled without setting '{}'", name), - } + fn check_isa_flags(&mut self, engine: &Engine) -> Result<()> { + for (name, val) in self.metadata.isa_flags.iter() { + engine + .check_compatible_with_isa_flag(name, val) + .context("compilation settings of module incompatible with native host")?; } - - for (name, _) in isa_flags { - bail!( - "Module was compiled with setting '{}' but it is not present for the host", - name - ); - } - Ok(()) } @@ -758,7 +707,6 @@ fn align_to(val: usize, align: usize) -> usize { mod test { use super::*; use crate::Config; - use std::borrow::Cow; #[test] fn test_architecture_mismatch() -> Result<()> { @@ -807,16 +755,20 @@ mod test { let module = Module::new(&engine, "(module)")?; let mut serialized = SerializedModule::new(&module); - serialized.metadata.shared_flags.insert( - "opt_level".to_string(), - FlagValue::Enum(Cow::Borrowed("none")), - ); + serialized + .metadata + .shared_flags + .insert("avoid_div_traps".to_string(), FlagValue::Bool(false)); match serialized.into_module(&engine) { Ok(_) => unreachable!(), Err(e) => assert_eq!( - e.to_string(), - "Module was compiled with a different 'opt_level' setting: expected 'none' but host has 'speed'" + format!("{:?}", e), + "\ +compilation settings of module incompatible with native host + +Caused by: + setting \"avoid_div_traps\" is configured to Bool(false) which is not supported" ), } @@ -838,8 +790,12 @@ mod test { match serialized.into_module(&engine) { Ok(_) => unreachable!(), Err(e) => assert_eq!( - e.to_string(), - "Module was compiled with setting 'not_a_flag' but it is not present for the host", + format!("{:?}", e), + "\ +compilation settings of module incompatible with native host + +Caused by: + cannot test if target-specific flag \"not_a_flag\" is available at runtime", ), } diff --git a/tests/all/wast.rs b/tests/all/wast.rs index 588400d0bb48..240a68fb72fb 100644 --- a/tests/all/wast.rs +++ b/tests/all/wast.rs @@ -12,6 +12,10 @@ include!(concat!(env!("OUT_DIR"), "/wast_testsuite_tests.rs")); // function which actually executes the `wast` test suite given the `strategy` // to compile it. fn run_wast(wast: &str, strategy: Strategy, pooling: bool) -> anyhow::Result<()> { + match strategy { + Strategy::Cranelift => {} + _ => unimplemented!(), + } let wast = Path::new(wast); let simd = feature_found(wast, "simd"); @@ -26,7 +30,6 @@ fn run_wast(wast: &str, strategy: Strategy, pooling: bool) -> anyhow::Result<()> .wasm_module_linking(module_linking) .wasm_threads(threads) .wasm_memory64(memory64) - .strategy(strategy)? .cranelift_debug_verifier(true); if feature_found(wast, "canonicalize-nan") {