Stream: git-wasmtime

Topic: wasmtime / PR #12566 Debugging: refactor stack frame curs...


view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 00:41):

cfallin opened PR #12566 from cfallin:a-new-framing-for-stack-frames to bytecodealliance:main:

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.)

<!--
Please make sure you include the following information:

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
-->

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 00:41):

cfallin requested fitzgen for a review on PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 00:41):

cfallin requested wasmtime-compiler-reviewers for a review on PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 00:41):

cfallin requested wasmtime-core-reviewers for a review on PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 01:03):

cfallin commented on PR #12566:

(I plan to add tests for various handle invalidation cases tomorrow -- sorry, missed including those here)

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 04:34):

github-actions[bot] added the label wasmtime:api on PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 16:07):

cfallin updated PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 19:41):

alexcrichton created PR review comment:

"hte"

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 19:41):

alexcrichton created PR review comment:

The impl Into<...> strategy may not be necessary here, can this be AsContextMut instead? This is also applicable to a lot of methods below too, basically anything that doesn't need to return a thing-with-'a can use AsContextMut instead of Into<...>

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 19:41):

alexcrichton created PR review comment:

Can you add // SAFETY: ... comments here and below?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 19:41):

alexcrichton created PR review comment:

Could this perhaps take just a pc instead of a full Frame to emphasize how it's a per-pc calculation and can't take anything into account like fp/sp/etc?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 19:41):

alexcrichton created PR review comment:

Can you expand the docs here with a # Panics section for the is_valid assertion?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 19:41):

alexcrichton created PR review comment:

Can you add a // SAFETY: ... comment here?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 19:41):

alexcrichton created PR review comment:

Can this, and all methods below, document # Panics with is_valid assertions?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 19:41):

alexcrichton submitted PR review:

In retrospect we probably should have had this earlier, but given all the unsafe here I think it'd be good to get Miri testing for this. Could you extend the pulley_provenance_test test in tests/all/pulley.rs (or something like that) to be able to exercise all this under Miri?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:21):

cfallin updated PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:42):

cfallin updated PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:42):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:42):

cfallin created PR review comment:

Fixed, thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:42):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:42):

cfallin created PR review comment:

Done in e1e24025927adb47b463159ab707951554bd6cd3.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:42):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:42):

cfallin created PR review comment:

Done in e1e24025927adb47b463159ab707951554bd6cd3.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:43):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:43):

cfallin created PR review comment:

Switched the API to return Result anywhere a handle is used that needs to be valid, instead, after discussion with @fitzgen; we'll need checks at some level anyway when we wrap this in a WIT and it makes more sense to produce the Err from the builtin check than do a double-check. Implemented in e1e24025927adb47b463159ab707951554bd6cd3.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:44):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:44):

cfallin created PR review comment:

Updated to return a Result instead (details below) in e1e24025927adb47b463159ab707951554bd6cd3.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:51):

cfallin updated PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:51):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:51):

cfallin created PR review comment:

Good idea -- done in dedd88b297.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:51):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 20:51):

cfallin created PR review comment:

Ah, yes, that's much cleaner -- possible everywhere but for fn module(...) -> Option<&'a Module>. Done in dedd88b297.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 21:05):

cfallin commented on PR #12566:

In retrospect we probably should have had this earlier, but given all the unsafe here I think it'd be good to get Miri testing for this. Could you extend the pulley_provenance_test test in tests/all/pulley.rs (or something like that) to be able to exercise all this under Miri?

I'm getting some UB errors in miri deep in the MmapVec code -- perhaps something to do with the private-code-copy support? In any case, I'd like to defer this miri testing to a followup if you don't mind as it's looking like it won't be trivial.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 21:38):

alexcrichton submitted PR review:

That sounds reasonable to me yeah, and thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 21:38):

alexcrichton added PR #12566 Debugging: refactor stack frame cursor into frame handle abstraction. to the merge queue

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:02):

github-merge-queue[bot] removed PR #12566 Debugging: refactor stack frame cursor into frame handle abstraction. from the merge queue

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:07):

cfallin updated PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:07):

cfallin has enabled auto merge for PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:13):

fitzgen submitted PR review:

(realizing I forgot to hit submit on my pending review comments)

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:13):

fitzgen created PR review comment:

Seems like this could be FrameCursor::new now?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:13):

fitzgen created PR review comment:

This seems like it should be an Activation::frame_cursor method, so that we don't need to make the activation's register state pub(crate) (which means we need to audit the whole crate to check the safety of passing their values into frame_cursor, making sure that nothing else is setting them to invalid values).

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:13):

fitzgen created PR review comment:

Can we avoid making these pub(crate)?

Because if frame_cursor/FrameCursor::new is going to continue to be unsafe (meaning that we are trusting its pc/fp/trampoline_fp arguments), then we should probably not make these pub(crate) to limit the scope of how much code needs to be trusted to maintain these values correctly.

Alternatively, we could also make frame_cursor/FrameCursor::new a safe function and have all the unsafe invariants bottleneck at FrameCursor::advance.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:15):

cfallin has disabled auto merge for PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:38):

cfallin updated PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:38):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:38):

cfallin created PR review comment:

Done in 40f26f8fa4.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:39):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:39):

cfallin created PR review comment:

Ah, yep, done in 40f26f8fa4 now that we have a method in Activation to use them directly.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:39):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:39):

cfallin created PR review comment:

Done in 40f26f8fa4.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:39):

cfallin has enabled auto merge for PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:40):

cfallin edited PR #12566:

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.)

Fixes #12486.

<!--
Please make sure you include the following information:

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
-->

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 22:41):

cfallin updated PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 23:01):

cfallin updated PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 23:14):

cfallin added PR #12566 Debugging: refactor stack frame cursor into frame handle abstraction. to the merge queue

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 23:28):

cfallin updated PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 23:29):

cfallin removed PR #12566 Debugging: refactor stack frame cursor into frame handle abstraction. from the merge queue

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 23:29):

cfallin has enabled auto merge for PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 11 2026 at 23:43):

cfallin added PR #12566 Debugging: refactor stack frame cursor into frame handle abstraction. to the merge queue

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 00:07):

cfallin merged PR #12566.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2026 at 00:07):

cfallin removed PR #12566 Debugging: refactor stack frame cursor into frame handle abstraction. from the merge queue


Last updated: Feb 24 2026 at 04:36 UTC