Skip to content

Commit

Permalink
[21.0.x] Fix backtraces through empty sequences of Wasm frames (#9412)
Browse files Browse the repository at this point in the history
* Fix backtraces through empty sequences of Wasm frames

This fixes a bug where we would not properly handle contiguous sequences
of Wasm frames that are empty. This was mistakenly believed to be an impossible
scenario, and before the tail-calls proposal it was impossible, however it can
now happen after the following series of events:

* Host calls into Wasm, pushing the entry trampoline frame.

* Entry trampoline calls the actual Wasm function, pushing a Wasm frame.

* Wasm function tail calls to an imported host function, *replacing*
  the Wasm frame with the exit trampoline's frame.

Now we have a stack like `[host, entry trampoline, exit trampoline]`, which has
zero Wasm frames between the entry and exit trampolines. If the host function
that the exit trampoline calls out to attempts to capture a backtrace, then --
before this commit -- we would fail an internal assertion and panic. That panic
would then unwind to the first Rust frame that is called by Wasm. With Rust 1.81
and later, Rust automatically inserts a panic handler that prevents the unwind
from continuing into external/foreign code, which is undefined behavior, and
aborts the process. Rust versions before 1.81 would attempt to continue
unwinding, hitting undefined behavior.

This commit fixes the backtrace capturing machinery to handle empty sequences of
Wasm frames, passes the assertion, and avoids unwinding into external/foreign
code.

Co-Authored-By: Alex Crichton <alex@alexcrichton.com>

* Add release notes

* Add more release notes

* Remove usage of wasmtime_test macro

* Enable more proposals

* ignore tail calls tests on s390x, since it didn't implement them in this release

---------

Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
  • Loading branch information
alexcrichton and fitzgen authored Oct 9, 2024
1 parent cc4917a commit c0e00c8
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 1 deletion.
16 changes: 16 additions & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,21 @@
--------------------------------------------------------------------------------

## 21.0.2

Released 2024-10-09.

### Fixed

* Fix a runtime crash when combining tail-calls with host imports that capture a
stack trace or trap.
[GHSA-q8hx-mm92-4wvg](https://github.com/bytecodealliance/wasmtime/security/advisories/GHSA-q8hx-mm92-4wvg)

* Fix a race condition could lead to WebAssembly control-flow integrity and type
safety violations.
[GHSA-7qmx-3fpx-r45m](https://github.com/bytecodealliance/wasmtime/security/advisories/GHSA-7qmx-3fpx-r45m)

--------------------------------------------------------------------------------

## 21.0.1

Released 2024-05-22.
Expand Down
20 changes: 20 additions & 0 deletions crates/wasmtime/src/runtime/vm/traphandlers/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,26 @@ impl Backtrace {

arch::assert_entry_sp_is_aligned(trampoline_sp);

// It is possible that the contiguous sequence of Wasm frames is
// empty. This is rare, but can happen if:
//
// * Host calls into Wasm, pushing the entry trampoline frame
//
// * Entry trampoline calls the actual Wasm function, pushing a Wasm frame
//
// * Wasm function tail calls to an imported host function, *replacing*
// the Wasm frame with the exit trampoline's frame
//
// Now we have a stack like `[host, entry trampoline, exit trampoline]`
// which has a contiguous sequence of Wasm frames that are empty.
//
// Therefore, check if we've reached the entry trampoline's SP as the
// first thing we do.
if arch::reached_entry_sp(fp, trampoline_sp) {
log::trace!("=== Empty contiguous sequence of Wasm frames ===");
return ControlFlow::Continue(());
}

loop {
// At the start of each iteration of the loop, we know that `fp` is
// a frame pointer from Wasm code. Therefore, we know it is not
Expand Down
1 change: 0 additions & 1 deletion tests/all/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,6 @@ fn concurrent_type_modifications_and_checks() -> Result<()> {

let mut config = Config::new();
config.wasm_function_references(true);

let engine = Engine::new(&config)?;

let mut threads = Vec::new();
Expand Down
124 changes: 124 additions & 0 deletions tests/all/traps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1679,3 +1679,127 @@ fn async_stack_size_ignored_if_disabled() -> Result<()> {

Ok(())
}

#[test]
#[cfg(not(target_arch = "s390x"))]
fn tail_call_to_imported_function() -> Result<()> {
let mut config = Config::new();
config.wasm_tail_call(true);
let engine = Engine::new(&config)?;

let module = Module::new(
&engine,
r#"
(module
(import "" "" (func (result i32)))
(func (export "run") (result i32)
return_call 0
)
)
"#,
)?;

let mut store = Store::new(&engine, ());
let host_func = Func::wrap(&mut store, || -> Result<i32> { bail!("whoopsie") });
let instance = Instance::new(&mut store, &module, &[host_func.into()])?;

let run = instance.get_typed_func::<(), i32>(&mut store, "run")?;
let err = run.call(&mut store, ()).unwrap_err();
assert!(err.to_string().contains("whoopsie"));

Ok(())
}

#[test]
#[cfg(not(target_arch = "s390x"))]
fn tail_call_to_imported_function_in_start_function() -> Result<()> {
let mut config = Config::new();
config.wasm_tail_call(true);
let engine = Engine::new(&config)?;

let module = Module::new(
&engine,
r#"
(module
(import "" "" (func))
(func $f
return_call 0
)
(start $f)
)
"#,
)?;

let mut store = Store::new(&engine, ());
let host_func = Func::wrap(&mut store, || -> Result<()> { bail!("whoopsie") });
let err = Instance::new(&mut store, &module, &[host_func.into()]).unwrap_err();
assert!(err.to_string().contains("whoopsie"));

Ok(())
}

#[test]
#[cfg(not(target_arch = "s390x"))]
fn return_call_ref_to_imported_function() -> Result<()> {
let mut config = Config::new();
config.wasm_tail_call(true);
config.wasm_function_references(true);
let engine = Engine::new(&config)?;

let module = Module::new(
&engine,
r#"
(module
(type (func (result i32)))
(func (export "run") (param (ref 0)) (result i32)
(return_call_ref 0 (local.get 0))
)
)
"#,
)?;

let mut store = Store::new(&engine, ());
let host_func = Func::wrap(&mut store, || -> Result<i32> { bail!("whoopsie") });
let instance = Instance::new(&mut store, &module, &[])?;

let run = instance.get_typed_func::<Func, i32>(&mut store, "run")?;
let err = run.call(&mut store, host_func).unwrap_err();
assert!(err.to_string().contains("whoopsie"));

Ok(())
}

#[test]
#[cfg(not(target_arch = "s390x"))]
fn return_call_indirect_to_imported_function() -> Result<()> {
let mut config = Config::new();
config.wasm_tail_call(true);
config.wasm_function_references(true);
let engine = Engine::new(&config)?;

let module = Module::new(
&engine,
r#"
(module
(import "" "" (func (result i32)))
(table 1 funcref (ref.func 0))
(func (export "run") (result i32)
(return_call_indirect (result i32) (i32.const 0))
)
)
"#,
)?;

let mut store = Store::new(&engine, ());
let host_func = Func::wrap(&mut store, || -> Result<i32> { bail!("whoopsie") });
let instance = Instance::new(&mut store, &module, &[host_func.into()])?;

let run = instance.get_typed_func::<(), i32>(&mut store, "run")?;
let err = run.call(&mut store, ()).unwrap_err();
assert!(err.to_string().contains("whoopsie"));

Ok(())
}

0 comments on commit c0e00c8

Please sign in to comment.