Debugging: refactor stack frame cursor into frame handle abstraction. (#12566)

* Debugging: refactor stack frame cursor into frame handle abstraction.

This addresses some of the issues described #12486: we need the ability
to keep a handle to a stack frame as long as execution is frozen, and
keep multiple of these handles around, alongside the `Store`, without
any handle directly holding a borrow of the store.

The frame handles work by means of an "execution version" scheme: the
idea is that whenever any execution resumes in a given store, all
handles to existing frames could be invalidated, but if no such
execution occurs, all handles should still be valid. A tuple of
(globally unique for process lifetime) store ID, and execution version
within that store, should be sufficient to uniquely identify any
frozen-stack period during execution. This accomplishes cheap handle
invalidation without the need to track existing handles.

This PR also implements a cache of parsed frame-table data. Previously
this was lazily parsed by the cursor as it walked up a stack, but with
multiple handles hanging around, and with handles meant to be cheap to
hold and clone, and with handles being invalidated eagerly, it makes
much more sense to persist this parsed metadata at the `Store` level.
(It cannot persist at the `Engine` level because PCs are local per
store.)

* Re-bless disas tests (offsets in VMStoreContext changed).

* Handle invalidation tests.

* Review comments, and make API return `Result`s rather than panic'ing on stale handles.

* Review feedback.

* Doc-comment link fix.

* Review feedback.

* cfg-gate Activation method to `debug` feature only.

* Fix unused-import warning in no-debug cfg.

* Fix doc link (again, after rename from latest feedback).

Author: cfallin

crates/cranelift/src/compiler.rs

@@ -434,8 +434,36 @@ impl wasmtime_environ::Compiler for Compiler {
             callee,
             &[callee_vmctx, caller_vmctx, args_base, args_len],
         );
+
+        // Increment the "execution version" on the VMStoreContext if
+        // guest debugging is enabled.
+        if self.tunables.debug_guest {
+            let vmstore_ctx_ptr = builder.ins().load(
+                pointer_type,
+                MemFlags::trusted().with_readonly(),
+                caller_vmctx,
+                i32::from(ptr_size.vmctx_store_context()),
+            );
+            let old_version = builder.ins().load(
+                ir::types::I64,
+                MemFlags::trusted(),
+                vmstore_ctx_ptr,
+                i32::from(ptr_size.vmstore_context_execution_version()),
+            );
+            let new_version = builder.ins().iadd_imm(old_version, 1);
+            builder.ins().store(
+                MemFlags::trusted(),
+                new_version,
+                vmstore_ctx_ptr,
+                i32::from(ptr_size.vmstore_context_execution_version()),
+            );
+        }
+
+        // Invoke `raise` if the callee (host) returned an error.
         let succeeded = builder.func.dfg.inst_results(call)[0];
         self.raise_if_host_trapped(&mut builder, caller_vmctx, succeeded);
+
+        // Return results from the array as native return values.
         let results =
             self.load_values_from_array(wasm_func_ty.returns(), &mut builder, args_base, args_len);
         builder.ins().return_(&results);

crates/debugger/src/lib.rs

@@ -498,75 +498,131 @@ mod test {
         assert!(matches!(event, DebugRunResult::Breakpoint));
         // At (before executing) first `local.get`.
         debugger
-            .with_store(|store| {
-                let mut frame = store.debug_frames().unwrap();
-                assert!(!frame.done());
-                assert_eq!(frame.wasm_function_index_and_pc().unwrap().0.as_u32(), 0);
-                assert_eq!(frame.wasm_function_index_and_pc().unwrap().1, 36);
-                assert_eq!(frame.num_locals(), 2);
-                assert_eq!(frame.num_stacks(), 0);
-                assert_eq!(frame.local(0).unwrap_i32(), 1);
-                assert_eq!(frame.local(1).unwrap_i32(), 2);
-                assert_eq!(frame.move_to_parent(), FrameParentResult::SameActivation);
-                assert!(frame.done());
+            .with_store(|mut store| {
+                let frame = store.debug_exit_frames().next().unwrap();
+                assert_eq!(
+                    frame
+                        .wasm_function_index_and_pc(&mut store)
+                        .unwrap()
+                        .unwrap()
+                        .0
+                        .as_u32(),
+                    0
+                );
+                assert_eq!(
+                    frame
+                        .wasm_function_index_and_pc(&mut store)
+                        .unwrap()
+                        .unwrap()
+                        .1,
+                    36
+                );
+                assert_eq!(frame.num_locals(&mut store).unwrap(), 2);
+                assert_eq!(frame.num_stacks(&mut store).unwrap(), 0);
+                assert_eq!(frame.local(&mut store, 0).unwrap().unwrap_i32(), 1);
+                assert_eq!(frame.local(&mut store, 1).unwrap().unwrap_i32(), 2);
+                let frame = frame.parent(&mut store).unwrap();
+                assert!(frame.is_none());
             })
             .await?;
 
         let event = debugger.run().await?;
         // At second `local.get`.
         assert!(matches!(event, DebugRunResult::Breakpoint));
         debugger
-            .with_store(|store| {
-                let mut frame = store.debug_frames().unwrap();
-                assert!(!frame.done());
-                assert_eq!(frame.wasm_function_index_and_pc().unwrap().0.as_u32(), 0);
-                assert_eq!(frame.wasm_function_index_and_pc().unwrap().1, 38);
-                assert_eq!(frame.num_locals(), 2);
-                assert_eq!(frame.num_stacks(), 1);
-                assert_eq!(frame.local(0).unwrap_i32(), 1);
-                assert_eq!(frame.local(1).unwrap_i32(), 2);
-                assert_eq!(frame.stack(0).unwrap_i32(), 1);
-                assert_eq!(frame.move_to_parent(), FrameParentResult::SameActivation);
-                assert!(frame.done());
+            .with_store(|mut store| {
+                let frame = store.debug_exit_frames().next().unwrap();
+                assert_eq!(
+                    frame
+                        .wasm_function_index_and_pc(&mut store)
+                        .unwrap()
+                        .unwrap()
+                        .0
+                        .as_u32(),
+                    0
+                );
+                assert_eq!(
+                    frame
+                        .wasm_function_index_and_pc(&mut store)
+                        .unwrap()
+                        .unwrap()
+                        .1,
+                    38
+                );
+                assert_eq!(frame.num_locals(&mut store).unwrap(), 2);
+                assert_eq!(frame.num_stacks(&mut store).unwrap(), 1);
+                assert_eq!(frame.local(&mut store, 0).unwrap().unwrap_i32(), 1);
+                assert_eq!(frame.local(&mut store, 1).unwrap().unwrap_i32(), 2);
+                assert_eq!(frame.stack(&mut store, 0).unwrap().unwrap_i32(), 1);
+                let frame = frame.parent(&mut store).unwrap();
+                assert!(frame.is_none());
             })
             .await?;
 
         let event = debugger.run().await?;
         // At `i32.add`.
         assert!(matches!(event, DebugRunResult::Breakpoint));
         debugger
-            .with_store(|store| {
-                let mut frame = store.debug_frames().unwrap();
-                assert!(!frame.done());
-                assert_eq!(frame.wasm_function_index_and_pc().unwrap().0.as_u32(), 0);
-                assert_eq!(frame.wasm_function_index_and_pc().unwrap().1, 40);
-                assert_eq!(frame.num_locals(), 2);
-                assert_eq!(frame.num_stacks(), 2);
-                assert_eq!(frame.local(0).unwrap_i32(), 1);
-                assert_eq!(frame.local(1).unwrap_i32(), 2);
-                assert_eq!(frame.stack(0).unwrap_i32(), 1);
-                assert_eq!(frame.stack(1).unwrap_i32(), 2);
-                assert_eq!(frame.move_to_parent(), FrameParentResult::SameActivation);
-                assert!(frame.done());
+            .with_store(|mut store| {
+                let frame = store.debug_exit_frames().next().unwrap();
+                assert_eq!(
+                    frame
+                        .wasm_function_index_and_pc(&mut store)
+                        .unwrap()
+                        .unwrap()
+                        .0
+                        .as_u32(),
+                    0
+                );
+                assert_eq!(
+                    frame
+                        .wasm_function_index_and_pc(&mut store)
+                        .unwrap()
+                        .unwrap()
+                        .1,
+                    40
+                );
+                assert_eq!(frame.num_locals(&mut store).unwrap(), 2);
+                assert_eq!(frame.num_stacks(&mut store).unwrap(), 2);
+                assert_eq!(frame.local(&mut store, 0).unwrap().unwrap_i32(), 1);
+                assert_eq!(frame.local(&mut store, 1).unwrap().unwrap_i32(), 2);
+                assert_eq!(frame.stack(&mut store, 0).unwrap().unwrap_i32(), 1);
+                assert_eq!(frame.stack(&mut store, 1).unwrap().unwrap_i32(), 2);
+                let frame = frame.parent(&mut store).unwrap();
+                assert!(frame.is_none());
             })
             .await?;
 
         let event = debugger.run().await?;
         // At return point.
         assert!(matches!(event, DebugRunResult::Breakpoint));
         debugger
-            .with_store(|store| {
-                let mut frame = store.debug_frames().unwrap();
-                assert!(!frame.done());
-                assert_eq!(frame.wasm_function_index_and_pc().unwrap().0.as_u32(), 0);
-                assert_eq!(frame.wasm_function_index_and_pc().unwrap().1, 41);
-                assert_eq!(frame.num_locals(), 2);
-                assert_eq!(frame.num_stacks(), 1);
-                assert_eq!(frame.local(0).unwrap_i32(), 1);
-                assert_eq!(frame.local(1).unwrap_i32(), 2);
-                assert_eq!(frame.stack(0).unwrap_i32(), 3);
-                assert_eq!(frame.move_to_parent(), FrameParentResult::SameActivation);
-                assert!(frame.done());
+            .with_store(|mut store| {
+                let frame = store.debug_exit_frames().next().unwrap();
+                assert_eq!(
+                    frame
+                        .wasm_function_index_and_pc(&mut store)
+                        .unwrap()
+                        .unwrap()
+                        .0
+                        .as_u32(),
+                    0
+                );
+                assert_eq!(
+                    frame
+                        .wasm_function_index_and_pc(&mut store)
+                        .unwrap()
+                        .unwrap()
+                        .1,
+                    41
+                );
+                assert_eq!(frame.num_locals(&mut store).unwrap(), 2);
+                assert_eq!(frame.num_stacks(&mut store).unwrap(), 1);
+                assert_eq!(frame.local(&mut store, 0).unwrap().unwrap_i32(), 1);
+                assert_eq!(frame.local(&mut store, 1).unwrap().unwrap_i32(), 2);
+                assert_eq!(frame.stack(&mut store, 0).unwrap().unwrap_i32(), 3);
+                let frame = frame.parent(&mut store).unwrap();
+                assert!(frame.is_none());
             })
             .await?;
 

crates/environ/src/vmoffsets.rs

@@ -181,10 +181,17 @@ pub trait PtrSize {
         self.vmstore_context_fuel_consumed() + 8
     }
 
+    /// Return the offset of the `execution_version` field of
+    /// `VMStoreContext`
+    #[inline]
+    fn vmstore_context_execution_version(&self) -> u8 {
+        self.vmstore_context_epoch_deadline() + 8
+    }
+
     /// Return the offset of the `stack_limit` field of `VMStoreContext`
     #[inline]
     fn vmstore_context_stack_limit(&self) -> u8 {
-        self.vmstore_context_epoch_deadline() + 8
+        self.vmstore_context_execution_version() + 8
     }
 
     /// Return the offset of the `gc_heap` field of `VMStoreContext`.