diff --git a/RELEASES.md b/RELEASES.md index 035213f4b003..768a3566f928 100644 --- a/RELEASES.md +++ b/RELEASES.md @@ -8,6 +8,17 @@ Unreleased. ### Changed +* The `Config::strategy`, `Config::cranelift_flag_enable`, `Config::cranelift_flag_set` + and `Config::profiler` APIs now return `&mut Self` instead of `Result<&mut Self>` + since the validation is deferred until `Engine::new`. + [#4252](https://github.com/bytecodealliance/wasmtime/pull/4252) + +### Fixed + +* A refactor of `Config` was made to fix an issue that the order of calls to `Config` + matters now, which may lead to unexpected behavior. + [#4252](https://github.com/bytecodealliance/wasmtime/pull/4252) + -------------------------------------------------------------------------------- ## 0.38.0 diff --git a/crates/c-api/include/wasmtime/config.h b/crates/c-api/include/wasmtime/config.h index dcef84fa6a67..2d04ca7179b1 100644 --- a/crates/c-api/include/wasmtime/config.h +++ b/crates/c-api/include/wasmtime/config.h @@ -201,7 +201,7 @@ WASMTIME_CONFIG_PROP(void, wasm_memory64, bool) * If the compilation strategy selected could not be enabled then an error is * returned. */ -WASMTIME_CONFIG_PROP(wasmtime_error_t*, strategy, wasmtime_strategy_t) +WASMTIME_CONFIG_PROP(void, strategy, wasmtime_strategy_t) /** * \brief Configures whether Cranelift's debug verifier is enabled. @@ -238,7 +238,7 @@ WASMTIME_CONFIG_PROP(void, cranelift_opt_level, wasmtime_opt_level_t) * * This setting in #WASMTIME_PROFILING_STRATEGY_NONE by default. */ -WASMTIME_CONFIG_PROP(wasmtime_error_t*, profiler, wasmtime_profiling_strategy_t) +WASMTIME_CONFIG_PROP(void, profiler, wasmtime_profiling_strategy_t) /** * \brief Configures the maximum size for memory to be considered "static" diff --git a/crates/c-api/src/config.rs b/crates/c-api/src/config.rs index f928ff39fb36..ee973892903a 100644 --- a/crates/c-api/src/config.rs +++ b/crates/c-api/src/config.rs @@ -103,13 +103,12 @@ pub extern "C" fn wasmtime_config_wasm_memory64_set(c: &mut wasm_config_t, enabl pub extern "C" fn wasmtime_config_strategy_set( c: &mut wasm_config_t, strategy: wasmtime_strategy_t, -) -> Option> { +) { use wasmtime_strategy_t::*; - let result = c.config.strategy(match strategy { + c.config.strategy(match strategy { WASMTIME_STRATEGY_AUTO => Strategy::Auto, WASMTIME_STRATEGY_CRANELIFT => Strategy::Cranelift, }); - handle_result(result, |_cfg| {}) } #[no_mangle] @@ -145,13 +144,12 @@ pub extern "C" fn wasmtime_config_cranelift_opt_level_set( pub extern "C" fn wasmtime_config_profiler_set( c: &mut wasm_config_t, strategy: wasmtime_profiling_strategy_t, -) -> Option> { +) { use wasmtime_profiling_strategy_t::*; - let result = c.config.profiler(match strategy { + c.config.profiler(match strategy { WASMTIME_PROFILING_STRATEGY_NONE => ProfilingStrategy::None, WASMTIME_PROFILING_STRATEGY_JITDUMP => ProfilingStrategy::JitDump, }); - handle_result(result, |_cfg| {}) } #[no_mangle] diff --git a/crates/cli-flags/src/lib.rs b/crates/cli-flags/src/lib.rs index 532fd3213be4..49ad0ce06a9f 100644 --- a/crates/cli-flags/src/lib.rs +++ b/crates/cli-flags/src/lib.rs @@ -260,20 +260,20 @@ impl CommonOptions { .cranelift_debug_verifier(self.enable_cranelift_debug_verifier) .debug_info(self.debug_info) .cranelift_opt_level(self.opt_level()) - .profiler(pick_profiling_strategy(self.jitdump, self.vtune)?)? + .profiler(pick_profiling_strategy(self.jitdump, self.vtune)?) .cranelift_nan_canonicalization(self.enable_cranelift_nan_canonicalization); self.enable_wasm_features(&mut config); for name in &self.cranelift_enable { unsafe { - config.cranelift_flag_enable(name)?; + config.cranelift_flag_enable(name); } } for (name, value) in &self.cranelift_set { unsafe { - config.cranelift_flag_set(name, value)?; + config.cranelift_flag_set(name, value); } } diff --git a/crates/cranelift/src/builder.rs b/crates/cranelift/src/builder.rs index 9ccc6f64ca0f..c810e3498d9e 100644 --- a/crates/cranelift/src/builder.rs +++ b/crates/cranelift/src/builder.rs @@ -9,7 +9,6 @@ use cranelift_codegen::settings::{self, Configurable, SetError}; use std::fmt; use wasmtime_environ::{CompilerBuilder, Setting, SettingKind}; -#[derive(Clone)] struct Builder { flags: settings::Builder, isa_flags: isa::Builder, @@ -55,10 +54,6 @@ impl CompilerBuilder for Builder { self.isa_flags.triple() } - fn clone(&self) -> Box { - Box::new(Clone::clone(self)) - } - fn target(&mut self, target: target_lexicon::Triple) -> Result<()> { self.isa_flags = isa::lookup(target)?; Ok(()) diff --git a/crates/environ/src/compilation.rs b/crates/environ/src/compilation.rs index a50257a566c8..3713868f302a 100644 --- a/crates/environ/src/compilation.rs +++ b/crates/environ/src/compilation.rs @@ -76,9 +76,6 @@ pub enum CompileError { /// This is used in Wasmtime to separate compiler implementations, currently /// mostly used to separate Cranelift from Wasmtime itself. pub trait CompilerBuilder: Send + Sync + fmt::Debug { - /// Like the `Clone` trait, but for the boxed trait object. - fn clone(&self) -> Box; - /// Sets the target of compilation to the target specified. fn target(&mut self, target: target_lexicon::Triple) -> Result<()>; diff --git a/crates/fuzzing/src/generators.rs b/crates/fuzzing/src/generators.rs index 74bdd7f423e0..ee0e0f509176 100644 --- a/crates/fuzzing/src/generators.rs +++ b/crates/fuzzing/src/generators.rs @@ -421,8 +421,7 @@ impl Config { if self.wasmtime.force_jump_veneers { unsafe { - cfg.cranelift_flag_set("wasmtime_linkopt_force_jump_veneer", "true") - .unwrap(); + cfg.cranelift_flag_set("wasmtime_linkopt_force_jump_veneer", "true"); } } @@ -431,8 +430,7 @@ impl Config { cfg.cranelift_flag_set( "wasmtime_linkopt_padding_between_functions", &pad.to_string(), - ) - .unwrap(); + ); } } @@ -658,7 +656,7 @@ impl CodegenSettings { config.target(target).unwrap(); for (key, value) in flags { unsafe { - config.cranelift_flag_set(key, value).unwrap(); + config.cranelift_flag_set(key, value); } } } diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs index ed800bacafb3..e6c46d56af52 100644 --- a/crates/wasmtime/src/config.rs +++ b/crates/wasmtime/src/config.rs @@ -2,15 +2,18 @@ use crate::memory::MemoryCreator; use crate::trampoline::MemoryCreatorProxy; use anyhow::{bail, Result}; use serde::{Deserialize, Serialize}; -use std::cmp; use std::fmt; #[cfg(feature = "cache")] use std::path::Path; use std::sync::Arc; +use std::{ + cmp, + collections::{HashMap, HashSet}, +}; use wasmparser::WasmFeatures; #[cfg(feature = "cache")] use wasmtime_cache::CacheConfig; -use wasmtime_environ::{CompilerBuilder, Tunables}; +use wasmtime_environ::Tunables; use wasmtime_jit::{JitDumpAgent, NullProfilerAgent, ProfilingAgent, VTuneAgent}; use wasmtime_runtime::{InstanceAllocator, OnDemandInstanceAllocator, RuntimeMemoryCreator}; @@ -79,14 +82,18 @@ impl Default for ModuleVersionStrategy { /// and customize its behavior. /// /// This structure exposed a builder-like interface and is primarily consumed by -/// [`Engine::new()`](crate::Engine::new) +/// [`Engine::new()`](crate::Engine::new). +/// +/// The validation of `Config` is deferred until the engine is being built, thus +/// a problematic config may cause `Engine::new` to fail. +#[derive(Clone)] pub struct Config { #[cfg(compiler)] - pub(crate) compiler: Box, + compiler_config: CompilerConfig, pub(crate) tunables: Tunables, #[cfg(feature = "cache")] pub(crate) cache_config: CacheConfig, - pub(crate) profiler: Arc, + pub(crate) profiling_strategy: ProfilingStrategy, pub(crate) mem_creator: Option>, pub(crate) allocation_strategy: InstanceAllocationStrategy, pub(crate) max_wasm_stack: usize, @@ -103,6 +110,54 @@ pub struct Config { pub(crate) force_memory_init_memfd: bool, } +/// User-provided configuration for the compiler. +#[cfg(compiler)] +#[derive(Debug, Clone)] +struct CompilerConfig { + strategy: Strategy, + target: Option, + settings: HashMap, + flags: HashSet, +} + +#[cfg(compiler)] +impl CompilerConfig { + fn new(strategy: Strategy) -> Self { + Self { + strategy, + target: None, + settings: HashMap::new(), + flags: HashSet::new(), + } + } + + /// Ensures that the key is not set or equals to the given value. + /// If the key is not set, it will be set to the given value. + /// + /// # Returns + /// + /// Returns true if successfully set or already had the given setting + /// value, or false if the setting was explicitly set to something + /// else previously. + fn ensure_setting_unset_or_given(&mut self, k: &str, v: &str) -> bool { + if let Some(value) = self.settings.get(k) { + if value != v { + return false; + } + } else { + self.settings.insert(k.to_string(), v.to_string()); + } + true + } +} + +#[cfg(compiler)] +impl Default for CompilerConfig { + fn default() -> Self { + Self::new(Strategy::Auto) + } +} + impl Config { /// Creates a new configuration object with the default configuration /// specified. @@ -110,10 +165,10 @@ impl Config { let mut ret = Self { tunables: Tunables::default(), #[cfg(compiler)] - compiler: compiler_builder(Strategy::Auto).unwrap(), + compiler_config: CompilerConfig::default(), #[cfg(feature = "cache")] cache_config: CacheConfig::new_cache_disabled(), - profiler: Arc::new(NullProfilerAgent), + profiling_strategy: ProfilingStrategy::None, mem_creator: None, allocation_strategy: InstanceAllocationStrategy::OnDemand, // 512k of stack -- note that this is chosen currently to not be too @@ -166,8 +221,8 @@ impl Config { #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs pub fn target(&mut self, target: &str) -> Result<&mut Self> { use std::str::FromStr; - self.compiler - .target(target_lexicon::Triple::from_str(target).map_err(|e| anyhow::anyhow!(e))?)?; + self.compiler_config.target = + Some(target_lexicon::Triple::from_str(target).map_err(|e| anyhow::anyhow!(e))?); Ok(self) } @@ -290,20 +345,6 @@ impl Config { /// will always return `None`. pub fn wasm_backtrace(&mut self, enable: bool) -> &mut Self { self.wasm_backtrace = enable; - #[cfg(compiler)] - { - // unwind_info must be enabled when either backtraces or reference types are enabled: - self.compiler - .set( - "unwind_info", - if enable || self.features.reference_types { - "true" - } else { - "false" - }, - ) - .unwrap(); - } self } @@ -507,8 +548,8 @@ impl Config { /// be enabled through this method for appropriate wasm modules. /// /// This feature gates items such as shared memories and atomic - /// instructions. Note that enabling the threads feature will - /// also enable the bulk memory feature. + /// instructions. Note that the threads feature depends on the + /// bulk memory feature, which is enabled by default. /// /// This is `false` by default. /// @@ -517,13 +558,14 @@ impl Config { /// > expected. This should not be enabled in a production setting right /// > now. /// + /// # Errors + /// + /// The validation of this feature are deferred until the engine is being built, + /// and thus may cause `Engine::new` fail if the `bulk_memory` feature is disabled. + /// /// [threads]: https://github.com/webassembly/threads pub fn wasm_threads(&mut self, enable: bool) -> &mut Self { self.features.threads = enable; - // The threads proposal depends on the bulk memory proposal - if enable { - self.wasm_bulk_memory(true); - } self } @@ -533,38 +575,18 @@ impl Config { /// This feature gates items such as the `externref` and `funcref` types as /// well as allowing a module to define multiple tables. /// - /// Note that enabling the reference types feature will also enable the bulk - /// memory feature. + /// Note that the reference types proposal depends on the bulk memory proposal. /// /// This feature is `true` by default. /// + /// # Errors + /// + /// The validation of this feature are deferred until the engine is being built, + /// and thus may cause `Engine::new` fail if the `bulk_memory` feature is disabled. + /// /// [proposal]: https://github.com/webassembly/reference-types pub fn wasm_reference_types(&mut self, enable: bool) -> &mut Self { self.features.reference_types = enable; - - #[cfg(compiler)] - { - self.compiler - .set("enable_safepoints", if enable { "true" } else { "false" }) - .unwrap(); - // unwind_info must be enabled when either backtraces or reference types are enabled: - self.compiler - .set( - "unwind_info", - if enable || self.wasm_backtrace { - "true" - } else { - "false" - }, - ) - .unwrap(); - } - - // The reference types proposal depends on the bulk memory proposal. - if enable { - self.wasm_bulk_memory(true); - } - self } @@ -586,13 +608,6 @@ impl Config { /// [relaxed simd proposal]: https://github.com/WebAssembly/relaxed-simd pub fn wasm_simd(&mut self, enable: bool) -> &mut Self { self.features.simd = enable; - #[cfg(compiler)] - { - let val = if enable { "true" } else { "false" }; - self.compiler - .set("enable_simd", val) - .expect("should be valid flag"); - } self } @@ -604,6 +619,15 @@ impl Config { /// /// This is `true` by default. /// + /// Feature `reference_types`, which is also `true` by default, requires + /// this feature to be enabled. Thus disabling this feature must also disable + /// `reference_types` as well using [`wasm_reference_types`](crate::Config::wasm_reference_types). + /// + /// # Errors + /// + /// Disabling this feature without disabling `reference_types` will cause + /// `Engine::new` to fail. + /// /// [proposal]: https://github.com/webassembly/bulk-memory-operations pub fn wasm_bulk_memory(&mut self, enable: bool) -> &mut Self { self.features.bulk_memory = enable; @@ -674,30 +698,30 @@ impl Config { /// and its documentation. /// /// The default value for this is `Strategy::Auto`. - /// - /// # Errors - /// - /// Some compilation strategies require compile-time options of `wasmtime` - /// itself to be set, but if they're not set and the strategy is specified - /// here then an error will be returned. #[cfg(compiler)] #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs - pub fn strategy(&mut self, strategy: Strategy) -> Result<&mut Self> { - self.compiler = compiler_builder(strategy)?; - Ok(self) + pub fn strategy(&mut self, strategy: Strategy) -> &mut Self { + self.compiler_config.strategy = strategy; + self } /// Creates a default profiler based on the profiling strategy chosen. /// /// Profiler creation calls the type's default initializer where the purpose is /// really just to put in place the type used for profiling. - pub fn profiler(&mut self, profile: ProfilingStrategy) -> Result<&mut Self> { - self.profiler = match profile { - ProfilingStrategy::JitDump => Arc::new(JitDumpAgent::new()?) as Arc, - ProfilingStrategy::VTune => Arc::new(VTuneAgent::new()?) as Arc, - ProfilingStrategy::None => Arc::new(NullProfilerAgent), - }; - Ok(self) + /// + /// Some [`ProfilingStrategy`] require specific platforms or particular feature + /// to be enabled, such as `ProfilingStrategy::JitDump` requires the `jitdump` + /// feature. + /// + /// # Errors + /// + /// The validation of this field is deferred until the engine is being built, and thus may + /// cause `Engine::new` fail if the required feature is disabled, or the platform is not + /// supported. + pub fn profiler(&mut self, profile: ProfilingStrategy) -> &mut Self { + self.profiling_strategy = profile; + self } /// Configures whether the debug verifier of Cranelift is enabled or not. @@ -712,9 +736,9 @@ impl Config { #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs pub fn cranelift_debug_verifier(&mut self, enable: bool) -> &mut Self { let val = if enable { "true" } else { "false" }; - self.compiler - .set("enable_verifier", val) - .expect("should be valid flag"); + self.compiler_config + .settings + .insert("enable_verifier".to_string(), val.to_string()); self } @@ -733,9 +757,9 @@ impl Config { OptLevel::Speed => "speed", OptLevel::SpeedAndSize => "speed_and_size", }; - self.compiler - .set("opt_level", val) - .expect("should be valid flag"); + self.compiler_config + .settings + .insert("opt_level".to_string(), val.to_string()); self } @@ -751,9 +775,9 @@ impl Config { #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs pub fn cranelift_nan_canonicalization(&mut self, enable: bool) -> &mut Self { let val = if enable { "true" } else { "false" }; - self.compiler - .set("enable_nan_canonicalization", val) - .expect("should be valid flag"); + self.compiler_config + .settings + .insert("enable_nan_canonicalization".to_string(), val.to_string()); self } @@ -770,12 +794,14 @@ impl Config { /// /// # Errors /// - /// This method can fail if the flag's name does not exist. + /// The validation of the flags are deferred until the engine is being built, and thus may + /// cause `Engine::new` fail if the flag's name does not exist, or the value is not appropriate + /// for the flag type. #[cfg(compiler)] #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs - pub unsafe fn cranelift_flag_enable(&mut self, flag: &str) -> Result<&mut Self> { - self.compiler.enable(flag)?; - Ok(self) + pub unsafe fn cranelift_flag_enable(&mut self, flag: &str) -> &mut Self { + self.compiler_config.flags.insert(flag.to_string()); + self } /// Allows settings another Cranelift flag defined by a flag name and value. This allows @@ -784,18 +810,26 @@ impl Config { /// Since Cranelift flags may be unstable, this method should not be considered to be stable /// either; other `Config` functions should be preferred for stability. /// - /// Note that this is marked as unsafe, because setting the wrong flag might break invariants, + /// # Safety + /// + /// This is marked as unsafe, because setting the wrong flag might break invariants, /// resulting in execution hazards. /// /// # Errors /// - /// This method can fail if the flag's name does not exist, or the value is not appropriate for - /// the flag type. + /// The validation of the flags are deferred until the engine is being built, and thus may + /// cause `Engine::new` fail if the flag's name does not exist, or incompatible with other + /// settings. + /// + /// For example, feature `wasm_backtrace` will set `unwind_info` to `true`, but if it's + /// manually set to false then it will fail. #[cfg(compiler)] #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs - pub unsafe fn cranelift_flag_set(&mut self, name: &str, value: &str) -> Result<&mut Self> { - self.compiler.set(name, value)?; - Ok(self) + pub unsafe fn cranelift_flag_set(&mut self, name: &str, value: &str) -> &mut Self { + self.compiler_config + .settings + .insert(name.to_string(), value.to_string()); + self } /// Loads cache configuration specified at `path`. @@ -1294,6 +1328,16 @@ impl Config { self } + pub(crate) fn validate(&self) -> Result<()> { + if self.features.reference_types && !self.features.bulk_memory { + bail!("feature 'reference_types' requires 'bulk_memory' to be enabled"); + } + if self.features.threads && !self.features.bulk_memory { + bail!("feature 'threads' requires 'bulk_memory' to be enabled"); + } + Ok(()) + } + pub(crate) fn build_allocator(&self) -> Result> { #[cfg(feature = "async")] let stack_size = self.async_stack_size; @@ -1318,12 +1362,59 @@ impl Config { )?)), } } -} -#[cfg(compiler)] -fn compiler_builder(strategy: Strategy) -> Result> { - match strategy { - Strategy::Auto | Strategy::Cranelift => Ok(wasmtime_cranelift::builder()), + pub(crate) fn build_profiler(&self) -> Result> { + Ok(match self.profiling_strategy { + ProfilingStrategy::JitDump => Box::new(JitDumpAgent::new()?) as Box, + ProfilingStrategy::VTune => Box::new(VTuneAgent::new()?) as Box, + ProfilingStrategy::None => Box::new(NullProfilerAgent), + }) + } + + #[cfg(compiler)] + pub(crate) fn build_compiler(&mut self) -> Result> { + let mut compiler = match self.compiler_config.strategy { + Strategy::Auto | Strategy::Cranelift => wasmtime_cranelift::builder(), + }; + if let Some(target) = &self.compiler_config.target { + compiler.target(target.clone())?; + } + + // check for incompatible compiler options and set required values + if self.wasm_backtrace || self.features.reference_types { + if !self + .compiler_config + .ensure_setting_unset_or_given("unwind_info", "true") + { + bail!("compiler option 'unwind_info' must be enabled when either 'backtraces' or 'reference types' are enabled"); + } + } + if self.features.reference_types { + if !self + .compiler_config + .ensure_setting_unset_or_given("enable_safepoints", "true") + { + bail!("compiler option 'enable_safepoints' must be enabled when 'reference types' is enabled"); + } + } + if self.features.simd { + if !self + .compiler_config + .ensure_setting_unset_or_given("enable_simd", "true") + { + bail!("compiler option 'enable_simd' must be enabled when 'simd' is enabled"); + } + } + + // Apply compiler settings and flags + for (k, v) in self.compiler_config.settings.iter() { + compiler.set(k, v)?; + } + for flag in self.compiler_config.flags.iter() { + compiler.enable(flag)?; + } + + compiler.build() } } @@ -1332,7 +1423,7 @@ fn round_up_to_pages(val: u64) -> u64 { debug_assert!(page_size.is_power_of_two()); val.checked_add(page_size - 1) .map(|val| val & !(page_size - 1)) - .unwrap_or(u64::max_value() / page_size + 1) + .unwrap_or(u64::MAX / page_size + 1) } impl Default for Config { @@ -1341,33 +1432,6 @@ impl Default for Config { } } -impl Clone for Config { - fn clone(&self) -> Config { - Config { - #[cfg(compiler)] - compiler: self.compiler.clone(), - tunables: self.tunables.clone(), - #[cfg(feature = "cache")] - cache_config: self.cache_config.clone(), - profiler: self.profiler.clone(), - features: self.features.clone(), - mem_creator: self.mem_creator.clone(), - allocation_strategy: self.allocation_strategy.clone(), - max_wasm_stack: self.max_wasm_stack, - wasm_backtrace: self.wasm_backtrace, - wasm_backtrace_details_env_used: self.wasm_backtrace_details_env_used, - async_support: self.async_support, - #[cfg(feature = "async")] - async_stack_size: self.async_stack_size, - module_version: self.module_version.clone(), - parallel_compilation: self.parallel_compilation, - memory_init_cow: self.memory_init_cow, - memory_guaranteed_dense_image_size: self.memory_guaranteed_dense_image_size, - force_memory_init_memfd: self.force_memory_init_memfd, - } - } -} - impl fmt::Debug for Config { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let mut f = f.debug_struct("Config"); @@ -1398,7 +1462,7 @@ impl fmt::Debug for Config { .field("parallel_compilation", &self.parallel_compilation); #[cfg(compiler)] { - f.field("compiler", &self.compiler); + f.field("compiler_config", &self.compiler_config); } f.finish() } @@ -1408,7 +1472,7 @@ impl fmt::Debug for Config { /// /// This is used as an argument to the [`Config::strategy`] method. #[non_exhaustive] -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Copy)] pub enum Strategy { /// An indicator that the compilation strategy should be automatically /// selected. diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs index fbef9592313f..b0361ad3cd17 100644 --- a/crates/wasmtime/src/engine.rs +++ b/crates/wasmtime/src/engine.rs @@ -9,6 +9,7 @@ use std::sync::Arc; #[cfg(feature = "cache")] use wasmtime_cache::CacheConfig; use wasmtime_environ::FlagValue; +use wasmtime_jit::ProfilingAgent; use wasmtime_runtime::{debug_builtins, CompiledModuleIdAllocator, InstanceAllocator}; /// An `Engine` which is a global context for compilation and management of wasm @@ -43,6 +44,7 @@ struct EngineInner { #[cfg(compiler)] compiler: Box, allocator: Box, + profiler: Box, signatures: SignatureRegistry, epoch: AtomicU64, unique_id_allocator: CompiledModuleIdAllocator, @@ -55,6 +57,16 @@ struct EngineInner { impl Engine { /// Creates a new [`Engine`] with the specified compilation and /// configuration settings. + /// + /// # Errors + /// + /// This method can fail if the `config` is invalid or some + /// configurations are incompatible. + /// + /// For example, feature `reference_types` will need to set + /// the compiler setting `enable_safepoints` and `unwind_info` + /// to `true`, but explicitly disable these two compiler settings + /// will cause errors. pub fn new(config: &Config) -> Result { // Ensure that wasmtime_runtime's signal handlers are configured. This // is the per-program initialization required for handling traps, such @@ -64,15 +76,22 @@ impl Engine { let registry = SignatureRegistry::new(); let mut config = config.clone(); + config.validate()?; + + #[cfg(compiler)] + let compiler = config.build_compiler()?; + let allocator = config.build_allocator()?; allocator.adjust_tunables(&mut config.tunables); + let profiler = config.build_profiler()?; Ok(Engine { inner: Arc::new(EngineInner { #[cfg(compiler)] - compiler: config.compiler.build()?, + compiler, config, allocator, + profiler, signatures: registry, epoch: AtomicU64::new(0), unique_id_allocator: CompiledModuleIdAllocator::new(), @@ -117,6 +136,10 @@ impl Engine { self.inner.allocator.as_ref() } + pub(crate) fn profiler(&self) -> &dyn ProfilingAgent { + self.inner.profiler.as_ref() + } + #[cfg(feature = "cache")] pub(crate) fn cache_config(&self) -> &CacheConfig { &self.config().cache_config diff --git a/crates/wasmtime/src/module.rs b/crates/wasmtime/src/module.rs index 547e66bd7e1c..1ee02855830b 100644 --- a/crates/wasmtime/src/module.rs +++ b/crates/wasmtime/src/module.rs @@ -493,7 +493,7 @@ impl Module { let module = Arc::new(CompiledModule::from_artifacts( mmap, info, - &*engine.config().profiler, + engine.profiler(), engine.unique_id_allocator(), )?); diff --git a/crates/wasmtime/src/trampoline/func.rs b/crates/wasmtime/src/trampoline/func.rs index bb77dbe53153..fce77bd2b928 100644 --- a/crates/wasmtime/src/trampoline/func.rs +++ b/crates/wasmtime/src/trampoline/func.rs @@ -127,7 +127,7 @@ where let mut code_memory = CodeMemory::new(obj); let code = code_memory.publish()?; - register_trampolines(engine.config().profiler.as_ref(), &code.obj); + register_trampolines(engine.profiler(), &code.obj); // Extract the host/wasm trampolines from the results of compilation since // we know their start/length. diff --git a/fuzz/fuzz_targets/compile.rs b/fuzz/fuzz_targets/compile.rs index 9af4b5ae0f40..93ff23c1b0c0 100644 --- a/fuzz/fuzz_targets/compile.rs +++ b/fuzz/fuzz_targets/compile.rs @@ -12,7 +12,7 @@ fn create_engine() -> Engine { // the generated code at all; it only does extra checking after // compilation. unsafe { - config.cranelift_flag_enable("regalloc_checker").unwrap(); + config.cranelift_flag_enable("regalloc_checker"); } Engine::new(&config).expect("Could not construct Engine") } diff --git a/tests/all/relocs.rs b/tests/all/relocs.rs index fdd0730dfa52..c799fe1cbe37 100644 --- a/tests/all/relocs.rs +++ b/tests/all/relocs.rs @@ -21,7 +21,7 @@ fn store_with_padding(padding: usize) -> Result> { config.cranelift_flag_set( "wasmtime_linkopt_padding_between_functions", &padding.to_string(), - )?; + ); } let engine = Engine::new(&config)?; Ok(Store::new(&engine, ())) @@ -78,7 +78,7 @@ fn mixed() -> Result<()> { fn mixed_forced() -> Result<()> { let mut config = Config::new(); unsafe { - config.cranelift_flag_set("wasmtime_linkopt_force_jump_veneer", "true")?; + config.cranelift_flag_set("wasmtime_linkopt_force_jump_veneer", "true"); } let engine = Engine::new(&config)?; test_many_call_module(Store::new(&engine, ()))