From 4f609f7c829aa35825c0ba613776b8ad82508570 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 28 Jun 2022 20:03:16 -0700 Subject: [PATCH 1/3] Wasmtime: disable unwind_info unless needed fixes #4350 Otherwise wasm modules will be built with unwind info, even if backtraces are disabled. This can get expensive in deeply recursive modules. --- crates/wasmtime/src/config.rs | 4 ++++ crates/wasmtime/src/engine.rs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs index b41ae9396f43..6cd656402af8 100644 --- a/crates/wasmtime/src/config.rs +++ b/crates/wasmtime/src/config.rs @@ -1423,6 +1423,10 @@ impl Config { { 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 diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs index 9dd637e7143f..b86e636da535 100644 --- a/crates/wasmtime/src/engine.rs +++ b/crates/wasmtime/src/engine.rs @@ -345,7 +345,6 @@ impl Engine { // 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 @@ -381,6 +380,7 @@ impl Engine { | "enable_verifier" | "regalloc_checker" | "is_pic" + | "unwind_info" | "machine_code_cfg_info" | "tls_model" // wasmtime doesn't use tls right now | "opt_level" // opt level doesn't change semantics From 61312a9bf4b6a5d0922f77901f9b3f025de324ef Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Wed, 29 Jun 2022 08:54:27 -0700 Subject: [PATCH 2/3] Wasmtime: test that disabling backtraces disables unwind_info --- crates/wasmtime/src/engine.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs index b86e636da535..c4bfb75e33d6 100644 --- a/crates/wasmtime/src/engine.rs +++ b/crates/wasmtime/src/engine.rs @@ -519,8 +519,10 @@ impl Default for Engine { #[cfg(test)] mod tests { use crate::{Config, Engine, Module, OptLevel}; + use anyhow::Result; use tempfile::TempDir; + use wasmtime_environ::FlagValue; #[test] fn cache_accounts_for_opt_level() -> Result<()> { @@ -585,4 +587,20 @@ 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" + ); + } } From 0d342e649660f36b6ae36e393a0f817251c3801d Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Wed, 29 Jun 2022 16:32:53 -0700 Subject: [PATCH 3/3] fix: make sure we have unwind_info when the engine needs it --- crates/wasmtime/src/engine.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs index c4bfb75e33d6..c644d742dc2b 100644 --- a/crates/wasmtime/src/engine.rs +++ b/crates/wasmtime/src/engine.rs @@ -368,6 +368,16 @@ impl Engine { } } + // If reference types or backtraces are enabled, we need unwind info. Otherwise, we + // don't care. + "unwind_info" => { + if self.config().wasm_backtrace || 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. @@ -380,7 +390,6 @@ impl Engine { | "enable_verifier" | "regalloc_checker" | "is_pic" - | "unwind_info" | "machine_code_cfg_info" | "tls_model" // wasmtime doesn't use tls right now | "opt_level" // opt level doesn't change semantics