Skip to content

Commit

Permalink
code review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
Pat Hickey committed May 24, 2022
1 parent 8096d4c commit 9e29128
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 34 deletions.
18 changes: 5 additions & 13 deletions crates/runtime/src/traphandlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,18 +175,10 @@ impl Trap {

fn insert_backtrace(&mut self, bt: Backtrace) {
match self {
Trap::User {
ref mut backtrace, ..
} => *backtrace = Some(bt),
Trap::Jit {
ref mut backtrace, ..
} => *backtrace = Some(bt),
Trap::Wasm {
ref mut backtrace, ..
} => *backtrace = Some(bt),
Trap::OOM {
ref mut backtrace, ..
} => *backtrace = Some(bt),
Trap::User { backtrace, .. } => *backtrace = Some(bt),
Trap::Jit { backtrace, .. } => *backtrace = Some(bt),
Trap::Wasm { backtrace, .. } => *backtrace = Some(bt),
Trap::OOM { backtrace, .. } => *backtrace = Some(bt),
}
}
}
Expand Down Expand Up @@ -273,7 +265,7 @@ impl CallThreadState {
trap.insert_backtrace(backtrace);
}
trap
} // XXX fix this
}
(UnwindReason::JitTrap { pc }, backtrace) => Trap::Jit { pc, backtrace },
(UnwindReason::Panic(panic), _) => std::panic::resume_unwind(panic),
})
Expand Down
4 changes: 0 additions & 4 deletions crates/wasmtime/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,6 @@ impl Config {
ret.cranelift_opt_level(OptLevel::Speed);
}
ret.wasm_reference_types(true);
ret.features.reference_types = true;
ret.wasm_multi_value(true);
ret.wasm_bulk_memory(true);
ret.wasm_simd(true);
Expand Down Expand Up @@ -530,9 +529,6 @@ impl Config {
/// [proposal]: https://github.com/webassembly/reference-types
pub fn wasm_reference_types(&mut self, enable: bool) -> &mut Self {
self.features.reference_types = enable;
if enable {
self.wasm_backtrace(true);
}

#[cfg(compiler)]
{
Expand Down
28 changes: 11 additions & 17 deletions crates/wasmtime/src/trap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,17 +257,12 @@ impl Trap {
}

/// Creates a new `Trap`.
/// TODO fix these docs
/// * `trap_pc` - this is the precise program counter, if available, that
/// wasm trapped at. This is used when learning about the wasm stack trace
/// to ensure we assign the correct source to every frame.
///
/// * `reason` - this is the wasmtime-internal reason for why this trap is
/// being created.
///
/// * `native_trace` - this is a captured backtrace from when the trap
/// occurred, and this will iterate over the frames to find frames that
/// lie in wasm jit code.
/// * `backtrace` - this is a captured backtrace from when the trap
/// occurred. Contains the native backtrace, and the backtrace of
/// WebAssembly frames.
fn new_with_trace(reason: TrapReason, backtrace: Option<TrapBacktrace>) -> Self {
let backtrace = if let Some(bt) = backtrace {
OnceCell::with_value(bt)
Expand Down Expand Up @@ -318,15 +313,14 @@ impl Trap {
}

fn record_backtrace(&self, backtrace: TrapBacktrace) {
// Check if a backtrace has already been recorded. When a trap is
// created on top of the wasm stack, the trampoline will re-raise it
// via `wasmtime_runtime::raise_user_trap(trap.into::<Box<dyn
// Error>>())` after panic::catch_unwind. We don't want to overwrite the
// first backtrace recorded, as it is most precise.
if self.inner.backtrace.get().is_none() {
// Don't inspect result: we checked to make sure the once cell is empty.
let _ = self.inner.backtrace.set(backtrace);
}
// When a trap is created on top of the wasm stack, the trampoline will
// re-raise it via
// `wasmtime_runtime::raise_user_trap(trap.into::<Box<dyn Error>>())`
// after panic::catch_unwind. We don't want to overwrite the first
// backtrace recorded, as it is most precise.
// FIXME: make sure backtraces are only created once per trap! they are
// actually kinda expensive to create.
let _ = self.inner.backtrace.try_insert(backtrace);
}
}

Expand Down

0 comments on commit 9e29128

Please sign in to comment.