Skip to content

Commit

Permalink
make backtrace collection a Config field rather than a cargo feature (b…
Browse files Browse the repository at this point in the history
…ytecodealliance#4183)

* sorta working in runtime

* wasmtime-runtime: get rid of wasm-backtrace feature

* wasmtime: factor to make backtraces recording optional. not configurable yet

* get rid of wasm-backtrace features

* trap tests: now a Trap optionally contains backtrace

* eliminate wasm-backtrace feature

* code review fixes

* ci: no more wasm-backtrace feature

* c_api: backtraces always enabled

* config: unwind required by backtraces and ref types

* plumbed

* test that disabling backtraces works

* code review comments

* fuzzing generator: wasm_backtrace is a runtime config now

* doc fix
  • Loading branch information
Pat Hickey authored May 25, 2022
1 parent 010e028 commit bffce37
Show file tree
Hide file tree
Showing 20 changed files with 291 additions and 215 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -136,9 +136,8 @@ jobs:
- run: cargo check -p wasmtime --no-default-features --features async
- run: cargo check -p wasmtime --no-default-features --features pooling-allocator
- run: cargo check -p wasmtime --no-default-features --features cranelift
- run: cargo check -p wasmtime --no-default-features --features wasm-backtrace
- run: cargo check -p wasmtime --no-default-features --features component-model
- run: cargo check -p wasmtime --no-default-features --features cranelift,wat,async,cache,wasm-backtrace
- run: cargo check -p wasmtime --no-default-features --features cranelift,wat,async,cache
- run: cargo check --features component-model

# Check that benchmarks of the cranelift project build
Expand Down
4 changes: 2 additions & 2 deletions Cargo.lock

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

2 changes: 0 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ default = [
"wasi-nn",
"pooling-allocator",
"memory-init-cow",
"wasm-backtrace",
]
jitdump = ["wasmtime/jitdump"]
vtune = ["wasmtime/vtune"]
Expand All @@ -108,7 +107,6 @@ memory-init-cow = ["wasmtime/memory-init-cow", "wasmtime-cli-flags/memory-init-c
pooling-allocator = ["wasmtime/pooling-allocator", "wasmtime-cli-flags/pooling-allocator"]
all-arch = ["wasmtime/all-arch"]
posix-signals-on-macos = ["wasmtime/posix-signals-on-macos"]
wasm-backtrace = ["wasmtime/wasm-backtrace", "wasmtime-cli-flags/wasm-backtrace"]
component-model = ["wasmtime/component-model", "wasmtime-wast/component-model", "wasmtime-cli-flags/component-model"]

# Stub feature that does nothing, for Cargo-features compatibility: the new
Expand Down
2 changes: 1 addition & 1 deletion crates/c-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ doctest = false
env_logger = "0.9"
anyhow = "1.0"
once_cell = "1.3"
wasmtime = { path = "../wasmtime", default-features = false, features = ['cranelift', 'wasm-backtrace'] }
wasmtime = { path = "../wasmtime", default-features = false, features = ['cranelift'] }
wasmtime-c-api-macros = { path = "macros" }

# Optional dependency for the `wat2wasm` API
Expand Down
24 changes: 17 additions & 7 deletions crates/c-api/src/trap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,13 @@ pub extern "C" fn wasm_trap_message(trap: &wasm_trap_t, out: &mut wasm_message_t

#[no_mangle]
pub extern "C" fn wasm_trap_origin(raw: &wasm_trap_t) -> Option<Box<wasm_frame_t>> {
if raw.trap.trace().len() > 0 {
if raw
.trap
.trace()
.expect("backtraces are always enabled")
.len()
> 0
{
Some(Box::new(wasm_frame_t {
trap: raw.trap.clone(),
idx: 0,
Expand All @@ -78,7 +84,11 @@ pub extern "C" fn wasm_trap_origin(raw: &wasm_trap_t) -> Option<Box<wasm_frame_t

#[no_mangle]
pub extern "C" fn wasm_trap_trace(raw: &wasm_trap_t, out: &mut wasm_frame_vec_t) {
let vec = (0..raw.trap.trace().len())
let vec = (0..raw
.trap
.trace()
.expect("backtraces are always enabled")
.len())
.map(|idx| {
Some(Box::new(wasm_frame_t {
trap: raw.trap.clone(),
Expand Down Expand Up @@ -128,15 +138,15 @@ pub extern "C" fn wasmtime_trap_exit_status(raw: &wasm_trap_t, status: &mut i32)

#[no_mangle]
pub extern "C" fn wasm_frame_func_index(frame: &wasm_frame_t) -> u32 {
frame.trap.trace()[frame.idx].func_index()
frame.trap.trace().expect("backtraces are always enabled")[frame.idx].func_index()
}

#[no_mangle]
pub extern "C" fn wasmtime_frame_func_name(frame: &wasm_frame_t) -> Option<&wasm_name_t> {
frame
.func_name
.get_or_init(|| {
frame.trap.trace()[frame.idx]
frame.trap.trace().expect("backtraces are always enabled")[frame.idx]
.func_name()
.map(|s| wasm_name_t::from(s.to_string().into_bytes()))
})
Expand All @@ -148,7 +158,7 @@ pub extern "C" fn wasmtime_frame_module_name(frame: &wasm_frame_t) -> Option<&wa
frame
.module_name
.get_or_init(|| {
frame.trap.trace()[frame.idx]
frame.trap.trace().expect("backtraces are always enabled")[frame.idx]
.module_name()
.map(|s| wasm_name_t::from(s.to_string().into_bytes()))
})
Expand All @@ -157,7 +167,7 @@ pub extern "C" fn wasmtime_frame_module_name(frame: &wasm_frame_t) -> Option<&wa

#[no_mangle]
pub extern "C" fn wasm_frame_func_offset(frame: &wasm_frame_t) -> usize {
frame.trap.trace()[frame.idx]
frame.trap.trace().expect("backtraces are always enabled")[frame.idx]
.func_offset()
.unwrap_or(usize::MAX)
}
Expand All @@ -169,7 +179,7 @@ pub extern "C" fn wasm_frame_instance(_arg1: *const wasm_frame_t) -> *mut wasm_i

#[no_mangle]
pub extern "C" fn wasm_frame_module_offset(frame: &wasm_frame_t) -> usize {
frame.trap.trace()[frame.idx]
frame.trap.trace().expect("backtraces are always enabled")[frame.idx]
.module_offset()
.unwrap_or(usize::MAX)
}
Expand Down
1 change: 0 additions & 1 deletion crates/cli-flags/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,4 @@ default = [
]
pooling-allocator = []
memory-init-cow = []
wasm-backtrace = []
component-model = []
2 changes: 0 additions & 2 deletions crates/cli-flags/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,9 +346,7 @@ impl CommonOptions {
config.wasm_bulk_memory(enable);
}
if let Some(enable) = reference_types {
#[cfg(feature = "wasm-backtrace")]
config.wasm_reference_types(enable);
drop(enable); // suppress unused warnings
}
if let Some(enable) = multi_value {
config.wasm_multi_value(enable);
Expand Down
2 changes: 2 additions & 0 deletions crates/fuzzing/src/generators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ pub struct WasmtimeConfig {
codegen: CodegenSettings,
padding_between_functions: Option<u16>,
generate_address_map: bool,
wasm_backtraces: bool,
}

/// Configuration for linear memories in Wasmtime.
Expand Down Expand Up @@ -387,6 +388,7 @@ impl Config {
.wasm_multi_memory(self.module_config.config.max_memories > 1)
.wasm_simd(self.module_config.config.simd_enabled)
.wasm_memory64(self.module_config.config.memory64_enabled)
.wasm_backtrace(self.wasmtime.wasm_backtraces)
.cranelift_nan_canonicalization(self.wasmtime.canonicalize_nans)
.cranelift_opt_level(self.wasmtime.opt_level.to_wasmtime())
.consume_fuel(self.wasmtime.consume_fuel)
Expand Down
3 changes: 1 addition & 2 deletions crates/runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ indexmap = "1.0.2"
thiserror = "1.0.4"
more-asserts = "0.2.1"
cfg-if = "1.0"
backtrace = { version = "0.3.61", optional = true }
backtrace = { version = "0.3.61" }
rand = "0.8.3"
anyhow = "1.0.38"
memfd = { version = "0.4.1", optional = true }
Expand All @@ -44,7 +44,6 @@ maintenance = { status = "actively-developed" }

[features]
memory-init-cow = ['memfd']
wasm-backtrace = ["backtrace"]

async = ["wasmtime-fiber"]

Expand Down
4 changes: 0 additions & 4 deletions crates/runtime/src/externref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,6 @@ impl VMExternRefActivationsTable {
}
}

#[cfg_attr(not(feature = "wasm-backtrace"), allow(dead_code))]
fn insert_precise_stack_root(
precise_stack_roots: &mut HashSet<VMExternRefWithTraits>,
root: NonNull<VMExternData>,
Expand Down Expand Up @@ -867,7 +866,6 @@ impl<T> std::ops::DerefMut for DebugOnly<T> {
///
/// Additionally, you must have registered the stack maps for every Wasm module
/// that has frames on the stack with the given `stack_maps_registry`.
#[cfg_attr(not(feature = "wasm-backtrace"), allow(unused_mut, unused_variables))]
pub unsafe fn gc(
module_info_lookup: &dyn ModuleInfoLookup,
externref_activations_table: &mut VMExternRefActivationsTable,
Expand Down Expand Up @@ -895,7 +893,6 @@ pub unsafe fn gc(
None => {
if cfg!(debug_assertions) {
// Assert that there aren't any Wasm frames on the stack.
#[cfg(feature = "wasm-backtrace")]
backtrace::trace(|frame| {
assert!(module_info_lookup.lookup(frame.ip() as usize).is_none());
true
Expand Down Expand Up @@ -937,7 +934,6 @@ pub unsafe fn gc(
});
}

#[cfg(feature = "wasm-backtrace")]
backtrace::trace(|frame| {
let pc = frame.ip() as usize;
let sp = frame.sp() as usize;
Expand Down
6 changes: 3 additions & 3 deletions crates/runtime/src/libcalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ pub unsafe extern "C" fn memory_atomic_notify(
// just to be sure.
let addr_to_check = addr.checked_add(4).unwrap();
validate_atomic_addr(instance, memory, addr_to_check).and_then(|()| {
Err(Trap::User(anyhow::anyhow!(
Err(Trap::user(anyhow::anyhow!(
"unimplemented: wasm atomics (fn memory_atomic_notify) unsupported",
)))
})
Expand All @@ -534,7 +534,7 @@ pub unsafe extern "C" fn memory_atomic_wait32(
// but we still double-check
let addr_to_check = addr.checked_add(4).unwrap();
validate_atomic_addr(instance, memory, addr_to_check).and_then(|()| {
Err(Trap::User(anyhow::anyhow!(
Err(Trap::user(anyhow::anyhow!(
"unimplemented: wasm atomics (fn memory_atomic_wait32) unsupported",
)))
})
Expand All @@ -561,7 +561,7 @@ pub unsafe extern "C" fn memory_atomic_wait64(
// but we still double-check
let addr_to_check = addr.checked_add(8).unwrap();
validate_atomic_addr(instance, memory, addr_to_check).and_then(|()| {
Err(Trap::User(anyhow::anyhow!(
Err(Trap::user(anyhow::anyhow!(
"unimplemented: wasm atomics (fn memory_atomic_wait64) unsupported",
)))
})
Expand Down
Loading

0 comments on commit bffce37

Please sign in to comment.