Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wasmtime: Add a Config::native_unwind_info method #4643

Merged
merged 1 commit into from
Aug 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/fuzzing/src/generators/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -389,7 +389,7 @@ pub struct WasmtimeConfig {
codegen: CodegenSettings,
padding_between_functions: Option<u16>,
generate_address_map: bool,
wasm_backtraces: bool,
native_unwind_info: bool,
}

#[derive(Arbitrary, Clone, Debug, PartialEq, Eq, Hash)]
Expand Down
52 changes: 40 additions & 12 deletions crates/wasmtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
///
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
22 changes: 2 additions & 20 deletions crates/wasmtime/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down Expand Up @@ -544,7 +543,6 @@ mod tests {

use anyhow::Result;
use tempfile::TempDir;
use wasmtime_environ::FlagValue;

#[test]
fn cache_accounts_for_opt_level() -> Result<()> {
Expand Down Expand Up @@ -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"
);
}
}
1 change: 1 addition & 0 deletions tests/all/traps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down