From ec47335b9c1706f40571941cd6dea3ca65236ed9 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 8 Aug 2022 13:54:51 -0700 Subject: [PATCH] wasmtime: Add a `Config::native_unwind_info` method (#4643) This method configures whether native unwind information (e.g. `.eh_frame` on Linux) is generated or not. This helps integrate with third-party stack capturing tools, such as the system unwinder or the `backtrace` crate. It does not affect whether Wasmtime can capture stack traces in Wasm code that it is running or not. Unwind info is always enabled on Windows, since the Windows ABI requires it. This configuration option defaults to true. Additionally, we deprecate `Config::wasm_backtrace` since we can always cheaply capture stack traces ever since https://github.com/bytecodealliance/wasmtime/pull/4431. Fixes https://github.com/bytecodealliance/wasmtime/issues/4554 --- crates/fuzzing/src/generators/config.rs | 4 +- crates/wasmtime/src/config.rs | 52 +++++++++++++++++++------ crates/wasmtime/src/engine.rs | 22 +---------- tests/all/traps.rs | 1 + 4 files changed, 45 insertions(+), 34 deletions(-) diff --git a/crates/fuzzing/src/generators/config.rs b/crates/fuzzing/src/generators/config.rs index 0358d7075727..a2bd2dc2ff67 100644 --- a/crates/fuzzing/src/generators/config.rs +++ b/crates/fuzzing/src/generators/config.rs @@ -172,7 +172,7 @@ impl Config { .wasm_simd(self.module_config.config.simd_enabled) .wasm_memory64(self.module_config.config.memory64_enabled) .wasm_threads(self.module_config.config.threads_enabled) - .wasm_backtrace(self.wasmtime.wasm_backtraces) + .native_unwind_info(self.wasmtime.native_unwind_info) .cranelift_nan_canonicalization(self.wasmtime.canonicalize_nans) .cranelift_opt_level(self.wasmtime.opt_level.to_wasmtime()) .consume_fuel(self.wasmtime.consume_fuel) @@ -389,7 +389,7 @@ pub struct WasmtimeConfig { codegen: CodegenSettings, padding_between_functions: Option, generate_address_map: bool, - wasm_backtraces: bool, + native_unwind_info: bool, } #[derive(Arbitrary, Clone, Debug, PartialEq, Eq, Hash)] diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs index 6bb890be3ff5..78c483d5f040 100644 --- a/crates/wasmtime/src/config.rs +++ b/crates/wasmtime/src/config.rs @@ -98,6 +98,7 @@ pub struct Config { pub(crate) features: WasmFeatures, pub(crate) wasm_backtrace: bool, pub(crate) wasm_backtrace_details_env_used: bool, + pub(crate) native_unwind_info: bool, #[cfg(feature = "async")] pub(crate) async_stack_size: usize, pub(crate) async_support: bool, @@ -180,6 +181,7 @@ impl Config { max_wasm_stack: 512 * 1024, wasm_backtrace: true, wasm_backtrace_details_env_used: false, + native_unwind_info: true, features: WasmFeatures::default(), #[cfg(feature = "async")] async_stack_size: 2 << 20, @@ -341,6 +343,8 @@ impl Config { /// /// When disabled, wasm backtrace details are ignored, and [`crate::Trap::trace()`] /// will always return `None`. + #[deprecated = "Backtraces will always be enabled in future Wasmtime releases; if this \ + causes problems for you, please file an issue."] pub fn wasm_backtrace(&mut self, enable: bool) -> &mut Self { self.wasm_backtrace = enable; self @@ -372,6 +376,24 @@ impl Config { self } + /// Configures whether to generate native unwind information + /// (e.g. `.eh_frame` on Linux). + /// + /// This configuration option only exists to help third-party stack + /// capturing mechanisms, such as the system's unwinder or the `backtrace` + /// crate, determine how to unwind through Wasm frames. It does not affect + /// whether Wasmtime can capture Wasm backtraces or not, or whether + /// [`Trap::trace`][crate::Trap::trace] returns `Some` or `None`. + /// + /// Note that native unwind information is always generated when targeting + /// Windows, since the Windows ABI requires it. + /// + /// This option defaults to `true`. + pub fn native_unwind_info(&mut self, enable: bool) -> &mut Self { + self.native_unwind_info = enable; + self + } + /// Configures whether execution of WebAssembly will "consume fuel" to /// either halt or yield execution as desired. /// @@ -1415,6 +1437,24 @@ impl Config { compiler.target(target.clone())?; } + if self.native_unwind_info || + // Windows always needs unwind info, since it is part of the ABI. + self + .compiler_config + .target + .as_ref() + .map_or(cfg!(target_os = "windows"), |target| { + target.operating_system == target_lexicon::OperatingSystem::Windows + }) + { + if !self + .compiler_config + .ensure_setting_unset_or_given("unwind_info", "true") + { + bail!("compiler option 'unwind_info' must be enabled profiling"); + } + } + // We require frame pointers for correct stack walking, which is safety // critical in the presence of reference types, and otherwise it is just // really bad developer experience to get wrong. @@ -1423,18 +1463,6 @@ impl Config { .insert("preserve_frame_pointers".into(), "true".into()); // 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"); - } - } else { - self.compiler_config - .settings - .insert("unwind_info".to_string(), "false".to_string()); - } if self.features.reference_types { if !self .compiler_config diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs index d7c43713e332..e1c985208d23 100644 --- a/crates/wasmtime/src/engine.rs +++ b/crates/wasmtime/src/engine.rs @@ -367,10 +367,9 @@ impl Engine { } } - // If reference types or backtraces are enabled, we need unwind info. Otherwise, we - // don't care. + // Windows requires unwind info as part of its ABI. "unwind_info" => { - if self.config().wasm_backtrace || self.config().features.reference_types { + if self.target().operating_system == target_lexicon::OperatingSystem::Windows { *value == FlagValue::Bool(true) } else { return Ok(()) @@ -544,7 +543,6 @@ mod tests { use anyhow::Result; use tempfile::TempDir; - use wasmtime_environ::FlagValue; #[test] fn cache_accounts_for_opt_level() -> Result<()> { @@ -606,20 +604,4 @@ mod tests { Ok(()) } - - #[test] - #[cfg(compiler)] - fn test_disable_backtraces() { - let engine = Engine::new( - Config::new() - .wasm_backtrace(false) - .wasm_reference_types(false), - ) - .expect("failed to construct engine"); - assert_eq!( - engine.compiler().flags().get("unwind_info"), - Some(&FlagValue::Bool(false)), - "unwind info should be disabled unless needed" - ); - } } diff --git a/tests/all/traps.rs b/tests/all/traps.rs index 8f3dead699a7..884c934e1d2d 100644 --- a/tests/all/traps.rs +++ b/tests/all/traps.rs @@ -71,6 +71,7 @@ fn test_trap_trace() -> Result<()> { } #[test] +#[allow(deprecated)] fn test_trap_backtrace_disabled() -> Result<()> { let mut config = Config::default(); config.wasm_backtrace(false);