Skip to content

Commit 6c97d5e

Browse files
authored
Simplify some trap handling in wasmtime runtime (#9673)
The `needs_backtrace` field on `TrapReason` is an old artifact of having the `wasmtime` and `wasmtime-runtime` crates split and there's no longer any need for that. This commit removes the field and directly queries it from the error as necessary when capturing backtraces.
1 parent 6767488 commit 6c97d5e

File tree

7 files changed

+19
-51
lines changed

7 files changed

+19
-51
lines changed

crates/wasmtime/src/runtime/component/func/host.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ unsafe fn call_host_and_handle_result<T>(
311311

312312
match res {
313313
Ok(()) => {}
314-
Err(e) => crate::trap::raise(e),
314+
Err(e) => crate::runtime::vm::raise_user_trap(e),
315315
}
316316
}
317317

crates/wasmtime/src/runtime/func.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2371,7 +2371,7 @@ impl HostContext {
23712371

23722372
match result {
23732373
Ok(val) => val,
2374-
Err(err) => crate::trap::raise(err),
2374+
Err(err) => crate::runtime::vm::raise_user_trap(err),
23752375
}
23762376
}
23772377
}

crates/wasmtime/src/runtime/trampoline/func.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ unsafe extern "C" fn array_call_shim<F>(
6161
// call-site, which gets unwrapped in `Trap::from_runtime` later on as we
6262
// convert from the internal `Trap` type to our own `Trap` type in this
6363
// crate.
64-
Err(trap) => crate::trap::raise(trap.into()),
64+
Err(err) => crate::runtime::vm::raise_user_trap(err),
6565
}
6666
}
6767

crates/wasmtime/src/runtime/trap.rs

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,6 @@ use wasmtime_environ::{demangle_function_name, demangle_function_name_or_index,
6868
/// ```
6969
pub use wasmtime_environ::Trap;
7070

71-
// Same safety requirements and caveats as
72-
// `crate::runtime::vm::raise_user_trap`.
73-
pub(crate) unsafe fn raise(error: anyhow::Error) -> ! {
74-
let needs_backtrace = error.downcast_ref::<WasmBacktrace>().is_none();
75-
crate::runtime::vm::raise_user_trap(error, needs_backtrace)
76-
}
77-
7871
#[cold] // traps are exceptional, this helps move handling off the main path
7972
pub(crate) fn from_runtime_box(
8073
store: &mut StoreOpaque,
@@ -99,15 +92,7 @@ pub(crate) fn from_runtime_box(
9992
// provide useful information to debug with for the embedder/caller,
10093
// otherwise the information about what the wasm was doing when the
10194
// error was generated would be lost.
102-
crate::runtime::vm::TrapReason::User {
103-
error,
104-
needs_backtrace,
105-
} => {
106-
debug_assert!(
107-
needs_backtrace == backtrace.is_some() || !store.engine().config().wasm_backtrace
108-
);
109-
(error, None)
110-
}
95+
crate::runtime::vm::TrapReason::User(error) => (error, None),
11196
#[cfg(all(feature = "signals-based-traps", not(miri)))]
11297
crate::runtime::vm::TrapReason::Jit {
11398
pc,

crates/wasmtime/src/runtime/vm/component/libcalls.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,7 @@ mod trampolines {
132132
match result {
133133
Ok(ret) => shims!(@convert_ret ret $($pname: $param)*),
134134
Err(err) => crate::runtime::vm::traphandlers::raise_trap(
135-
crate::runtime::vm::traphandlers::TrapReason::User {
136-
error: err,
137-
needs_backtrace: true,
138-
},
135+
crate::runtime::vm::traphandlers::TrapReason::User(err)
139136
),
140137
}
141138
}

crates/wasmtime/src/runtime/vm/libcalls.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,7 @@ fn memory32_grow(
199199
memory_index: u32,
200200
) -> Result<*mut u8, TrapReason> {
201201
let memory_index = MemoryIndex::from_u32(memory_index);
202-
let result = match instance
203-
.memory_grow(store, memory_index, delta)
204-
.map_err(|error| TrapReason::User {
205-
error,
206-
needs_backtrace: true,
207-
})? {
202+
let result = match instance.memory_grow(store, memory_index, delta)? {
208203
Some(size_in_bytes) => size_in_bytes / instance.memory_page_size(memory_index),
209204
None => usize::max_value(),
210205
};

crates/wasmtime/src/runtime/vm/traphandlers.rs

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ pub use self::signals::*;
1818
use crate::prelude::*;
1919
use crate::runtime::vm::sys::traphandlers;
2020
use crate::runtime::vm::{Instance, VMContext, VMRuntimeLimits};
21+
use crate::WasmBacktrace;
2122
use core::cell::{Cell, UnsafeCell};
2223
use core::mem::MaybeUninit;
2324
use core::ops::Range;
@@ -58,11 +59,8 @@ pub unsafe fn raise_trap(reason: TrapReason) -> ! {
5859
/// Only safe to call when wasm code is on the stack, aka `catch_traps` must
5960
/// have been previously called. Additionally no Rust destructors can be on the
6061
/// stack. They will be skipped and not executed.
61-
pub unsafe fn raise_user_trap(error: Error, needs_backtrace: bool) -> ! {
62-
raise_trap(TrapReason::User {
63-
error,
64-
needs_backtrace,
65-
})
62+
pub unsafe fn raise_user_trap(error: Error) -> ! {
63+
raise_trap(TrapReason::User(error))
6664
}
6765

6866
/// Invokes the closure `f` and returns the result.
@@ -113,12 +111,7 @@ pub struct Trap {
113111
#[derive(Debug)]
114112
pub enum TrapReason {
115113
/// A user-raised trap through `raise_user_trap`.
116-
User {
117-
/// The actual user trap error.
118-
error: Error,
119-
/// Whether we need to capture a backtrace for this error or not.
120-
needs_backtrace: bool,
121-
},
114+
User(Error),
122115

123116
/// A trap raised from Cranelift-generated code.
124117
#[cfg(all(feature = "signals-based-traps", not(miri)))]
@@ -150,17 +143,14 @@ pub enum TrapReason {
150143

151144
impl TrapReason {
152145
/// Create a new `TrapReason::User` that does not have a backtrace yet.
153-
pub fn user_without_backtrace(error: Error) -> Self {
154-
TrapReason::User {
155-
error,
156-
needs_backtrace: true,
157-
}
146+
pub fn user(error: Error) -> Self {
147+
TrapReason::User(error)
158148
}
159149
}
160150

161151
impl From<Error> for TrapReason {
162152
fn from(err: Error) -> Self {
163-
TrapReason::user_without_backtrace(err)
153+
TrapReason::user(err)
164154
}
165155
}
166156

@@ -359,7 +349,7 @@ impl CallThreadState {
359349
}
360350

361351
fn unwind_with(&self, reason: UnwindReason) -> ! {
362-
let (backtrace, coredump) = match reason {
352+
let (backtrace, coredump) = match &reason {
363353
// Panics don't need backtraces. There is nowhere to attach the
364354
// hypothetical backtrace to and it doesn't really make sense to try
365355
// in the first place since this is a Rust problem rather than a
@@ -369,10 +359,11 @@ impl CallThreadState {
369359
// And if we are just propagating an existing trap that already has
370360
// a backtrace attached to it, then there is no need to capture a
371361
// new backtrace either.
372-
UnwindReason::Trap(TrapReason::User {
373-
needs_backtrace: false,
374-
..
375-
}) => (None, None),
362+
UnwindReason::Trap(TrapReason::User(err))
363+
if err.downcast_ref::<WasmBacktrace>().is_some() =>
364+
{
365+
(None, None)
366+
}
376367
UnwindReason::Trap(_) => (
377368
self.capture_backtrace(self.limits, None),
378369
self.capture_coredump(self.limits, None),

0 commit comments

Comments
 (0)