Repository: bytecodealliance/wasmtime
Pull request #6321 (2023-05-01 23:08:16)

`wasmtime`: Fix resetting stack-walking registers when entering/exiting Wasm
Fixes a regression from #6262, originally reported in https://github.com/bytecodealliance/wasmtime-dotnet/pull/245

The issue was that we would enter Wasm and save the stack-walking registers but never clear them after Wasm returns. Then if a host-to-host call tried to capture a stack, we would mistakenly attempt to use those stale registers to start the stack walk. This mistake would be caught by an assertion, triggering a panic.

This commit fixes the issue by managing the save/restore in the `CallThreadState` construction/drop, rather than in the old `set_prev` method.

<!--
Please make sure you include the following information:

- If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

- Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->


alexcrichton: Oh one thing I thought of this morning -- this means that the backtrace code is no longer guaranteed to walk the stack in-order of activations. I don't believe there's any issues with that, but it's worth being aware of. The reason for this is that while the activations are ordered when we walk each activation we walk its `old_*` pointers which are the previous pointers for the limits. 

If you had A call B call A which trapped, then you'd get the first contiguous region in A but then when you walk over the `CallThreadState` the first one points to the first A on the stack. The next `CallThreadState` would... actually I think B would be dropped entirely?

Hm ok so now as I write this out, I think it means that `Backtrace` is no longer guaranteed to capture all frames on the stack, no in order. That's ok though since it's guaranteed to capture everything within one store's limits in order which is all we need.
jameysharp: I don't understand the issue you're raising, Alex, but my profiling work will produce misleading results if the subset of stack frames which are part of the user-selected modules are returned out of order. Should I be concerned about this?
jameysharp: The new profiler actually does filter to only frames from a specified set of modules, but your point still stands because it has the same problem that `WasmBacktrace` does, that the same module might be instantiated in multiple stores.
github-actions[bot]: #### Subscribe to Label Action

cc @fitzgen, @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:api", "wasmtime:ref-types"

Thus the following users have been cc'd because of the following labels:

* fitzgen: wasmtime:ref-types
* peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

[Learn more.](https://github.com/bytecodealliance/subscribe-to-label-action)
</details>

Patch:
diff --git a/crates/runtime/src/externref.rs b/crates/runtime/src/externref.rs
index cc90b48ac..b2d2e10c9 100644
--- a/crates/runtime/src/externref.rs
+++ b/crates/runtime/src/externref.rs
@@ -99,6 +99,7 @@
 //! Examination of Deferred Reference Counting and Cycle Detection* by Quinane:
 //! <https://openresearch-repository.anu.edu.au/bitstream/1885/42030/2/hon-thesis.pdf>
 
+use crate::{Backtrace, VMRuntimeLimits};
 use std::alloc::Layout;
 use std::any::Any;
 use std::cell::UnsafeCell;
@@ -111,8 +112,6 @@ use std::ptr::{self, NonNull};
 use std::sync::atomic::{self, AtomicUsize, Ordering};
 use wasmtime_environ::StackMap;
 
-use crate::Backtrace;
-
 /// An external reference to some opaque data.
 ///
 /// `VMExternRef`s dereference to their underlying opaque data as `dyn Any`.
@@ -649,6 +648,7 @@ impl VMExternRefActivationsTable {
     #[inline]
     pub unsafe fn insert_with_gc(
         &mut self,
+        limits: *const VMRuntimeLimits,
         externref: VMExternRef,
         module_info_lookup: &dyn ModuleInfoLookup,
     ) {
@@ -656,17 +656,18 @@ impl VMExternRefActivationsTable {
         assert!(self.gc_okay);
 
         if let Err(externref) = self.try_insert(externref) {
-            self.gc_and_insert_slow(externref, module_info_lookup);
+            self.gc_and_insert_slow(limits, externref, module_info_lookup);
         }
     }
 
     #[inline(never)]
     unsafe fn gc_and_insert_slow(
         &mut self,
+        limits: *const VMRuntimeLimits,
         externref: VMExternRef,
         module_info_lookup: &dyn ModuleInfoLookup,
     ) {
-        gc(module_info_lookup, self);
+        gc(limits, module_info_lookup, self);
 
         // Might as well insert right into the hash set, rather than the bump
         // chunk, since we are already on a slow path and we get de-duplication
@@ -854,6 +855,7 @@ 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`.
 pub unsafe fn gc(
+    limits: *const VMRuntimeLimits,
     module_info_lookup: &dyn ModuleInfoLookup,
     externref_activations_table: &mut VMExternRefActivationsTable,
 ) {
@@ -894,7 +896,7 @@ pub unsafe fn gc(
     }
 
     log::trace!("begin GC trace");
-    Backtrace::trace(|frame| {
+    Backtrace::trace(limits, |frame| {
         let pc = frame.pc();
         debug_assert!(pc != 0, "we should always get a valid PC for Wasm frames");
 
diff --git a/crates/runtime/src/libcalls.rs b/crates/runtime/src/libcalls.rs
index 8f5f84eae..4aa8133db 100644
--- a/crates/runtime/src/libcalls.rs
+++ b/crates/runtime/src/libcalls.rs
@@ -394,7 +394,8 @@ unsafe fn drop_externref(_vmctx: *mut VMContext, externref: *mut u8) {
 // `VMExternRefActivationsTable`.
 unsafe fn activations_table_insert_with_gc(vmctx: *mut VMContext, externref: *mut u8) {
     let externref = VMExternRef::clone_from_raw(externref);
-    let instance = (*vmctx).instance();
+    let instance = (*vmctx).instance_mut();
+    let limits = *instance.runtime_limits();
     let (activations_table, module_info_lookup) = (*instance.store()).externref_activations_table();
 
     // Invariant: all `externref`s on the stack have an entry in the activations
@@ -406,13 +407,14 @@ unsafe fn activations_table_insert_with_gc(vmctx: *mut VMContext, externref: *mu
     // but it isn't really a concern because this is already a slow path.
     activations_table.insert_without_gc(externref.clone());
 
-    activations_table.insert_with_gc(externref, module_info_lookup);
+    activations_table.insert_with_gc(limits, externref, module_info_lookup);
 }
 
 // Perform a Wasm `global.get` for `externref` globals.
 unsafe fn externref_global_get(vmctx: *mut VMContext, index: u32) -> *mut u8 {
     let index = GlobalIndex::from_u32(index);
     let instance = (*vmctx).instance_mut();
+    let limits = *instance.runtime_limits();
     let global = instance.defined_or_imported_global_ptr(index);
     match (*global).as_externref().clone() {
         None => ptr::null_mut(),
@@ -420,7 +422,7 @@ unsafe fn externref_global_get(vmctx: *mut VMContext, index: u32) -> *mut u8 {
             let raw = externref.as_raw();
             let (activations_table, module_info_lookup) =
                 (*instance.store()).externref_activations_table();
-            activations_table.insert_with_gc(externref, module_info_lookup);
+            activations_table.insert_with_gc(limits, externref, module_info_lookup);
             raw
         }
     }
diff --git a/crates/runtime/src/traphandlers.rs b/crates/runtime/src/traphandlers.rs
index e9dbc958d..bfae1568c 100644
--- a/crates/runtime/src/traphandlers.rs
+++ b/crates/runtime/src/traphandlers.rs
@@ -248,7 +248,6 @@ where
 // usage of its accessor methods.
 mod call_thread_state {
     use super::*;
-    use std::mem;
 
     /// Temporary state stored on the stack which is registered in the `tls` module
     /// below for calls into wasm.
@@ -262,18 +261,29 @@ mod call_thread_state {
 
         prev: Cell<tls::Ptr>,
 
-        // The values of `VMRuntimeLimits::last_wasm_{exit_{pc,fp},entry_sp}` for
-        // the *previous* `CallThreadState`. Our *current* last wasm PC/FP/SP are
-        // saved in `self.limits`. We save a copy of the old registers here because
-        // the `VMRuntimeLimits` typically doesn't change across nested calls into
-        // Wasm (i.e. they are typically calls back into the same store and
-        // `self.limits == self.prev.limits`) and we must to maintain the list of
+        // The values of `VMRuntimeLimits::last_wasm_{exit_{pc,fp},entry_sp}`
+        // for the *previous* `CallThreadState` for this same store/limits. Our
+        // *current* last wasm PC/FP/SP are saved in `self.limits`. We save a
+        // copy of the old registers here because the `VMRuntimeLimits`
+        // typically doesn't change across nested calls into Wasm (i.e. they are
+        // typically calls back into the same store and `self.limits ==
+        // self.prev.limits`) and we must to maintain the list of
         // contiguous-Wasm-frames stack regions for backtracing purposes.
         old_last_wasm_exit_fp: Cell<usize>,
         old_last_wasm_exit_pc: Cell<usize>,
         old_last_wasm_entry_sp: Cell<usize>,
     }
 
+    impl Drop for CallThreadState {
+        fn drop(&mut self) {
+            unsafe {
+                *(*self.limits).last_wasm_exit_fp.get() = self.old_last_wasm_exit_fp.get();
+                *(*self.limits).last_wasm_exit_pc.get() = self.old_last_wasm_exit_pc.get();
+                *(*self.limits).last_wasm_entry_sp.get() = self.old_last_wasm_entry_sp.get();
+            }
+        }
+    }
+
     impl CallThreadState {
         #[inline]
         pub(super) fn new(
@@ -288,9 +298,9 @@ mod call_thread_state {
                 capture_backtrace,
                 limits,
                 prev: Cell::new(ptr::null()),
-                old_last_wasm_exit_fp: Cell::new(0),
-                old_last_wasm_exit_pc: Cell::new(0),
-                old_last_wasm_entry_sp: Cell::new(0),
+                old_last_wasm_exit_fp: Cell::new(unsafe { *(*limits).last_wasm_exit_fp.get() }),
+                old_last_wasm_exit_pc: Cell::new(unsafe { *(*limits).last_wasm_exit_pc.get() }),
+                old_last_wasm_entry_sp: Cell::new(unsafe { *(*limits).last_wasm_entry_sp.get() }),
             }
         }
 
@@ -314,83 +324,15 @@ mod call_thread_state {
             self.prev.get()
         }
 
-        /// Connect the link to the previous `CallThreadState`.
-        ///
-        /// Synchronizes the last wasm FP, PC, and SP on `self` and the old
-        /// `self.prev` for the given new `prev`, and returns the old
-        /// `self.prev`.
-        pub unsafe fn set_prev(&self, prev: tls::Ptr) -> tls::Ptr {
-            let old_prev = self.prev.get();
-
-            // Restore the old `prev`'s saved registers in its
-            // `VMRuntimeLimits`. This is necessary for when we are async
-            // suspending the top `CallThreadState` and doing `set_prev(null)`
-            // on it, and so any stack walking we do subsequently will start at
-            // the old `prev` and look at its `VMRuntimeLimits` to get the
-            // initial saved registers.
-            if let Some(old_prev) = old_prev.as_ref() {
-                *(*old_prev.limits).last_wasm_exit_fp.get() = self.old_last_wasm_exit_fp();
-                *(*old_prev.limits).last_wasm_exit_pc.get() = self.old_last_wasm_exit_pc();
-                *(*old_prev.limits).last_wasm_entry_sp.get() = self.old_last_wasm_entry_sp();
-            }
-
-            self.prev.set(prev);
-
-            let mut old_last_wasm_exit_fp = 0;
-            let mut old_last_wasm_exit_pc = 0;
-            let mut old_last_wasm_entry_sp = 0;
-            if let Some(prev) = prev.as_ref() {
-                // We are entering a new `CallThreadState` or resuming a
-                // previously suspended one. This means we will push new Wasm
-                // frames that save the new Wasm FP/SP/PC registers into
-                // `VMRuntimeLimits`, we need to first save the old Wasm
-                // FP/SP/PC registers into this new `CallThreadState` to
-                // maintain our list of contiguous Wasm frame regions that we
-                // use when capturing stack traces.
-                //
-                // NB: the Wasm<--->host trampolines saved the Wasm FP/SP/PC
-                // registers in the active-at-that-time store's
-                // `VMRuntimeLimits`. For the most recent FP/PC/SP that is the
-                // `state.prev.limits` (since we haven't entered this
-                // `CallThreadState` yet). And that can be a different
-                // `VMRuntimeLimits` instance from the currently active
-                // `state.limits`, which will be used by the upcoming call into
-                // Wasm! Consider the case where we have multiple, nested calls
-                // across stores (with host code in between, by necessity, since
-                // only things in the same store can be linked directly
-                // together):
-                //
-                //     | ...             |
-                //     | Host            |  |
-                //     +-----------------+  | stack
-                //     | Wasm in store A |  | grows
-                //     +-----------------+  | down
-                //     | Host            |  |
-                //     +-----------------+  |
-                //     | Wasm in store B |  V
-                //     +-----------------+
-                //
-                // In this scenario `state.limits != state.prev.limits`,
-                // i.e. `B.limits != A.limits`! Therefore we must take care to
-                // read the old FP/SP/PC from `state.prev.limits`, rather than
-                // `state.limits`, and store those saved registers into the
-                // current `state`.
-                //
-                // See also the comment above the
-                // `CallThreadState::old_last_wasm_*` fields.
-                old_last_wasm_exit_fp =
-                    mem::replace(&mut *(*prev.limits).last_wasm_exit_fp.get(), 0);
-                old_last_wasm_exit_pc =
-                    mem::replace(&mut *(*prev.limits).last_wasm_exit_pc.get(), 0);
-                old_last_wasm_entry_sp =
-                    mem::replace(&mut *(*prev.limits).last_wasm_entry_sp.get(), 0);
-            }
-
-            self.old_last_wasm_exit_fp.set(old_last_wasm_exit_fp);
-            self.old_last_wasm_exit_pc.set(old_last_wasm_exit_pc);
-            self.old_last_wasm_entry_sp.set(old_last_wasm_entry_sp);
+        pub(crate) unsafe fn push(&self) {
+            assert!(self.prev.get().is_null());
+            self.prev.set(tls::raw::replace(self));
+        }
 
-            old_prev
+        pub(crate) unsafe fn pop(&self) {
+            let prev = self.prev.replace(ptr::null());
+            let head = tls::raw::replace(prev);
+            assert!(std::ptr::eq(head, self));
         }
     }
 }
@@ -433,7 +375,7 @@ impl CallThreadState {
                 needs_backtrace: false,
                 ..
             }) => None,
-            UnwindReason::Trap(_) => self.capture_backtrace(None),
+            UnwindReason::Trap(_) => self.capture_backtrace(self.limits, None),
         };
         unsafe {
             (*self.unwind.get()).as_mut_ptr().write((reason, backtrace));
@@ -487,7 +429,7 @@ impl CallThreadState {
     }
 
     fn set_jit_trap(&self, pc: *const u8, fp: usize, faulting_addr: Option<usize>) {
-        let backtrace = self.capture_backtrace(Some((pc as usize, fp)));
+        let backtrace = self.capture_backtrace(self.limits, Some((pc as usize, fp)));
         unsafe {
             (*self.unwind.get()).as_mut_ptr().write((
                 UnwindReason::Trap(TrapReason::Jit {
@@ -499,12 +441,16 @@ impl CallThreadState {
         }
     }
 
-    fn capture_backtrace(&self, pc_and_fp: Option<(usize, usize)>) -> Option<Backtrace> {
+    fn capture_backtrace(
+        &self,
+        limits: *const VMRuntimeLimits,
+        trap_pc_and_fp: Option<(usize, usize)>,
+    ) -> Option<Backtrace> {
         if !self.capture_backtrace {
             return None;
         }
 
-        Some(unsafe { Backtrace::new_with_trap_state(self, pc_and_fp) })
+        Some(unsafe { Backtrace::new_with_trap_state(limits, self, trap_pc_and_fp) })
     }
 
     pub(crate) fn iter<'a>(&'a self) -> impl Iterator<Item = &Self> + 'a {
@@ -533,7 +479,6 @@ impl<T: Copy> Drop for ResetCell<'_, T> {
 // the caller to the trap site.
 mod tls {
     use super::CallThreadState;
-    use std::ptr;
 
     pub use raw::Ptr;
 
@@ -551,7 +496,7 @@ mod tls {
     //
     // Note, though, that if async support is disabled at compile time then
     // these functions are free to be inlined.
-    mod raw {
+    pub(super) mod raw {
         use super::CallThreadState;
         use std::cell::Cell;
         use std::ptr;
@@ -625,8 +570,7 @@ mod tls {
             // accidentally used later.
             let state = raw::get();
             if let Some(state) = state.as_ref() {
-                let prev_state = state.set_prev(ptr::null());
-                raw::replace(prev_state);
+                state.pop();
             } else {
                 // Null case: we aren't in a wasm context, so theres no tls to
                 // save for restoration.
@@ -640,18 +584,12 @@ mod tls {
         /// This is unsafe because it's intended to only be used within the
         /// context of stack switching within wasmtime.
         pub unsafe fn replace(self) {
-            // Null case: we aren't in a wasm context, so theres no tls
-            // to restore.
-            if self.state.is_null() {
-                return;
+            if let Some(state) = self.state.as_ref() {
+                state.push();
+            } else {
+                // Null case: we aren't in a wasm context, so theres no tls
+                // to restore.
             }
-
-            // We need to configure our previous TLS pointer to whatever is in
-            // TLS at this time, and then we set the current state to ourselves.
-            let prev = raw::get();
-            assert!((*self.state).prev().is_null());
-            (*self.state).set_prev(prev);
-            raw::replace(self.state);
         }
     }
 
@@ -668,18 +606,13 @@ mod tls {
             #[inline]
             fn drop(&mut self) {
                 unsafe {
-                    let prev = self.state.set_prev(ptr::null());
-                    let old_state = raw::replace(prev);
-                    debug_assert!(std::ptr::eq(old_state, self.state));
+                    self.state.pop();
                 }
             }
         }
 
-        let prev = raw::replace(state);
-
         unsafe {
-            state.set_prev(prev);
-
+            state.push();
             let reset = Reset { state };
             closure(reset.state)
         }
diff --git a/crates/runtime/src/traphandlers/backtrace.rs b/crates/runtime/src/traphandlers/backtrace.rs
index a1abdeb8a..0b61fb425 100644
--- a/crates/runtime/src/traphandlers/backtrace.rs
+++ b/crates/runtime/src/traphandlers/backtrace.rs
@@ -20,7 +20,10 @@
 //! exit FP and stopping once we reach the entry SP (meaning that the next older
 //! frame is a host frame).
 
-use crate::traphandlers::{tls, CallThreadState};
+use crate::{
+    traphandlers::{tls, CallThreadState},
+    VMRuntimeLimits,
+};
 use cfg_if::cfg_if;
 use std::ops::ControlFlow;
 
@@ -80,9 +83,9 @@ impl Backtrace {
     }
 
     /// Capture the current Wasm stack in a backtrace.
-    pub fn new() -> Backtrace {
+    pub fn new(limits: *const VMRuntimeLimits) -> Backtrace {
         tls::with(|state| match state {
-            Some(state) => unsafe { Self::new_with_trap_state(state, None) },
+            Some(state) => unsafe { Self::new_with_trap_state(limits, state, None) },
             None => Backtrace(vec![]),
         })
     }
@@ -93,11 +96,12 @@ impl Backtrace {
     /// Wasm exit trampoline didn't run, and we use the provided PC and FP
     /// instead of looking them up in `VMRuntimeLimits`.
     pub(crate) unsafe fn new_with_trap_state(
+        limits: *const VMRuntimeLimits,
         state: &CallThreadState,
         trap_pc_and_fp: Option<(usize, usize)>,
     ) -> Backtrace {
         let mut frames = vec![];
-        Self::trace_with_trap_state(state, trap_pc_and_fp, |frame| {
+        Self::trace_with_trap_state(limits, state, trap_pc_and_fp, |frame| {
             frames.push(frame);
             ControlFlow::Continue(())
         });
@@ -105,9 +109,9 @@ impl Backtrace {
     }
 
     /// Walk the current Wasm stack, calling `f` for each frame we walk.
-    pub fn trace(f: impl FnMut(Frame) -> ControlFlow<()>) {
+    pub fn trace(limits: *const VMRuntimeLimits, f: impl FnMut(Frame) -> ControlFlow<()>) {
         tls::with(|state| match state {
-            Some(state) => unsafe { Self::trace_with_trap_state(state, None, f) },
+            Some(state) => unsafe { Self::trace_with_trap_state(limits, state, None, f) },
             None => {}
         });
     }
@@ -118,88 +122,63 @@ impl Backtrace {
     /// Wasm exit trampoline didn't run, and we use the provided PC and FP
     /// instead of looking them up in `VMRuntimeLimits`.
     pub(crate) unsafe fn trace_with_trap_state(
+        limits: *const VMRuntimeLimits,
         state: &CallThreadState,
         trap_pc_and_fp: Option<(usize, usize)>,
         mut f: impl FnMut(Frame) -> ControlFlow<()>,
     ) {
         log::trace!("====== Capturing Backtrace ======");
+
         let (last_wasm_exit_pc, last_wasm_exit_fp) = match trap_pc_and_fp {
             // If we exited Wasm by catching a trap, then the Wasm-to-host
             // trampoline did not get a chance to save the last Wasm PC and FP,
             // and we need to use the plumbed-through values instead.
-            Some((pc, fp)) => (pc, fp),
+            Some((pc, fp)) => {
+                assert!(std::ptr::eq(limits, state.limits));
+                (pc, fp)
+            }
             // Either there is no Wasm currently on the stack, or we exited Wasm
             // through the Wasm-to-host trampoline.
             None => {
-                let pc = *(*state.limits).last_wasm_exit_pc.get();
-                let fp = *(*state.limits).last_wasm_exit_fp.get();
-
-                if pc == 0 {
-                    // Host function calling another host function that
-                    // traps. No Wasm on the stack.
-                    assert_eq!(fp, 0);
-                    return;
-                }
-
+                let pc = *(*limits).last_wasm_exit_pc.get();
+                let fp = *(*limits).last_wasm_exit_fp.get();
                 (pc, fp)
             }
         };
 
-        // Trace through the first contiguous sequence of Wasm frames on the
-        // stack.
-        if let ControlFlow::Break(()) = Self::trace_through_wasm(
+        let activations = std::iter::once((
             last_wasm_exit_pc,
             last_wasm_exit_fp,
-            *(*state.limits).last_wasm_entry_sp.get(),
-            &mut f,
-        ) {
-            log::trace!("====== Done Capturing Backtrace ======");
-            return;
-        }
-
-        // And then trace through each of the older contiguous sequences of Wasm
-        // frames on the stack.
-        for state in state.iter() {
-            // If there is no previous call state, then there is nothing more to
-            // trace through (since each `CallTheadState` saves the *previous*
-            // call into Wasm's saved registers, and the youngest call into
-            // Wasm's registers are saved in the `VMRuntimeLimits`)
-            if state.prev().is_null() {
-                debug_assert_eq!(state.old_last_wasm_exit_pc(), 0);
-                debug_assert_eq!(state.old_last_wasm_exit_fp(), 0);
-                debug_assert_eq!(state.old_last_wasm_entry_sp(), 0);
-                log::trace!("====== Done Capturing Backtrace ======");
-                return;
-            }
-
-            // We save `CallThreadState` linked list entries for various kinds
-            // of {native,array} x {native,array} calls -- and we technically
-            // "shouldn't" because these calls can't enter Wasm -- because our
-            // Wasm call path unconditionally calls
-            // `wasmtime_runtime::catch_traps` even when the callee is not
-            // actually Wasm. We do this because the host-to-Wasm call path is
-            // very hot and these host-to-host calls that flow through that code
-            // path are very rare and also not hot. Anyways, these unnecessary
-            // `catch_traps` calls result in these null/empty `CallThreadState`
-            // entries. Recognize and ignore them.
-            if state.old_last_wasm_entry_sp() == 0 {
-                debug_assert_eq!(state.old_last_wasm_exit_fp(), 0);
-                debug_assert_eq!(state.old_last_wasm_exit_pc(), 0);
-                continue;
+            *(*limits).last_wasm_entry_sp.get(),
+        ))
+        .chain(
+            state
+                .iter()
+                .filter(|state| std::ptr::eq(limits, state.limits))
+                .map(|state| {
+                    (
+                        state.old_last_wasm_exit_pc(),
+                        state.old_last_wasm_exit_fp(),
+                        state.old_last_wasm_entry_sp(),
+                    )
+                }),
+        )
+        .take_while(|&(pc, fp, sp)| {
+            if pc == 0 {
+                debug_assert_eq!(fp, 0);
+                debug_assert_eq!(sp, 0);
             }
+            pc != 0
+        });
 
-            if let ControlFlow::Break(()) = Self::trace_through_wasm(
-                state.old_last_wasm_exit_pc(),
-                state.old_last_wasm_exit_fp(),
-                state.old_last_wasm_entry_sp(),
-                &mut f,
-            ) {
-                log::trace!("====== Done Capturing Backtrace ======");
+        for (pc, fp, sp) in activations {
+            if let ControlFlow::Break(()) = Self::trace_through_wasm(pc, fp, sp, &mut f) {
+                log::trace!("====== Done Capturing Backtrace (closure break) ======");
                 return;
             }
         }
 
-        unreachable!()
+        log::trace!("====== Done Capturing Backtrace (reached end of activations) ======");
     }
 
     /// Walk through a contiguous sequence of Wasm frames starting with the
diff --git a/crates/wasmtime/src/profiling.rs b/crates/wasmtime/src/profiling.rs
index b0620789a..9b59e584a 100644
--- a/crates/wasmtime/src/profiling.rs
+++ b/crates/wasmtime/src/profiling.rs
@@ -1,3 +1,4 @@
+use crate::{AsContext, Module};
 use anyhow::Result;
 use fxprof_processed_profile::debugid::DebugId;
 use fxprof_processed_profile::{
@@ -10,8 +11,6 @@ use std::time::{Duration, Instant};
 use wasmtime_jit::CompiledModule;
 use wasmtime_runtime::Backtrace;
 
-use crate::Module;
-
 // TODO: collect more data
 // - Provide additional hooks for recording host-guest transitions, to be
 //   invoked from a Store::call_hook
@@ -129,12 +128,12 @@ impl GuestProfiler {
     /// any stack frames for allowed modules on the current stack. It should
     /// typically be called from a callback registered using
     /// [`Store::epoch_deadline_callback()`](crate::Store::epoch_deadline_callback).
-    pub fn sample(&mut self) {
+    pub fn sample(&mut self, store: impl AsContext) {
         let now = Timestamp::from_nanos_since_reference(
             self.start.elapsed().as_nanos().try_into().unwrap(),
         );
 
-        let backtrace = Backtrace::new();
+        let backtrace = Backtrace::new(store.as_context().0.vmruntime_limits());
         let frames = backtrace
             .frames()
             // Samply needs to see the oldest frame first, but we list the newest
diff --git a/crates/wasmtime/src/store.rs b/crates/wasmtime/src/store.rs
index b36e9650c..240955aa1 100644
--- a/crates/wasmtime/src/store.rs
+++ b/crates/wasmtime/src/store.rs
@@ -1228,7 +1228,13 @@ impl StoreOpaque {
     pub fn gc(&mut self) {
         // For this crate's API, we ensure that `set_stack_canary` invariants
         // are upheld for all host-->Wasm calls.
-        unsafe { wasmtime_runtime::gc(&self.modules, &mut self.externref_activations_table) }
+        unsafe {
+            wasmtime_runtime::gc(
+                self.runtime_limits(),
+                &self.modules,
+                &mut self.externref_activations_table,
+            )
+        }
     }
 
     /// Yields the async context, assuming that we are executing on a fiber and
diff --git a/crates/wasmtime/src/trap.rs b/crates/wasmtime/src/trap.rs
index 80d367d8a..6b98ceafd 100644
--- a/crates/wasmtime/src/trap.rs
+++ b/crates/wasmtime/src/trap.rs
@@ -263,7 +263,11 @@ impl WasmBacktrace {
     /// always captures a backtrace.
     pub fn force_capture(store: impl AsContext) -> WasmBacktrace {
         let store = store.as_context();
-        Self::from_captured(store.0, wasmtime_runtime::Backtrace::new(), None)
+        Self::from_captured(
+            store.0,
+            wasmtime_runtime::Backtrace::new(store.0.runtime_limits()),
+            None,
+        )
     }
 
     fn from_captured(
diff --git a/src/commands/run.rs b/src/commands/run.rs
index c7a4ac483..4d01ce2ff 100644
--- a/src/commands/run.rs
+++ b/src/commands/run.rs
@@ -10,8 +10,8 @@ use std::path::{Component, Path, PathBuf};
 use std::thread;
 use std::time::Duration;
 use wasmtime::{
-    Engine, Func, GuestProfiler, Linker, Module, Store, StoreLimits, StoreLimitsBuilder, Val,
-    ValType,
+    AsContextMut, Engine, Func, GuestProfiler, Linker, Module, Store, StoreLimits,
+    StoreLimitsBuilder, Val, ValType,
 };
 use wasmtime_cli_flags::{CommonOptions, WasiModules};
 use wasmtime_wasi::maybe_exit_on_error;
@@ -403,13 +403,24 @@ impl RunCommand {
             store.data_mut().guest_profiler =
                 Some(Arc::new(GuestProfiler::new(module_name, interval, modules)));
 
+            fn sample(mut store: impl AsContextMut<Data = Host>) {
+                let mut profiler = store
+                    .as_context_mut()
+                    .data_mut()
+                    .guest_profiler
+                    .take()
+                    .unwrap();
+                Arc::get_mut(&mut profiler)
+                    .expect("profiling doesn't support threads yet")
+                    .sample(&store);
+                store.as_context_mut().data_mut().guest_profiler = Some(profiler);
+            }
+
             if let Some(timeout) = self.wasm_timeout {
                 let mut timeout = (timeout.as_secs_f64() / interval.as_secs_f64()).ceil() as u64;
                 assert!(timeout > 0);
                 store.epoch_deadline_callback(move |mut store| {
-                    Arc::get_mut(store.data_mut().guest_profiler.as_mut().unwrap())
-                        .expect("profiling doesn't support threads yet")
-                        .sample();
+                    sample(&mut store);
                     timeout -= 1;
                     if timeout == 0 {
                         bail!("timeout exceeded");
@@ -418,9 +429,7 @@ impl RunCommand {
                 });
             } else {
                 store.epoch_deadline_callback(move |mut store| {
-                    Arc::get_mut(store.data_mut().guest_profiler.as_mut().unwrap())
-                        .expect("profiling doesn't support threads yet")
-                        .sample();
+                    sample(&mut store);
                     Ok(1)
                 });
             }


Test patch:
diff --git a/tests/all/async_functions.rs b/tests/all/async_functions.rs
index fdb26f247..aec49b3fb 100644
--- a/tests/all/async_functions.rs
+++ b/tests/all/async_functions.rs
@@ -1,6 +1,7 @@
 use anyhow::{anyhow, bail, Result};
 use std::future::Future;
 use std::pin::Pin;
+use std::sync::{Arc, Mutex};
 use std::task::{Context, Poll, RawWaker, RawWakerVTable, Waker};
 use wasmtime::*;
 
@@ -706,3 +707,139 @@ fn noop_waker() -> Waker {
     const RAW: RawWaker = RawWaker::new(0 as *const (), &VTABLE);
     unsafe { Waker::from_raw(RAW) }
 }
+
+#[tokio::test]
+async fn non_stacky_async_activations() -> Result<()> {
+    let mut config = Config::new();
+    config.async_support(true);
+    let engine = Engine::new(&config)?;
+    let mut store1: Store<Option<Pin<Box<dyn Future<Output = Result<()>> + Send>>>> =
+        Store::new(&engine, None);
+    let mut linker1 = Linker::new(&engine);
+
+    let module1 = Module::new(
+        &engine,
+        r#"
+            (module $m1
+                (import "" "host_capture_stack" (func $host_capture_stack))
+                (import "" "start_async_instance" (func $start_async_instance))
+                (func $capture_stack (export "capture_stack")
+                    call $host_capture_stack
+                )
+                (func $run_sync (export "run_sync")
+                    call $start_async_instance
+                )
+            )
+        "#,
+    )?;
+
+    let module2 = Module::new(
+        &engine,
+        r#"
+            (module $m2
+                (import "" "yield" (func $yield))
+
+                (func $run_async (export "run_async")
+                    call $yield
+                )
+            )
+        "#,
+    )?;
+
+    let stacks = Arc::new(Mutex::new(vec![]));
+    fn capture_stack(stacks: &Arc<Mutex<Vec<WasmBacktrace>>>, store: impl AsContext) {
+        let mut stacks = stacks.lock().unwrap();
+        stacks.push(wasmtime::WasmBacktrace::force_capture(store));
+    }
+
+    linker1.func_wrap0_async("", "host_capture_stack", {
+        let stacks = stacks.clone();
+        move |caller| {
+            capture_stack(&stacks, &caller);
+            Box::new(async { Ok(()) })
+        }
+    })?;
+
+    linker1.func_wrap0_async("", "start_async_instance", {
+        let stacks = stacks.clone();
+        move |mut caller| {
+            let stacks = stacks.clone();
+            capture_stack(&stacks, &caller);
+
+            let module2 = module2.clone();
+            let mut store2 = Store::new(caller.engine(), ());
+            let mut linker2 = Linker::new(caller.engine());
+            linker2
+                .func_wrap0_async("", "yield", {
+                    let stacks = stacks.clone();
+                    move |caller| {
+                        let stacks = stacks.clone();
+                        Box::new(async move {
+                            capture_stack(&stacks, &caller);
+                            tokio::task::yield_now().await;
+                            capture_stack(&stacks, &caller);
+                            Ok(())
+                        })
+                    }
+                })
+                .unwrap();
+
+            Box::new(async move {
+                let future = PollOnce::new(Box::pin({
+                    let stacks = stacks.clone();
+                    async move {
+                        let instance2 = linker2.instantiate_async(&mut store2, &module2).await?;
+
+                        instance2
+                            .get_func(&mut store2, "run_async")
+                            .unwrap()
+                            .call_async(&mut store2, &[], &mut [])
+                            .await?;
+
+                        capture_stack(&stacks, &store2);
+                        Ok(())
+                    }
+                }) as _)
+                .await;
+                capture_stack(&stacks, &caller);
+                *caller.data_mut() = Some(future);
+                Ok(())
+            })
+        }
+    })?;
+
+    let instance1 = linker1.instantiate_async(&mut store1, &module1).await?;
+    instance1
+        .get_typed_func::<(), ()>(&mut store1, "run_sync")?
+        .call_async(&mut store1, ())
+        .await?;
+    let future = store1.data_mut().take().unwrap();
+    future.await?;
+
+    instance1
+        .get_typed_func::<(), ()>(&mut store1, "capture_stack")?
+        .call_async(&mut store1, ())
+        .await?;
+
+    let stacks = stacks.lock().unwrap();
+    eprintln!("stacks = {stacks:#?}");
+
+    assert_eq!(stacks.len(), 6);
+    for (actual, expected) in stacks.iter().zip(vec![
+        vec!["run_sync"],
+        vec!["run_async"],
+        vec!["run_sync"],
+        vec!["run_async"],
+        vec![],
+        vec!["capture_stack"],
+    ]) {
+        eprintln!("expected = {expected:?}");
+        eprintln!("actual = {actual:?}");
+        assert_eq!(actual.frames().len(), expected.len());
+        for (actual, expected) in actual.frames().iter().zip(expected) {
+            assert_eq!(actual.func_name(), Some(expected));
+        }
+    }
+
+    Ok(())
+}
diff --git a/tests/all/traps.rs b/tests/all/traps.rs
index 84d8fe587..a5f7463ca 100644
--- a/tests/all/traps.rs
+++ b/tests/all/traps.rs
@@ -1,6 +1,7 @@
 use anyhow::{bail, Error, Result};
 use std::panic::{self, AssertUnwindSafe};
 use std::process::Command;
+use std::sync::{Arc, Mutex};
 use wasmtime::*;
 
 #[test]
@@ -1468,3 +1469,175 @@ fn trap_with_native_to_wasm_stack_args() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn dont_see_stale_stack_walking_registers() -> Result<()> {
+    let engine = Engine::default();
+
+    let module = Module::new(
+        &engine,
+        r#"
+            (module
+                (import "" "host_start" (func $host_start))
+                (import "" "host_get_trap" (func $host_get_trap))
+                (export "get_trap" (func $host_get_trap))
+
+                ;; We enter and exit Wasm, which saves registers in the
+                ;; `VMRuntimeLimits`. Later, when we call a re-exported host
+                ;; function, we should not accidentally reuse those saved
+                ;; registers.
+                (start $start)
+                (func $start
+                    (call $host_start)
+                )
+            )
+        "#,
+    )?;
+
+    let mut store = Store::new(&engine, ());
+    let mut linker = Linker::new(&engine);
+
+    let host_start = Func::new(
+        &mut store,
+        FuncType::new([], []),
+        |_caller, _args, _results| Ok(()),
+    );
+    linker.define(&store, "", "host_start", host_start)?;
+
+    let host_get_trap = Func::new(
+        &mut store,
+        FuncType::new([], []),
+        |_caller, _args, _results| Err(anyhow::anyhow!("trap!!!")),
+    );
+    linker.define(&store, "", "host_get_trap", host_get_trap)?;
+
+    let instance = linker.instantiate(&mut store, &module)?;
+    let get_trap = instance.get_func(&mut store, "get_trap").unwrap();
+
+    let err = get_trap.call(&mut store, &[], &mut []).unwrap_err();
+    assert!(err.to_string().contains("trap!!!"));
+
+    Ok(())
+}
+
+#[test]
+fn same_module_multiple_stores() -> Result<()> {
+    let _ = env_logger::try_init();
+
+    let engine = Engine::default();
+
+    let module = Module::new(
+        &engine,
+        r#"
+            (module
+                (import "" "f" (func $f))
+                (import "" "call_ref" (func $call_ref (param funcref)))
+                (global $g (mut i32) (i32.const 0))
+                (func $a (export "a")
+                    call $b
+                )
+                (func $b
+                    call $c
+                )
+                (func $c
+                    global.get $g
+                    if
+                        call $f
+                    else
+                        i32.const 1
+                        global.set $g
+                        ref.func $a
+                        call $call_ref
+                    end
+                )
+            )
+        "#,
+    )?;
+
+    let stacks = Arc::new(Mutex::new(vec![]));
+
+    let mut store3 = Store::new(&engine, ());
+    let f3 = Func::new(&mut store3, FuncType::new([], []), {
+        let stacks = stacks.clone();
+        move |caller, _params, _results| {
+            stacks
+                .lock()
+                .unwrap()
+                .push(WasmBacktrace::force_capture(caller));
+            Ok(())
+        }
+    });
+    let call_ref3 = Func::wrap(&mut store3, |caller: Caller<'_, _>, f: Option<Func>| {
+        f.unwrap().call(caller, &[], &mut [])
+    });
+    let instance3 = Instance::new(&mut store3, &module, &[f3.into(), call_ref3.into()])?;
+
+    let mut store2 = Store::new(&engine, store3);
+    let f2 = Func::new(&mut store2, FuncType::new([], []), {
+        let stacks = stacks.clone();
+        move |mut caller, _params, _results| {
+            stacks
+                .lock()
+                .unwrap()
+                .push(WasmBacktrace::force_capture(&mut caller));
+            instance3
+                .get_typed_func::<(), ()>(caller.data_mut(), "a")
+                .unwrap()
+                .call(caller.data_mut(), ())
+                .unwrap();
+            Ok(())
+        }
+    });
+    let call_ref2 = Func::wrap(&mut store2, |caller: Caller<'_, _>, f: Option<Func>| {
+        f.unwrap().call(caller, &[], &mut [])
+    });
+    let instance2 = Instance::new(&mut store2, &module, &[f2.into(), call_ref2.into()])?;
+
+    let mut store1 = Store::new(&engine, store2);
+    let f1 = Func::new(&mut store1, FuncType::new([], []), {
+        let stacks = stacks.clone();
+        move |mut caller, _params, _results| {
+            stacks
+                .lock()
+                .unwrap()
+                .push(WasmBacktrace::force_capture(&mut caller));
+            instance2
+                .get_typed_func::<(), ()>(caller.data_mut(), "a")
+                .unwrap()
+                .call(caller.data_mut(), ())
+                .unwrap();
+            Ok(())
+        }
+    });
+    let call_ref1 = Func::wrap(&mut store1, |caller: Caller<'_, _>, f: Option<Func>| {
+        f.unwrap().call(caller, &[], &mut [])
+    });
+    let instance1 = Instance::new(&mut store1, &module, &[f1.into(), call_ref1.into()])?;
+
+    instance1
+        .get_typed_func(&mut store1, "a")?
+        .call(&mut store1, ())?;
+
+    let expected_stacks = vec![
+        // [f1, c1, b1, a1, call_ref1, c1, b1, a1]
+        vec!["c", "b", "a", "c", "b", "a"],
+        // [f2, c2, b2, a2, call_ref2, c2, b2, a2, f1, c1, b1, a1, call_ref1, c1, b1, a1]
+        vec!["c", "b", "a", "c", "b", "a"],
+        // [f3, c3, b3, a3, call_ref3, c3, b3, a3, f2, c2, b2, a2, call_ref2, c2, b2, a2, f1, c1, b1, a1, call_ref1, c1, b1, a1]
+        vec!["c", "b", "a", "c", "b", "a"],
+    ];
+    eprintln!("expected = {expected_stacks:#?}");
+    let actual_stacks = stacks.lock().unwrap();
+    eprintln!("actaul = {actual_stacks:#?}");
+
+    assert_eq!(actual_stacks.len(), expected_stacks.len());
+    for (expected_stack, actual_stack) in expected_stacks.into_iter().zip(actual_stacks.iter()) {
+        assert_eq!(expected_stack.len(), actual_stack.frames().len());
+        for (expected_frame, actual_frame) in expected_stack.into_iter().zip(actual_stack.frames())
+        {
+            assert_eq!(actual_frame.func_name(), Some(expected_frame));
+        }
+    }
+
+    Ok(())
+}