Debugging: add a debugger callback mechanism to handle debug events. (#11895)

* Debugging: add a debugger callback mechanism to handle debug events.

This PR adds a notion of "debug events", and a mechanism in Wasmtime to
associate a "debug handler" with a store such that the handler is
invoked as-if it were an async hostcall on each event. The async
handler owns the store while its future exists, so the whole "world"
(within the store) is frozen and the handler can examine any state it
likes with a `StoreContextMut`.

Note that this callback-based scheme is a compromise: eventually, we
would like to have a native async API that produces a stream of events,
as sketched in #11826 and in [this branch]. However, the async approach
implemented naively (that is, with manual fiber suspends and with state
passed on the store) suffers from unsoundness in the presence of dropped
futures. Alex, Nick and I discussed this extensively and agreed that the
`Accessor` mechanism is the right way to allow for a debugger to have
"timesliced"/"shared" access to a store (only when polled/when an event
is delivered), but we will defer that for now, because it requires
additional work (mainly, converting existing async yield points in the
runtime to "give up" the store with the `run_concurrent` mechanism).
I'll file a followup issue to track that. The idea is that we can
eventually build that when ready, but the API we provide to a debugger
component can remain unchanged; only this plumbing and the glue to the
debugger component will be reworked.

With this scheme based on callbacks, we expect that one should be able
to implement a debugger using async channels to communicate with the
callback. The idea is that there would be a protocol where the callback
sends a debug event to the debugger main loop elsewhere in the executor
(e.g., over a Tokio channel or other async channel mechanism), and when
the debugger wants to allow execution to continue, it sends a "continue"
message back. In the meantime, while the world is paused, the debugger
can send messages to the callback to query the `StoreContextMut` it has
and read out state. This indirection/proxying of Store access is
necessary for soundness: again, teleporting the Store out may look like
it almost works ("it is like a mutable reborrow on a hostcall") except
in the presence of dropped futures with sandwiched Wasm->host->Wasm
situations.

This PR implements debug events for a few cases that can be caught
directly in the runtime, e.g., exceptions and traps raised just before
re-entry to Wasm. Other kinds of traps, such as those normally
implemented by host signals, require additional work (as in #11826) to
implement "hostcall injection" on signal reception; and breakpoints will
be built on top of that. The point of this PR is only to get the initial
plumbing in place for events.

[this branch]: https://github.com/cfallin/wasmtime/tree/wasmtime-debug-async

* Add some more tests.

* Review feedback: comment updates, and make `debug` feature depend on `async`.

* Review feedback: debug-hook setter requires guest debugging to be enabled.

* Review feedback: ThrownException event; handle block_on errors; explicitly list UnwindState cases.

* Add comment about load-bearing Send requirement.

* Fix no-unwind build.

* Review feedback: pass in hostcall error messages while keeping the trait object-safe.

Co-authored-by: Alex Crichton <[email protected]>

* Ignore divide-trapping test on Pulley for now.

---------

Co-authored-by: Alex Crichton <[email protected]>

Author: cfallin

crates/wasmtime/Cargo.toml

@@ -415,5 +415,8 @@ component-model-async-bytes = [
 ]
 
 # Enables support for guest debugging.
-debug = ['runtime']
+debug = [
+    'runtime',
+    'async'
+]
 

crates/wasmtime/src/runtime/debug.rs

@@ -1,8 +1,8 @@
 //! Debugging API.
 
 use crate::{
-    AnyRef, AsContext, AsContextMut, ExnRef, ExternRef, Func, Instance, Module, StoreContext,
-    StoreContextMut, Val,
+    AnyRef, AsContext, AsContextMut, ExnRef, ExternRef, Func, Instance, Module, OwnedRooted,
+    StoreContext, StoreContextMut, Val,
     store::{AutoAssertNoGc, StoreOpaque},
     vm::{CurrentActivationBacktrace, VMContext},
 };
@@ -13,7 +13,7 @@ use core::{ffi::c_void, ptr::NonNull};
 use wasmtime_environ::FrameTable;
 use wasmtime_environ::{
     DefinedFuncIndex, FrameInstPos, FrameStackShape, FrameStateSlot, FrameStateSlotOffset,
-    FrameTableDescriptorIndex, FrameValType, FuncKey,
+    FrameTableDescriptorIndex, FrameValType, FuncKey, Trap,
 };
 use wasmtime_unwinder::Frame;
 
@@ -445,3 +445,75 @@ impl<'a, T: 'static> AsContextMut for DebugFrameCursor<'a, T> {
         StoreContextMut(self.iter.store.0)
     }
 }
+
+/// One debug event that occurs when running Wasm code on a store with
+/// a debug handler attached.
+#[derive(Debug)]
+pub enum DebugEvent<'a> {
+    /// An `anyhow::Error` was raised by a hostcall.
+    HostcallError(&'a anyhow::Error),
+    /// An exception is thrown and caught by Wasm. The current state
+    /// is at the throw-point.
+    CaughtExceptionThrown(OwnedRooted<ExnRef>),
+    /// An exception was not caught and is escaping to the host.
+    UncaughtExceptionThrown(OwnedRooted<ExnRef>),
+    /// A Wasm trap occurred.
+    Trap(Trap),
+}
+
+/// A handler for debug events.
+///
+/// This is an async callback that is invoked directly within the
+/// context of a debug event that occurs, i.e., with the Wasm code
+/// still on the stack. The callback can thus observe that stack, up
+/// to the most recent entry to Wasm.[^1]
+///
+/// Because this callback receives a `StoreContextMut`, it has full
+/// access to any state that any other hostcall has, including the
+/// `T`. In that way, it is like an epoch-deadline callback or a
+/// call-hook callback. It also "freezes" the entire store for the
+/// duration of the debugger callback future.
+///
+/// In the future, we expect to provide an "externally async" API on
+/// the `Store` that allows receiving a stream of debug events and
+/// accessing the store mutably while frozen; that will need to
+/// integrate with [`Store::run_concurrent`] to properly timeslice and
+/// scope the mutable access to the store, and has not been built
+/// yet. In the meantime, it should be possible to build a fully
+/// functional debugger with this async-callback API by channeling
+/// debug events out, and requests to read the store back in, over
+/// message-passing channels between the callback and an external
+/// debugger main loop.
+///
+/// Note that the `handle` hook may use its mutable store access to
+/// invoke another Wasm. Debug events will also be caught and will
+/// cause further `handle` invocations during this recursive
+/// invocation. It is up to the debugger to handle any implications of
+/// this reentrancy (e.g., implications on a duplex channel protocol
+/// with an event/continue handshake) if it does so.
+///
+/// Note also that this trait has `Clone` as a supertrait, and the
+/// handler is cloned at every invocation as an artifact of the
+/// internal ownership structure of Wasmtime: the handler itself is
+/// owned by the store, but also receives a mutable borrow to the
+/// whole store, so we need to clone it out to invoke it. It is
+/// recommended that this trait be implemented by a type that is cheap
+/// to clone: for example, a single `Arc` handle to debugger state.
+///
+/// [^1]: Providing visibility further than the most recent entry to
+///       Wasm is not directly possible because it could see into
+///       another async stack, and the stack that polls the future
+///       running a particular Wasm invocation could change after each
+///       suspend point in the handler.
+pub trait DebugHandler: Clone + Send + Sync + 'static {
+    /// The data expected on the store that this handler is attached
+    /// to.
+    type Data;
+
+    /// Handle a debug event.
+    fn handle(
+        &self,
+        store: StoreContextMut<'_, Self::Data>,
+        event: DebugEvent<'_>,
+    ) -> impl Future<Output = ()> + Send;
+}

crates/wasmtime/src/runtime/store.rs

@@ -76,6 +76,10 @@
 //! contents of `StoreOpaque`. This is an invariant that we, as the authors of
 //! `wasmtime`, must uphold for the public interface to be safe.
 
+#[cfg(feature = "debug")]
+use crate::DebugHandler;
+#[cfg(all(feature = "gc", feature = "debug"))]
+use crate::OwnedRooted;
 use crate::RootSet;
 #[cfg(feature = "gc")]
 use crate::ThrownException;
@@ -259,6 +263,50 @@ pub struct StoreInner<T: 'static> {
     ///
     /// For comments about `ManuallyDrop`, see `Store::into_data`.
     data_no_provenance: ManuallyDrop<T>,
+
+    /// The user's debug handler, if any. See [`crate::DebugHandler`]
+    /// for more documentation.
+    ///
+    /// We need this to be an `Arc` because the handler itself takes
+    /// `&self` and also the whole Store mutably (via
+    /// `StoreContextMut`); so we need to hold a separate reference to
+    /// it while invoking it.
+    #[cfg(feature = "debug")]
+    debug_handler: Option<Box<dyn StoreDebugHandler<T>>>,
+}
+
+/// Adapter around `DebugHandler` that gets monomorphized into an
+/// object-safe dyn trait to place in `store.debug_handler`.
+#[cfg(feature = "debug")]
+trait StoreDebugHandler<T: 'static>: Send + Sync {
+    fn handle<'a>(
+        self: Box<Self>,
+        store: StoreContextMut<'a, T>,
+        event: crate::DebugEvent<'a>,
+    ) -> Box<dyn Future<Output = ()> + Send + 'a>;
+}
+
+#[cfg(feature = "debug")]
+impl<D> StoreDebugHandler<D::Data> for D
+where
+    D: DebugHandler,
+    D::Data: Send,
+{
+    fn handle<'a>(
+        self: Box<Self>,
+        store: StoreContextMut<'a, D::Data>,
+        event: crate::DebugEvent<'a>,
+    ) -> Box<dyn Future<Output = ()> + Send + 'a> {
+        // Clone the underlying `DebugHandler` (the trait requires
+        // Clone as a supertrait), not the Box. The clone happens here
+        // rather than at the callsite because `Clone::clone` is not
+        // object-safe so needs to be in a monomorphized context.
+        let handler: D = (*self).clone();
+        // Since we temporarily took `self` off the store at the
+        // callsite, put it back now that we've cloned it.
+        store.0.debug_handler = Some(self);
+        Box::new(async move { handler.handle(store, event).await })
+    }
 }
 
 enum ResourceLimiterInner<T> {
@@ -716,6 +764,8 @@ impl<T> Store<T> {
             #[cfg(target_has_atomic = "64")]
             epoch_deadline_behavior: None,
             data_no_provenance: ManuallyDrop::new(data),
+            #[cfg(feature = "debug")]
+            debug_handler: None,
         });
 
         let store_data =
@@ -1188,21 +1238,61 @@ impl<T> Store<T> {
     /// VM-level values (locals and operand stack), when debugging is
     /// enabled.
     ///
-    /// This object views all activations for the current store that
-    /// are on the stack. An activation is a contiguous sequence of
-    /// Wasm frames (called functions) that were called from host code
-    /// and called back out to host code. If there are activations
-    /// from multiple stores on the stack, for example if Wasm code in
-    /// one store calls out to host code which invokes another Wasm
-    /// function in another store, then the other stores are "opaque"
-    /// to our view here in the same way that host code is.
+    /// This object views the frames from the most recent Wasm entry
+    /// onward (up to the exit that allows this host code to run). Any
+    /// Wasm stack frames upward from the most recent entry to Wasm
+    /// are not visible to this cursor.
     ///
     /// 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<'_, T>> {
         self.as_context_mut().debug_frames()
     }
+
+    /// Set the debug callback on this store.
+    ///
+    /// See [`crate::DebugHandler`] for more documentation.
+    ///
+    /// # Panics
+    ///
+    /// - Will panic if this store is not configured for async
+    ///   support.
+    /// - Will panic if guest-debug support was not enabled via
+    ///   [`crate::Config::guest_debug`].
+    #[cfg(feature = "debug")]
+    pub fn set_debug_handler(&mut self, handler: impl DebugHandler<Data = T>)
+    where
+        // We require `Send` here because the debug handler becomes
+        // referenced from a future: when `DebugHandler::handle` is
+        // invoked, its `self` references the `handler` with the
+        // user's state. Note that we are careful to keep this bound
+        // constrained to debug-handler-related code only and not
+        // propagate it outward to the store in general. The presence
+        // of the trait implementation serves as a witness that `T:
+        // Send`. This is required in particular because we will have
+        // a `&mut dyn VMStore` on the stack when we pause a fiber
+        // with `block_on` to run a debugger hook; that `VMStore` must
+        // be a `Store<T> where T: Send`.
+        T: Send,
+    {
+        assert!(
+            self.inner.async_support(),
+            "debug hooks rely on async support"
+        );
+        assert!(
+            self.engine().tunables().debug_guest,
+            "debug hooks require guest debugging to be enabled"
+        );
+        self.inner.debug_handler = Some(Box::new(handler));
+    }
+
+    /// Clear the debug handler on this store. If any existed, it will
+    /// be dropped.
+    #[cfg(feature = "debug")]
+    pub fn clear_debug_handler(&mut self) {
+        self.inner.debug_handler = None;
+    }
 }
 
 impl<'a, T> StoreContext<'a, T> {
@@ -1320,7 +1410,6 @@ impl<'a, T> StoreContextMut<'a, T> {
 
     /// Tests whether there is a pending exception.
     ///
-    ///
     /// See [`Store::has_pending_exception`] for more details.
     #[cfg(feature = "gc")]
     pub fn has_pending_exception(&self) -> bool {
@@ -2543,13 +2632,31 @@ at https://bytecodealliance.org/security.
         self.pending_exception.take()
     }
 
+    /// Tests whether there is a pending exception.
+    #[cfg(feature = "gc")]
+    pub fn has_pending_exception(&self) -> bool {
+        self.pending_exception.is_some()
+    }
+
     #[cfg(feature = "gc")]
     fn take_pending_exception_rooted(&mut self) -> Option<Rooted<ExnRef>> {
         let vmexnref = self.take_pending_exception()?;
         let mut nogc = AutoAssertNoGc::new(self);
         Some(Rooted::new(&mut nogc, vmexnref.into()))
     }
 
+    /// Get an owned rooted reference to the pending exception,
+    /// without taking it off the store.
+    #[cfg(all(feature = "gc", feature = "debug"))]
+    pub(crate) fn pending_exception_owned_rooted(&mut self) -> Option<OwnedRooted<ExnRef>> {
+        let mut nogc = AutoAssertNoGc::new(self);
+        nogc.pending_exception.take().map(|vmexnref| {
+            let cloned = nogc.clone_gc_ref(vmexnref.as_gc_ref());
+            nogc.pending_exception = Some(cloned.into_exnref_unchecked());
+            OwnedRooted::new(&mut nogc, vmexnref.into())
+        })
+    }
+
     #[cfg(feature = "gc")]
     fn throw_impl(&mut self, exception: Rooted<ExnRef>) {
         let mut nogc = AutoAssertNoGc::new(self);
@@ -2645,6 +2752,18 @@ unsafe impl<T> VMStore for StoreInner<T> {
     fn component_calls(&mut self) -> &mut vm::component::CallContexts {
         &mut self.component_calls
     }
+
+    #[cfg(feature = "debug")]
+    fn block_on_debug_handler(&mut self, event: crate::DebugEvent<'_>) -> anyhow::Result<()> {
+        if let Some(handler) = self.debug_handler.take() {
+            log::trace!("about to raise debug event {event:?}");
+            StoreContextMut(self).with_blocking(|store, cx| {
+                cx.block_on(Pin::from(handler.handle(store, event)).as_mut())
+            })
+        } else {
+            Ok(())
+        }
+    }
 }
 
 impl<T> StoreInner<T> {

crates/wasmtime/src/runtime/vm.rs

@@ -228,6 +228,10 @@ pub unsafe trait VMStore: 'static {
     fn component_async_store(
         &mut self,
     ) -> &mut dyn crate::runtime::component::VMComponentAsyncStore;
+
+    /// Invoke a debug handler, if present, at a debug event.
+    #[cfg(feature = "debug")]
+    fn block_on_debug_handler(&mut self, event: crate::DebugEvent) -> anyhow::Result<()>;
 }
 
 impl Deref for dyn VMStore + '_ {