Implement runtime checks for compilation settings (#3899)

* 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

* Fix fall-through logic to actually be correct

* Use a `OnceCell`, not an `AtomicBool`

* Fix some broken tests

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>),

crates/wasmtime/src/engine.rs

@@ -1,12 +1,14 @@
 use crate::signatures::SignatureRegistry;
 use crate::{Config, Trap};
 use anyhow::Result;
+use once_cell::sync::OnceCell;
 #[cfg(feature = "parallel-compilation")]
 use rayon::prelude::*;
 use std::sync::atomic::{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 +46,10 @@ struct EngineInner {
     signatures: SignatureRegistry,
     epoch: AtomicU64,
     unique_id_allocator: CompiledModuleIdAllocator,
+
+    // One-time check of whether the compiler's settings, if present, are
+    // compatible with the native host.
+    compatible_with_native_host: OnceCell<Result<(), String>>,
 }
 
 impl Engine {
@@ -70,6 +76,7 @@ impl Engine {
                 signatures: registry,
                 epoch: AtomicU64::new(0),
                 unique_id_allocator: CompiledModuleIdAllocator::new(),
+                compatible_with_native_host: OnceCell::new(),
             }),
         })
     }
@@ -217,6 +224,211 @@ impl Engine {
             .map(|a| f(a))
             .collect::<Result<Vec<B>, 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<()> {
+        self.inner
+            .compatible_with_native_host
+            .get_or_init(|| self._check_compatible_with_native_host())
+            .clone()
+            .map_err(anyhow::Error::msg)
+    }
+    fn _check_compatible_with_native_host(&self) -> Result<(), String> {
+        #[cfg(compiler)]
+        {
+            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() {
+                return Err(format!(
+                    "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)?;
+            }
+        }
+        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<(), String> {
+        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.
+            _ => {
+                return Err(format!("unknown shared setting {:?} configured to {:?}", flag, value))
+            }
+        };
+
+        if !ok {
+            return Err(format!(
+                "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<(), String> {
+        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.
+            _ => {
+                return Err(format!(
+                    "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,
+            };
+            match enabled {
+                Some(true) => return Ok(()),
+                Some(false) => {
+                    return Err(format!(
+                        "compilation setting {:?} is enabled but not available on the host",
+                        flag
+                    ))
+                }
+                // fall through
+                None => {}
+            }
+        }
+        Err(format!(
+            "cannot test if target-specific flag {:?} is available at runtime",
+            flag
+        ))
+    }
 }
 
 impl Default for Engine {

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<Module> {
-        // Check to see that the config's target matches the host
+        engine
-        let target = engine.compiler().triple();
+            .check_compatible_with_native_host()
-        if *target != target_lexicon::Triple::host() {
+            .context("compilation settings are not compatible with the native 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.
 
         cfg_if::cfg_if! {
             if #[cfg(feature = "cache")] {

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<SerializedModuleUpvar>,
     )> {
-        // Verify that the module we're loading matches the triple that `engine`
+        // Verify that the compilation settings in the engine match the
-        // is configured for. If compilation is disabled within engine then the
+        // compilation settings of the module that's being loaded.
-        // assumed triple is the host itself.
+        self.check_triple(engine)?;
-        #[cfg(compiler)]
+        self.check_shared_flags(engine)?;
-        let engine_triple = engine.compiler().triple();
+        self.check_isa_flags(engine)?;
-        #[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)?;
-        }
 
         self.check_tunables(&engine.config().tunables)?;
         self.check_features(&engine.config().features)?;
@@ -501,80 +487,45 @@ impl<'a> SerializedModule<'a> {
         }
     }
 
-    fn check_triple(&self, other: &target_lexicon::Triple) -> Result<()> {
+    fn check_triple(&self, engine: &Engine) -> Result<()> {
-        let triple =
+        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<()> {
+    fn check_shared_flags(&mut self, engine: &Engine) -> Result<()> {
-        let mut shared_flags = std::mem::take(&mut self.metadata.shared_flags);
+        for (name, val) in self.metadata.shared_flags.iter() {
-        for (name, host) in compiler.flags() {
+            engine
-            match shared_flags.remove(&name) {
+                .check_compatible_with_shared_flag(name, val)
-                Some(v) => {
+                .map_err(|s| anyhow::Error::msg(s))
-                    if v != host {
+                .context("compilation settings of module incompatible with native host")?;
-                        bail!("Module was compiled with a different '{}' setting: expected '{}' but host has '{}'", name, v, host);
         }
-                }
-                None => bail!("Module was compiled without setting '{}'", name),
-            }
-        }
-
-        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<()> {
+    fn check_isa_flags(&mut self, engine: &Engine) -> Result<()> {
-        let mut isa_flags = std::mem::take(&mut self.metadata.isa_flags);
+        for (name, val) in self.metadata.isa_flags.iter() {
-        for (name, host) in compiler.isa_flags() {
+            engine
-            match isa_flags.remove(&name) {
+                .check_compatible_with_isa_flag(name, val)
-                Some(v) => match (&v, &host) {
+                .map_err(|s| anyhow::Error::msg(s))
-                    (FlagValue::Bool(v), FlagValue::Bool(host)) => {
+                .context("compilation settings of module incompatible with native 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),
-            }
-        }
-
-        for (name, _) in isa_flags {
-            bail!(
-                "Module was compiled with setting '{}' but it is not present for the host",
-                name
-            );
-        }
-
         Ok(())
     }
 
@@ -758,7 +709,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 +757,20 @@ mod test {
         let module = Module::new(&engine, "(module)")?;
 
         let mut serialized = SerializedModule::new(&module);
-        serialized.metadata.shared_flags.insert(
+        serialized
-            "opt_level".to_string(),
+            .metadata
-            FlagValue::Enum(Cow::Borrowed("none")),
+            .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(),
+                format!("{:?}", e),
-                "Module was compiled with a different 'opt_level' setting: expected 'none' but host has 'speed'"
+                "\
+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 +792,12 @@ mod test {
         match serialized.into_module(&engine) {
             Ok(_) => unreachable!(),
             Err(e) => assert_eq!(
-                e.to_string(),
+                format!("{:?}", e),
-                "Module was compiled with setting 'not_a_flag' but it is not present for the host",
+                "\
+compilation settings of module incompatible with native host
+
+Caused by:
+    cannot test if target-specific flag \"not_a_flag\" is available at runtime",
             ),
         }
 

tests/all/module.rs

@@ -10,9 +10,11 @@ fn checks_incompatible_target() -> Result<()> {
         "(module)",
     ) {
         Ok(_) => unreachable!(),
-        Err(e) => assert!(e
+        Err(e) => assert!(
-            .to_string()
+            format!("{:?}", e).contains("configuration does not match the host"),
-            .contains("configuration does not match the host")),
+            "bad error: {:?}",
+            e
+        ),
     }
 
     Ok(())
@@ -31,33 +33,20 @@ fn caches_across_engines() {
         let res = Module::deserialize(&Engine::default(), &bytes);
         assert!(res.is_ok());
 
-        // differ in shared cranelift flags
+        // differ in runtime settings
         let res = Module::deserialize(
-            &Engine::new(Config::new().cranelift_nan_canonicalization(true)).unwrap(),
+            &Engine::new(Config::new().static_memory_maximum_size(0)).unwrap(),
             &bytes,
         );
         assert!(res.is_err());
 
-        // differ in cranelift settings
+        // differ in wasm features enabled (which can affect
+        // runtime/compilation settings)
         let res = Module::deserialize(
-            &Engine::new(Config::new().cranelift_opt_level(OptLevel::None)).unwrap(),
+            &Engine::new(Config::new().wasm_simd(false)).unwrap(),
             &bytes,
         );
         assert!(res.is_err());
-
-        // Missing required cpu flags
-        if cfg!(target_arch = "x86_64") {
-            let res = Module::deserialize(
-                &Engine::new(
-                    Config::new()
-                        .target(&target_lexicon::Triple::host().to_string())
-                        .unwrap(),
-                )
-                .unwrap(),
-                &bytes,
-            );
-            assert!(res.is_err());
-        }
     }
 }
 

tests/all/module_serialize.rs

@@ -66,7 +66,7 @@ fn test_module_serialize_fail() -> Result<()> {
     )?;
 
     let mut config = Config::new();
-    config.cranelift_opt_level(OptLevel::None);
+    config.static_memory_maximum_size(0);
     let mut store = Store::new(&Engine::new(&config)?, ());
     match unsafe { deserialize_and_instantiate(&mut store, &buffer) } {
         Ok(_) => bail!("expected failure at deserialization"),

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") {