Skip to content

Commit 92f1829

Browse files
authored
miri: add guest-debugging, including frame accesses. (#12575)
* miri: add guest-debugging, including frame accesses. The fix to `vm_store_context` provenance in `record_unwind`/`unwind` is a little weird to me. I was seeing mut access to the `vm_store_context` via `store.vm_store_context_mut()` in `record_unwind` (before this diff) then access via the previously saved raw pointer in the `CallThreadState`, which was registered as invalid and I believe is indeed invalid. This was only manifesting when setting `Config::guest_debug`, even without the frame-handle accesses added here. I didn't dig into the exact diff in codegen or runtime behavior that caused this but in any case, accessing `vm_store_context` via these two different paths (with one mut) appears to be unsound in any case. The fix here is to set the unwind state via the raw pointer in `CallThreadState` since that's the only path that the subsequent `unwind` has access to. Unrelated but useful: `ci/miri-provenance.test.sh` now accepts `MIRI_RUST_VERSION=+nightly` or whatnot, which is nice for running locally (I keep `stable` as my default toolchain). * Revert MIRI_RUST_VERSION in the CI script and add note about `rustup run` instead. * Add a bit more usage of debug API to Pulley provenance test. * Switch to using `Store`-derived `VMStoreContext` where available and re-deriving the raw pointer in `CallThreadState`. prtest:full
1 parent d9038ed commit 92f1829

7 files changed

Lines changed: 52 additions & 22 deletions

File tree

ci/miri-provenance-test.sh

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,20 @@
55
# host to compile the wasm module in question to avoid needing to run Cranelift
66
# under MIRI. That enables much faster iteration on the test here.
77

8+
# Note that this requires a nightly toolchain for the use of miri. If your
9+
# default toolchain is not a nightly toolchain, you can run this script via
10+
# `rustup run nightly ./ci/miri-provenance-test.sh`.
11+
812
set -ex
913

1014
compile() {
11-
cargo run --no-default-features --features compile,pulley,wat,gc-drc,component-model,component-model-async \
15+
cargo run --no-default-features --features compile,pulley,wat,gc-drc,component-model,component-model-async,debug \
1216
compile --target pulley64 $1 \
1317
-o ${1%.wat}.cwasm \
1418
-O memory-reservation=$((1 << 20)) \
1519
-O memory-guard-size=0 \
1620
-O signals-based-traps=n \
21+
-D guest-debug=y \
1722
-W function-references,component-model-async,component-model-async-stackful,component-model-async-builtins,component-model-error-context
1823
}
1924

crates/wasmtime/src/runtime/debug.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -432,8 +432,9 @@ impl FrameHandle {
432432
// crates/cranelift/src/func_environ.rs in
433433
// `update_stack_slot_vmctx()`.) The frame/activation is
434434
// still valid because we verified this in `frame_data` above.
435-
let vmctx: *mut VMContext =
436-
unsafe { *(frame_data.slot_addr(self.cursor.frame().fp()) as *mut _) };
435+
let vmctx: usize =
436+
unsafe { *(frame_data.slot_addr(self.cursor.frame().fp()) as *mut usize) };
437+
let vmctx: *mut VMContext = core::ptr::with_exposed_provenance_mut(vmctx);
437438
let vmctx = NonNull::new(vmctx).expect("null vmctx in debug state slot");
438439
// SAFETY: the stored vmctx value is a valid instance in this
439440
// store; we only visit frames from this store in the

crates/wasmtime/src/runtime/vm/sys/unix/machports.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ unsafe extern "C" fn sigbus_handler(
143143
};
144144
unsafe {
145145
let faulting_addr = (*siginfo).si_addr() as usize;
146-
let range = &info.vm_store_context.as_ref().async_guard_range;
146+
let range = &info.vm_store_context.get().as_ref().async_guard_range;
147147
if range.start.addr() <= faulting_addr && faulting_addr < range.end.addr() {
148148
super::signals::abort_stack_overflow();
149149
}

crates/wasmtime/src/runtime/vm/sys/unix/signals.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ unsafe extern "C" fn trap_handler(
171171
match test {
172172
TrapTest::NotWasm => {
173173
if let Some(faulting_addr) = faulting_addr {
174-
let range = unsafe { &info.vm_store_context.as_ref().async_guard_range };
174+
let range = unsafe { &info.vm_store_context.get().as_ref().async_guard_range };
175175
if range.start.addr() <= faulting_addr && faulting_addr < range.end.addr() {
176176
abort_stack_overflow();
177177
}

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -544,7 +544,7 @@ mod call_thread_state {
544544
#[cfg(feature = "coredump")]
545545
pub(super) capture_coredump: bool,
546546

547-
pub(crate) vm_store_context: NonNull<VMStoreContext>,
547+
pub(crate) vm_store_context: Cell<NonNull<VMStoreContext>>,
548548
pub(crate) unwinder: &'static dyn Unwind,
549549

550550
pub(super) prev: Cell<tls::Ptr>,
@@ -582,7 +582,7 @@ mod call_thread_state {
582582
capture_backtrace: store.engine().config().wasm_backtrace,
583583
#[cfg(feature = "coredump")]
584584
capture_coredump: store.engine().config().coredump_on_trap,
585-
vm_store_context: store.vm_store_context_ptr(),
585+
vm_store_context: Cell::new(store.vm_store_context_ptr()),
586586
prev: Cell::new(ptr::null()),
587587
old_state,
588588
}
@@ -670,7 +670,7 @@ mod call_thread_state {
670670
}
671671

672672
unsafe {
673-
let cx = self.vm_store_context.as_ref();
673+
let cx = self.vm_store_context.get().as_ref();
674674
swap(
675675
&cx.last_wasm_exit_trampoline_fp,
676676
&mut (*self.old_state).last_wasm_exit_trampoline_fp,
@@ -751,6 +751,10 @@ impl CallThreadState {
751751
let prev = self.unwind.replace(UnwindState::None);
752752
assert!(prev.is_none());
753753
}
754+
755+
// Avoid unused-variable warning in non-exceptions/GC build.
756+
let _ = store;
757+
754758
let state = match reason {
755759
#[cfg(all(feature = "std", panic = "unwind"))]
756760
UnwindReason::Panic(err) => {
@@ -807,10 +811,10 @@ impl CallThreadState {
807811
}
808812
};
809813

810-
// Avoid unused-variable warning in non-exceptions/GC build.
811-
let _ = store;
812-
813814
self.unwind.set(state);
815+
816+
// Re-derive our VMStoreContext pointer for provenance.
817+
self.vm_store_context.set(store.vm_store_context_ptr());
814818
}
815819

816820
/// Helper function to perform an actual unwinding operation.
@@ -948,7 +952,7 @@ impl CallThreadState {
948952

949953
pub(crate) fn entry_trap_handler(&self) -> Handler {
950954
unsafe {
951-
let vm_store_context = self.vm_store_context.as_ref();
955+
let vm_store_context = self.vm_store_context.get().as_ref();
952956
let fp = *vm_store_context.last_wasm_entry_fp.get();
953957
let sp = *vm_store_context.last_wasm_entry_sp.get();
954958
let pc = *vm_store_context.last_wasm_entry_trap_handler.get();
@@ -1057,8 +1061,10 @@ impl CallThreadState {
10571061
faulting_addr: Option<usize>,
10581062
trap: wasmtime_environ::Trap,
10591063
) {
1060-
let backtrace = self.capture_backtrace(self.vm_store_context.as_ptr(), Some((pc, fp)));
1061-
let coredump_stack = self.capture_coredump(self.vm_store_context.as_ptr(), Some((pc, fp)));
1064+
let backtrace =
1065+
self.capture_backtrace(self.vm_store_context.get().as_ptr(), Some((pc, fp)));
1066+
let coredump_stack =
1067+
self.capture_coredump(self.vm_store_context.get().as_ptr(), Some((pc, fp)));
10621068
self.unwind.set(UnwindState::UnwindToHost {
10631069
reason: UnwindReason::Trap(TrapReason::Jit {
10641070
pc,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ impl Backtrace {
220220
Some((pc, fp)) => {
221221
assert!(core::ptr::eq(
222222
vm_store_context,
223-
state.vm_store_context.as_ptr()
223+
state.vm_store_context.get().as_ptr()
224224
));
225225
(pc, fp)
226226
}
@@ -251,7 +251,7 @@ impl Backtrace {
251251
.iter()
252252
.flat_map(|state| state.iter())
253253
.filter(|state| {
254-
core::ptr::eq(vm_store_context, state.vm_store_context.as_ptr())
254+
core::ptr::eq(vm_store_context, state.vm_store_context.get().as_ptr())
255255
})
256256
.map(|state| unsafe {
257257
(

tests/all/pulley.rs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ fn provenance_test_config() -> Config {
8989
config.wasm_component_model_async_stackful(true);
9090
config.wasm_component_model_threading(true);
9191
config.wasm_component_model_error_context(true);
92+
config.guest_debug(true);
9293
config
9394
}
9495

@@ -133,12 +134,29 @@ fn pulley_provenance_test() -> Result<()> {
133134
vec![],
134135
vec![ValType::I32, ValType::I32, ValType::I32],
135136
);
136-
let host_new = Func::new(&mut store, host_new_ty, |_, _params, results| {
137-
results[0] = Val::I32(1);
138-
results[1] = Val::I32(2);
139-
results[2] = Val::I32(3);
140-
Ok(())
141-
});
137+
let module_clone = module.clone();
138+
let host_new = Func::new(
139+
&mut store,
140+
host_new_ty,
141+
move |mut caller, _params, results| {
142+
let caller_frame = caller.debug_exit_frames().next().unwrap();
143+
let caller_module = caller_frame.module(&mut caller).unwrap().unwrap();
144+
assert!(Module::same(caller_module, &module_clone));
145+
let (caller_func, pc) = caller_frame
146+
.wasm_function_index_and_pc(&mut caller)
147+
.unwrap()
148+
.unwrap();
149+
assert_eq!(caller_func.as_u32(), 3);
150+
assert_eq!(pc, 416);
151+
let parent_frame = caller_frame.parent(&mut caller).unwrap();
152+
assert!(parent_frame.is_none());
153+
154+
results[0] = Val::I32(1);
155+
results[1] = Val::I32(2);
156+
results[2] = Val::I32(3);
157+
Ok(())
158+
},
159+
);
142160
let instance = Instance::new(&mut store, &module, &[host_wrap.into(), host_new.into()])?;
143161

144162
for func in [

0 commit comments

Comments
 (0)