From bee530b7b140b509ed0f6eee6dd7611a8460f83e Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Fri, 4 Mar 2022 14:46:14 +0000 Subject: [PATCH] Don't try generating a backtrace on stack overflow --- lib/vm/src/trap/traphandlers.rs | 18 +++++++++++++++--- tests/compilers/traps.rs | 10 +++------- tests/ignores.txt | 6 ------ 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/vm/src/trap/traphandlers.rs b/lib/vm/src/trap/traphandlers.rs index 58736a94de7..7fd6db45182 100644 --- a/lib/vm/src/trap/traphandlers.rs +++ b/lib/vm/src/trap/traphandlers.rs @@ -715,9 +715,6 @@ impl TrapHandlerContextInner { return false; } - // Set up the register state for exception return to force the - // coroutine to return to its caller with UnwindReason::WasmTrap. - let backtrace = Backtrace::new_unresolved(); let signal_trap = maybe_fault_address.map(|addr| { if self.coro_trap_handler.stack_ptr_in_bounds(addr) { TrapCode::StackOverflow @@ -725,6 +722,21 @@ impl TrapHandlerContextInner { TrapCode::HeapAccessOutOfBounds } }); + + // Don't try to generate a backtrace for stack overflows: unwinding + // information is often not precise enough to properly describe what is + // happenning during a function prologue, which can lead the unwinder to + // read invalid memory addresses. + // + // See: https://github.com/rust-lang/backtrace-rs/pull/357 + let backtrace = if signal_trap == Some(TrapCode::StackOverflow) { + Backtrace::from(vec![]) + } else { + Backtrace::new_unresolved() + }; + + // Set up the register state for exception return to force the + // coroutine to return to its caller with UnwindReason::WasmTrap. let unwind = UnwindReason::WasmTrap { backtrace, signal_trap, diff --git a/tests/compilers/traps.rs b/tests/compilers/traps.rs index 493090c7e9a..77b5d6e8560 100644 --- a/tests/compilers/traps.rs +++ b/tests/compilers/traps.rs @@ -135,13 +135,9 @@ fn test_trap_stack_overflow(config: crate::Config) -> Result<()> { let e = run_func.call(&[]).err().expect("error calling function"); - let trace = e.trace(); - assert!(trace.len() >= 32); - for i in 0..trace.len() { - assert_eq!(trace[i].module_name(), "rec_mod"); - assert_eq!(trace[i].func_index(), 0); - assert_eq!(trace[i].function_name(), Some("run")); - } + // We specifically don't check the stack trace here: stack traces after + // stack overflows are not generally possible due to unreliable unwinding + // information. assert!(e.message().contains("call stack exhausted")); Ok(()) diff --git a/tests/ignores.txt b/tests/ignores.txt index ba4976b9a03..a7b67a9a6d3 100644 --- a/tests/ignores.txt +++ b/tests/ignores.txt @@ -36,16 +36,10 @@ aarch64 traps::start_trap_pretty singlepass multi_value_imports::dylib # Singlepass doesn't support multivalue singlepass multi_value_imports::dynamic # Singlepass doesn't support multivalue -# TODO: We need to fix this in ARM. The issue is caused by libunwind overflowing -# the stack while creating the stacktrace. -# https://github.com/rust-lang/backtrace-rs/issues/356 # Also neither LLVM nor Cranelift currently implement stack probing on AArch64. # https://github.com/wasmerio/wasmer/issues/2808 cranelift+aarch64 spec::skip_stack_guard_page llvm+aarch64 spec::skip_stack_guard_page -# TODO: Needs more investigation -cranelift+macos spec::skip_stack_guard_page -llvm+macos spec::skip_stack_guard_page # Some SIMD opperations are not yet supported by Cranelift # Cranelift just added support for most of those recently, it might be easy to update