From 8bc75502112d05e2f16e0714e59ae3cb4bd9c43c Mon Sep 17 00:00:00 2001 From: Peter Huene Date: Wed, 30 Nov 2022 07:57:53 -0800 Subject: [PATCH] wasmtime: enable stack probing for x86_64 targets. (#5350) * wasmtime: enable stack probing for x86_64 targets. This commit unconditionally enables stack probing for x86_64 targets. On Windows, stack probing is always required because of the way Windows commits stack pages (via guard page access). Fixes #5340. * Remove SIMD types from test case. --- crates/wasmtime/src/config.rs | 24 ++++++++++++++++++++++++ crates/wasmtime/src/engine.rs | 12 +++++++++--- tests/all/stack_overflow.rs | 35 ++++++++++++++++++++++++++++++++++- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/crates/wasmtime/src/config.rs b/crates/wasmtime/src/config.rs index cc345bc7f1..ed9c4f3dd0 100644 --- a/crates/wasmtime/src/config.rs +++ b/crates/wasmtime/src/config.rs @@ -191,6 +191,7 @@ impl Config { ret.cranelift_debug_verifier(false); ret.cranelift_opt_level(OptLevel::Speed); } + ret.wasm_reference_types(true); ret.wasm_multi_value(true); ret.wasm_bulk_memory(true); @@ -1469,13 +1470,36 @@ impl Config { #[cfg(compiler)] pub(crate) fn build_compiler(&mut self) -> Result> { + use target_lexicon::Architecture; + 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())?; } + // On x86-64 targets, we enable stack probing by default. + // This is required on Windows because of the way Windows + // commits its stacks, but it's also a good idea on other + // platforms to ensure guard pages are hit for large frame + // sizes. + if self + .compiler_config + .target + .as_ref() + .map(|t| t.architecture == Architecture::X86_64) + .unwrap_or(cfg!(target_arch = "x86_64")) + { + self.compiler_config + .flags + .insert("enable_probestack".into()); + self.compiler_config + .settings + .insert("probestack_strategy".into(), "inline".into()); + } + if self.native_unwind_info || // Windows always needs unwind info, since it is part of the ABI. self diff --git a/crates/wasmtime/src/engine.rs b/crates/wasmtime/src/engine.rs index 51cbeedae7..24961693bc 100644 --- a/crates/wasmtime/src/engine.rs +++ b/crates/wasmtime/src/engine.rs @@ -359,6 +359,9 @@ impl Engine { flag: &str, value: &FlagValue, ) -> Result<(), String> { + use target_lexicon::Architecture; + + let target = self.target(); let ok = match flag { // These settings must all have be enabled, since their value // can affect the way the generated code performs or behaves at @@ -367,13 +370,16 @@ impl Engine { "libcall_call_conv" => *value == FlagValue::Enum("isa_default".into()), "preserve_frame_pointers" => *value == FlagValue::Bool(true), + // On x86-64 targets, stack probing is enabled by default. + "enable_probestack" => *value == FlagValue::Bool(target.architecture == Architecture::X86_64), + // On x86-64 targets, the default stack probing strategy is inline. + "probestack_strategy" => *value == FlagValue::Enum(if target.architecture == Architecture::X86_64 { "inline" } else { "outline" }.into()), + // Features wasmtime doesn't use should all be disabled, since // otherwise if they are enabled it could change the behavior of // generated code. "enable_llvm_abi_extensions" => *value == FlagValue::Bool(false), "enable_pinned_reg" => *value == FlagValue::Bool(false), - "enable_probestack" => *value == FlagValue::Bool(false), - "probestack_strategy" => *value == FlagValue::Enum("outline".into()), "use_colocated_libcalls" => *value == FlagValue::Bool(false), "use_pinned_reg_as_heap_base" => *value == FlagValue::Bool(false), @@ -389,7 +395,7 @@ impl Engine { // Windows requires unwind info as part of its ABI. "unwind_info" => { - if self.target().operating_system == target_lexicon::OperatingSystem::Windows { + if target.operating_system == target_lexicon::OperatingSystem::Windows { *value == FlagValue::Bool(true) } else { return Ok(()) diff --git a/tests/all/stack_overflow.rs b/tests/all/stack_overflow.rs index 67ab7e22a7..f2e2417d46 100644 --- a/tests/all/stack_overflow.rs +++ b/tests/all/stack_overflow.rs @@ -1,8 +1,9 @@ +use anyhow::Result; use std::sync::atomic::{AtomicUsize, Ordering::SeqCst}; use wasmtime::*; #[test] -fn host_always_has_some_stack() -> anyhow::Result<()> { +fn host_always_has_some_stack() -> Result<()> { static HITS: AtomicUsize = AtomicUsize::new(0); // assume hosts always have at least 128k of stack const HOST_STACK: usize = 128 * 1024; @@ -54,3 +55,35 @@ fn host_always_has_some_stack() -> anyhow::Result<()> { consume_some_stack(space.as_mut_ptr() as usize, stack.saturating_sub(1024)) } } + +#[test] +fn big_stack_works_ok() -> Result<()> { + const N: usize = 10000; + + // Build a module with a function that uses a very large amount of stack space, + // modeled here by calling an i64-returning-function many times followed by + // adding them all into one i64. + // + // This should exercise the ability to consume multi-page stacks and + // only touch a few internals of it at a time. + let mut s = String::new(); + s.push_str("(module\n"); + s.push_str("(func (export \"\") (result i64)\n"); + s.push_str("i64.const 0\n"); + for _ in 0..N { + s.push_str("call $get\n"); + } + for _ in 0..N { + s.push_str("i64.add\n"); + } + s.push_str(")\n"); + s.push_str("(func $get (result i64) i64.const 0)\n"); + s.push_str(")\n"); + + let mut store = Store::<()>::default(); + let module = Module::new(store.engine(), &s)?; + let instance = Instance::new(&mut store, &module, &[])?; + let func = instance.get_typed_func::<(), i64>(&mut store, "")?; + assert_eq!(func.call(&mut store, ())?, 0); + Ok(()) +}