Debug API: switch DebugFrameCursor over to monomorphization on T, and provide AsContextMut. (#11873)

* Debug API: switch `DebugFrameCursor` over to monomorphization on T, and provide `AsContextMut`.

This permits use of ordinary store accessors, such as GC object
accessors, while viewing the stack. In #11835 and related discussion,
we've concluded that (i) it is safe to provide this mutable access,
including e.g. ability to do a GC, because debugger access comes only
at points that are morally like hostcalls (right now, literally with a
`Caller`, and eventually with debug yields); and (ii) we will need
this in order to provide the full range of expected debugger
functionality. Even before mutation-during-debugging comes into play,
with the API before this change, it was not possible to read GC
objects at all (because all accessors take a mutable store).

* Review feedback.

* Fix some disable-feature builds.

Author: cfallin

crates/wasmtime/Cargo.toml

@@ -416,3 +416,4 @@ component-model-async-bytes = [
 
 # Enables support for guest debugging.
 debug = ['runtime']
+

crates/wasmtime/src/runtime/debug.rs

@@ -1,21 +1,25 @@
 //! Debugging API.
 
 use crate::{
-    AnyRef, ExnRef, ExternRef, Func, Instance, Module, Val,
+    AnyRef, AsContext, AsContextMut, ExnRef, ExternRef, Func, Instance, Module, StoreContext,
+    StoreContextMut, Val,
     store::{AutoAssertNoGc, StoreOpaque},
     vm::{CurrentActivationBacktrace, VMContext},
 };
+use alloc::vec;
 use alloc::vec::Vec;
 use core::{ffi::c_void, ptr::NonNull};
+#[cfg(feature = "gc")]
+use wasmtime_environ::FrameTable;
 use wasmtime_environ::{
     DefinedFuncIndex, FrameInstPos, FrameStackShape, FrameStateSlot, FrameStateSlotOffset,
-    FrameTable, FrameTableDescriptorIndex, FrameValType, FuncKey,
+    FrameTableDescriptorIndex, FrameValType, FuncKey,
 };
 use wasmtime_unwinder::Frame;
 
 use super::store::AsStoreOpaque;
 
-impl StoreOpaque {
+impl<'a, T> StoreContextMut<'a, T> {
     /// Provide an object that captures Wasm stack state, including
     /// Wasm VM-level values (locals and operand stack).
     ///
@@ -30,7 +34,7 @@ impl StoreOpaque {
     ///
     /// Returns `None` if debug instrumentation is not enabled for
     /// the engine containing this store.
-    pub fn debug_frames(&mut self) -> Option<DebugFrameCursor<'_>> {
+    pub fn debug_frames(self) -> Option<DebugFrameCursor<'a, T>> {
         if !self.engine().tunables().debug_guest {
             return None;
         }
@@ -56,12 +60,12 @@ impl StoreOpaque {
 ///
 /// See the documentation on `Store::stack_value` for more information
 /// about which frames this view will show.
-pub struct DebugFrameCursor<'a> {
+pub struct DebugFrameCursor<'a, T: 'static> {
     /// Iterator over frames.
     ///
     /// This iterator owns the store while the view exists (accessible
     /// as `iter.store`).
-    iter: CurrentActivationBacktrace<'a>,
+    iter: CurrentActivationBacktrace<'a, T>,
 
     /// Is the next frame to be visited by the iterator a trapping
     /// frame?
@@ -84,7 +88,7 @@ pub struct DebugFrameCursor<'a> {
     current: Option<FrameData>,
 }
 
-impl<'a> DebugFrameCursor<'a> {
+impl<'a, T: 'static> DebugFrameCursor<'a, T> {
     /// Move up to the next frame in the activation.
     pub fn move_to_parent(&mut self) {
         // If there are no virtual frames to yield, take and decode
@@ -102,8 +106,11 @@ impl<'a> DebugFrameCursor<'a> {
             let Some(next_frame) = self.iter.next() else {
                 return;
             };
-            self.frames =
-                VirtualFrame::decode(&mut self.iter.store, next_frame, self.is_trapping_frame);
+            self.frames = VirtualFrame::decode(
+                self.iter.store.0.as_store_opaque(),
+                next_frame,
+                self.is_trapping_frame,
+            );
             debug_assert!(!self.frames.is_empty());
             self.is_trapping_frame = false;
         }
@@ -139,7 +146,7 @@ impl<'a> DebugFrameCursor<'a> {
     /// Get the instance associated with the current frame.
     pub fn instance(&mut self) -> Instance {
         let instance = self.raw_instance();
-        Instance::from_wasmtime(instance.id(), self.iter.store.as_store_opaque())
+        Instance::from_wasmtime(instance.id(), self.iter.store.0.as_store_opaque())
     }
 
     /// Get the module associated with the current frame, if any
@@ -190,7 +197,7 @@ impl<'a> DebugFrameCursor<'a> {
         let slot_addr = data.slot_addr;
         // SAFETY: compiler produced metadata to describe this local
         // slot and stored a value of the correct type into it.
-        unsafe { read_value(&mut self.iter.store, slot_addr, offset, ty) }
+        unsafe { read_value(&mut self.iter.store.0, slot_addr, offset, ty) }
     }
 
     /// Get the type and value of the given operand-stack value in
@@ -207,7 +214,7 @@ impl<'a> DebugFrameCursor<'a> {
         // SAFETY: compiler produced metadata to describe this
         // operand-stack slot and stored a value of the correct type
         // into it.
-        unsafe { read_value(&mut self.iter.store, slot_addr, offset, ty) }
+        unsafe { read_value(&mut self.iter.store.0, slot_addr, offset, ty) }
     }
 }
 
@@ -236,11 +243,7 @@ struct VirtualFrame {
 impl VirtualFrame {
     /// Return virtual frames corresponding to a physical frame, from
     /// outermost to innermost.
-    fn decode(
-        store: &mut AutoAssertNoGc<'_>,
-        frame: Frame,
-        is_trapping_frame: bool,
-    ) -> Vec<VirtualFrame> {
+    fn decode(store: &mut StoreOpaque, frame: Frame, is_trapping_frame: bool) -> Vec<VirtualFrame> {
         let module = store
             .modules()
             .lookup_module_by_pc(frame.pc())
@@ -336,7 +339,7 @@ impl FrameData {
 /// instrumentation is correct, and as long as the tables are
 /// preserved through serialization).
 unsafe fn read_value(
-    store: &mut AutoAssertNoGc<'_>,
+    store: &mut StoreOpaque,
     slot_base: *const u8,
     offset: FrameStateSlotOffset,
     ty: FrameValType,
@@ -399,6 +402,7 @@ unsafe fn read_value(
 // Note: ideally this would be an impl Iterator, but this is quite
 // awkward because of the locally computed data (FrameStateSlot::parse
 // structured result) within the closure borrowed by a nested closure.
+#[cfg(feature = "gc")]
 pub(crate) fn gc_refs_in_frame<'a>(ft: FrameTable<'a>, pc: u32, fp: *mut usize) -> Vec<*mut u32> {
     let fp = fp.cast::<u8>();
     let mut ret = vec![];
@@ -429,3 +433,15 @@ pub(crate) fn gc_refs_in_frame<'a>(ft: FrameTable<'a>, pc: u32, fp: *mut usize)
     }
     ret
 }
+
+impl<'a, T: 'static> AsContext for DebugFrameCursor<'a, T> {
+    type Data = T;
+    fn as_context(&self) -> StoreContext<'_, Self::Data> {
+        StoreContext(self.iter.store.0)
+    }
+}
+impl<'a, T: 'static> AsContextMut for DebugFrameCursor<'a, T> {
+    fn as_context_mut(&mut self) -> StoreContextMut<'_, Self::Data> {
+        StoreContextMut(self.iter.store.0)
+    }
+}

crates/wasmtime/src/runtime/func.rs

@@ -2235,8 +2235,8 @@ impl<T> Caller<'_, T> {
     ///
     /// See ['Store::debug_frames`] for more details.
     #[cfg(feature = "debug")]
-    pub fn debug_frames(&mut self) -> Option<crate::DebugFrameCursor<'_>> {
-        self.store.debug_frames()
+    pub fn debug_frames(&mut self) -> Option<crate::DebugFrameCursor<'_, T>> {
+        self.store.as_context_mut().debug_frames()
     }
 }
 

crates/wasmtime/src/runtime/store.rs

@@ -1184,8 +1184,8 @@ impl<T> Store<T> {
     /// Returns `None` if debug instrumentation is not enabled for
     /// the engine containing this store.
     #[cfg(feature = "debug")]
-    pub fn debug_frames(&mut self) -> Option<crate::DebugFrameCursor<'_>> {
-        self.inner.debug_frames()
+    pub fn debug_frames(&mut self) -> Option<crate::DebugFrameCursor<'_, T>> {
+        self.as_context_mut().debug_frames()
     }
 }
 
@@ -1310,16 +1310,6 @@ impl<'a, T> StoreContextMut<'a, T> {
     pub fn has_pending_exception(&self) -> bool {
         self.0.inner.pending_exception.is_some()
     }
-
-    /// Provide an object that views Wasm stack state, including Wasm
-    /// VM-level values (locals and operand stack), when debugging is
-    /// enabled.
-    ///
-    /// See ['Store::debug_frames`] for more details.
-    #[cfg(feature = "debug")]
-    pub fn debug_frames(&mut self) -> Option<crate::DebugFrameCursor<'_>> {
-        self.0.inner.debug_frames()
-    }
 }
 
 impl<T> StoreInner<T> {
@@ -2707,6 +2697,12 @@ impl AsStoreOpaque for dyn VMStore {
     }
 }
 
+impl<T: 'static> AsStoreOpaque for Store<T> {
+    fn as_store_opaque(&mut self) -> &mut StoreOpaque {
+        &mut self.inner.inner
+    }
+}
+
 impl<T: 'static> AsStoreOpaque for StoreInner<T> {
     fn as_store_opaque(&mut self) -> &mut StoreOpaque {
         self

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

@@ -21,15 +21,15 @@
 //! exit FP and stopping once we reach the entry SP (meaning that the next older
 //! frame is a host frame).
 
+#[cfg(feature = "debug")]
+use crate::StoreContextMut;
 use crate::prelude::*;
 use crate::runtime::store::StoreOpaque;
 use crate::runtime::vm::stack_switching::VMStackChain;
 use crate::runtime::vm::{
     Unwind, VMStoreContext,
     traphandlers::{CallThreadState, tls},
 };
-#[cfg(feature = "debug")]
-use crate::store::AutoAssertNoGc;
 #[cfg(all(feature = "gc", feature = "stack-switching"))]
 use crate::vm::stack_switching::{VMContRef, VMStackState};
 use core::ops::ControlFlow;
@@ -330,13 +330,13 @@ impl Backtrace {
 
 /// An iterator over one Wasm activation.
 #[cfg(feature = "debug")]
-pub(crate) struct CurrentActivationBacktrace<'a> {
-    pub(crate) store: AutoAssertNoGc<'a>,
+pub(crate) struct CurrentActivationBacktrace<'a, T: 'static> {
+    pub(crate) store: StoreContextMut<'a, T>,
     inner: Box<dyn Iterator<Item = Frame>>,
 }
 
 #[cfg(feature = "debug")]
-impl<'a> CurrentActivationBacktrace<'a> {
+impl<'a, T: 'static> CurrentActivationBacktrace<'a, T> {
     /// Return an iterator over the most recent Wasm activation.
     ///
     /// The iterator captures the store with a mutable borrow, and
@@ -360,25 +360,24 @@ impl<'a> CurrentActivationBacktrace<'a> {
     /// while within host code called from that activation (which will
     /// ordinarily be ensured if the `store`'s lifetime came from the
     /// host entry point) then everything will be sound.
-    pub(crate) unsafe fn new(store: &'a mut StoreOpaque) -> CurrentActivationBacktrace<'a> {
+    pub(crate) unsafe fn new(store: StoreContextMut<'a, T>) -> CurrentActivationBacktrace<'a, T> {
         // Get the initial exit FP, exit PC, and entry FP.
-        let vm_store_context = store.vm_store_context();
+        let vm_store_context = store.0.vm_store_context();
         let exit_pc = unsafe { *(*vm_store_context).last_wasm_exit_pc.get() };
         let exit_fp = unsafe { (*vm_store_context).last_wasm_exit_fp() };
         let trampoline_fp = unsafe { *(*vm_store_context).last_wasm_entry_fp.get() };
-        let unwind = store.unwinder();
+        let unwind = store.0.unwinder();
         // Establish the iterator.
         let inner = Box::new(unsafe {
             wasmtime_unwinder::frame_iterator(unwind, exit_pc, exit_fp, trampoline_fp)
         });
 
-        let store = AutoAssertNoGc::new(store);
         CurrentActivationBacktrace { store, inner }
     }
 }
 
 #[cfg(feature = "debug")]
-impl<'a> Iterator for CurrentActivationBacktrace<'a> {
+impl<'a, T: 'static> Iterator for CurrentActivationBacktrace<'a, T> {
     type Item = Frame;
     fn next(&mut self) -> Option<Self::Item> {
         self.inner.next()

tests/all/debug.rs

@@ -1,20 +1,26 @@
 //! Tests for instrumentation-based debugging.
 
-use wasmtime::{Caller, Config, Engine, Extern, Func, Instance, Module, Store};
+use wasmtime::{AsContextMut, Caller, Config, Engine, Extern, Func, Instance, Module, Store};
 
-fn test_stack_values<C: Fn(&mut Config), F: Fn(Caller<'_, ()>) + Send + Sync + 'static>(
-    wat: &str,
+fn get_module_and_store<C: Fn(&mut Config)>(
     c: C,
-    f: F,
-) -> anyhow::Result<()> {
+    wat: &str,
+) -> anyhow::Result<(Module, Store<()>)> {
     let mut config = Config::default();
     config.guest_debug(true);
     config.wasm_exceptions(true);
     c(&mut config);
     let engine = Engine::new(&config)?;
     let module = Module::new(&engine, wat)?;
+    Ok((module, Store::new(&engine, ())))
+}
 
-    let mut store = Store::new(&engine, ());
+fn test_stack_values<C: Fn(&mut Config), F: Fn(Caller<'_, ()>) + Send + Sync + 'static>(
+    wat: &str,
+    c: C,
+    f: F,
+) -> anyhow::Result<()> {
+    let (module, mut store) = get_module_and_store(c, wat)?;
     let func = Func::wrap(&mut store, move |caller: Caller<'_, ()>| {
         f(caller);
     });
@@ -137,3 +143,46 @@ fn stack_values_dead_gc_ref() -> anyhow::Result<()> {
         },
     )
 }
+
+#[test]
+#[cfg_attr(miri, ignore)]
+fn gc_access_during_call() -> anyhow::Result<()> {
+    test_stack_values(
+        r#"
+    (module
+      (type $s (struct (field i32)))
+      (import "" "host" (func))
+      (func (export "main")
+        (local $l (ref null $s))
+        (local.set $l (struct.new $s (i32.const 42)))
+        (call 0)))
+    "#,
+        |config| {
+            config.wasm_gc(true);
+        },
+        |mut caller: Caller<'_, ()>| {
+            let mut stack = caller.debug_frames().unwrap();
+
+            // Do a GC while we hold the stack cursor.
+            stack.as_context_mut().gc(None);
+
+            assert!(!stack.done());
+            assert_eq!(stack.num_stacks(), 0);
+            assert_eq!(stack.num_locals(), 1);
+            // Note that this struct is dead during the call, and the
+            // ref could otherwise be optimized away (no longer in the
+            // stackmap at this point); but we verify it is still
+            // alive here because it is rooted in the
+            // debug-instrumentation slot.
+            let s = stack
+                .local(0)
+                .unwrap_any_ref()
+                .unwrap()
+                .unwrap_struct(&stack)
+                .unwrap();
+            assert_eq!(s.field(&mut stack, 0).unwrap().unwrap_i32(), 42);
+            stack.move_to_parent();
+            assert!(stack.done());
+        },
+    )
+}