Skip to content

Commit

Permalink
Preserve full native stack traces in errors (bytecodealliance#823)
Browse files Browse the repository at this point in the history
* Preserve full native stack traces in errors

This commit builds on bytecodealliance#759 by performing a few refactorings:

* The `backtrace` crate is updated to 0.3.42 which incorporates the
  Windows-specific stack-walking code, so that's no longer needed.
* A full `backtrace::Backtrace` type is held in a trap at all times.
* The trap structures in the `wasmtime-*` internal crates were
  refactored a bit to preserve more information and deal with raw
  values rather than converting between various types and strings.
* The `wasmtime::Trap` type has been updated with these various changes.

Eventually I think we'll want to likely render full stack traces (and/or
partial wasm ones) into error messages, but for now that's left as-is
and we can always improve it later. I suspect the most relevant thing we
need to do is to implement function name symbolication for wasm
functions first, and then afterwards we can incorporate native function
names!

* Fix some test suite assertions
  • Loading branch information
alexcrichton authored Jan 15, 2020
1 parent b8e4354 commit e7e08f1
Show file tree
Hide file tree
Showing 12 changed files with 89 additions and 239 deletions.
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ anyhow = "1.0.19"
region = "2.0.0"
libc = "0.2"
cfg-if = "0.1.9"
backtrace = "0.3.42"

[target.'cfg(target_os = "windows")'.dependencies]
winapi = "0.3.7"
Expand Down
7 changes: 1 addition & 6 deletions crates/api/src/callable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,7 @@ impl WrappedCallable for WasmtimeFn {
)
})
} {
let message = error.0;
let backtrace = error.1;

let trap = take_api_trap().unwrap_or_else(|| {
Trap::new_with_trace(format!("call error: {}", message), backtrace)
});
let trap = take_api_trap().unwrap_or_else(|| Trap::from_jit(error));
return Err(trap);
}

Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ fn instantiate_in_context(
let instance = compiled_module.instantiate().map_err(|e| -> Error {
if let Some(trap) = take_api_trap() {
trap.into()
} else if let InstantiationError::StartTrap(msg) = e {
Trap::new(msg).into()
} else if let InstantiationError::StartTrap(trap) = e {
Trap::from_jit(trap).into()
} else {
e.into()
}
Expand Down
49 changes: 23 additions & 26 deletions crates/api/src/trap.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::instance::Instance;
use backtrace::Backtrace;
use std::fmt;
use std::sync::Arc;
use wasmtime_runtime::{get_backtrace, Backtrace, BacktraceFrame};

/// A struct representing an aborted instruction execution, with a message
/// indicating the cause.
Expand All @@ -12,7 +12,8 @@ pub struct Trap {

struct TrapInner {
message: String,
trace: Vec<FrameInfo>,
wasm_trace: Vec<FrameInfo>,
native_trace: Backtrace,
}

fn _assert_trap_is_sync_and_send(t: &Trap) -> (&dyn Sync, &dyn Send) {
Expand All @@ -27,21 +28,29 @@ impl Trap {
/// assert_eq!("unexpected error", trap.message());
/// ```
pub fn new<I: Into<String>>(message: I) -> Self {
Self::new_with_trace(message, get_backtrace())
Trap::new_with_trace(message.into(), Backtrace::new_unresolved())
}

pub(crate) fn new_with_trace<I: Into<String>>(message: I, backtrace: Backtrace) -> Self {
let mut trace = Vec::with_capacity(backtrace.len());
for i in 0..backtrace.len() {
// Don't include frames without backtrace info.
if let Some(info) = FrameInfo::try_from(backtrace[i]) {
trace.push(info);
pub(crate) fn from_jit(jit: wasmtime_runtime::Trap) -> Self {
Trap::new_with_trace(jit.to_string(), jit.backtrace)
}

fn new_with_trace(message: String, native_trace: Backtrace) -> Self {
let mut wasm_trace = Vec::new();
for frame in native_trace.frames() {
let pc = frame.ip() as usize;
if let Some(info) = wasmtime_runtime::jit_function_registry::find(pc) {
wasm_trace.push(FrameInfo {
func_index: info.func_index as u32,
module_name: info.module_id.clone(),
})
}
}
Trap {
inner: Arc::new(TrapInner {
message: message.into(),
trace,
message,
wasm_trace,
native_trace,
}),
}
}
Expand All @@ -52,15 +61,16 @@ impl Trap {
}

pub fn trace(&self) -> &[FrameInfo] {
&self.inner.trace
&self.inner.wasm_trace
}
}

impl fmt::Debug for Trap {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("Trap")
.field("message", &self.inner.message)
.field("trace", &self.inner.trace)
.field("wasm_trace", &self.inner.wasm_trace)
.field("native_trace", &self.inner.native_trace)
.finish()
}
}
Expand Down Expand Up @@ -99,17 +109,4 @@ impl FrameInfo {
pub fn module_name(&self) -> Option<&str> {
self.module_name.as_deref()
}

pub(crate) fn try_from(backtrace: BacktraceFrame) -> Option<FrameInfo> {
if let Some(tag) = backtrace.tag() {
let func_index = tag.func_index as u32;
let module_name = tag.module_id.clone();
Some(FrameInfo {
func_index,
module_name,
})
} else {
None
}
}
}
16 changes: 4 additions & 12 deletions crates/jit/src/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ use std::cmp::max;
use std::{fmt, mem, ptr, slice};
use thiserror::Error;
use wasmtime_environ::ir;
use wasmtime_runtime::{
wasmtime_call_trampoline, Backtrace, Export, InstanceHandle, TrapMessageAndStack,
VMInvokeArgument,
};
use wasmtime_runtime::{wasmtime_call_trampoline, Export, InstanceHandle, Trap, VMInvokeArgument};

/// A runtime value.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -103,12 +100,7 @@ pub enum ActionOutcome {
},

/// A trap occurred while the action was executing.
Trapped {
/// The trap message.
message: String,
/// Backtrace.
trace: Backtrace,
},
Trapped(Trap),
}

/// An error detected while invoking a wasm function or reading a wasm global.
Expand Down Expand Up @@ -196,7 +188,7 @@ pub fn invoke(
compiler.publish_compiled_code();

// Call the trampoline.
if let Err(TrapMessageAndStack(message, trace)) = unsafe {
if let Err(trap) = unsafe {
instance.with_signals_on(|| {
wasmtime_call_trampoline(
callee_vmctx,
Expand All @@ -205,7 +197,7 @@ pub fn invoke(
)
})
} {
return Ok(ActionOutcome::Trapped { message, trace });
return Ok(ActionOutcome::Trapped(trap));
}

// Load the return values out of `values_vec`.
Expand Down
2 changes: 1 addition & 1 deletion crates/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ indexmap = "1.0.2"
thiserror = "1.0.4"
more-asserts = "0.2.1"
cfg-if = "0.1.9"
backtrace = "0.3.40"
backtrace = "0.3.42"

[target.'cfg(target_os = "windows")'.dependencies]
winapi = { version = "0.3.7", features = ["winbase", "memoryapi"] }
Expand Down
148 changes: 0 additions & 148 deletions crates/runtime/src/backtrace.rs

This file was deleted.

8 changes: 4 additions & 4 deletions crates/runtime/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::memory::LinearMemory;
use crate::mmap::Mmap;
use crate::signalhandlers::{wasmtime_init_eager, wasmtime_init_finish};
use crate::table::Table;
use crate::traphandlers::{wasmtime_call, TrapMessageAndStack};
use crate::traphandlers::{wasmtime_call, Trap};
use crate::vmcontext::{
VMBuiltinFunctionsArray, VMCallerCheckedAnyfunc, VMContext, VMFunctionBody, VMFunctionImport,
VMGlobalDefinition, VMGlobalImport, VMMemoryDefinition, VMMemoryImport, VMSharedSignatureIndex,
Expand Down Expand Up @@ -568,7 +568,7 @@ impl Instance {

// Make the call.
unsafe { wasmtime_call(callee_vmctx, callee_address) }
.map_err(|TrapMessageAndStack(msg, _)| InstantiationError::StartTrap(msg))
.map_err(InstantiationError::StartTrap)
}

/// Invoke the WebAssembly start function of the instance, if one is present.
Expand Down Expand Up @@ -1398,6 +1398,6 @@ pub enum InstantiationError {
Link(#[from] LinkError),

/// A compilation error occured.
#[error("Trap occurred while invoking start function: {0}")]
StartTrap(String),
#[error("Trap occurred while invoking start function")]
StartTrap(#[source] Trap),
}
Loading

0 comments on commit e7e08f1

Please sign in to comment.