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: disable unwind_info unless needed #4351

Merged
merged 3 commits into from
Jun 30, 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: 4 additions & 0 deletions crates/wasmtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 28 additions & 1 deletion crates/wasmtime/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -369,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.
Expand Down Expand Up @@ -519,8 +528,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<()> {
Expand Down Expand Up @@ -585,4 +596,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"
);
}
}