miri: add guest-debugging, including frame accesses.#12575
Merged
cfallin merged 5 commits intobytecodealliance:mainfrom Feb 13, 2026
Merged
miri: add guest-debugging, including frame accesses.#12575cfallin merged 5 commits intobytecodealliance:mainfrom
cfallin merged 5 commits intobytecodealliance:mainfrom
Conversation
cfallin
force-pushed
the
debug-frames-miri
branch
2 times, most recently
from
f0d35ce to
c43a590
Compare
The fix to `vm_store_context` provenance in `record_unwind`/`unwind` is a little weird to me. I was seeing mut access to the `vm_store_context` via `store.vm_store_context_mut()` in `record_unwind` (before this diff) then access via the previously saved raw pointer in the `CallThreadState`, which was registered as invalid and I believe is indeed invalid. This was only manifesting when setting `Config::guest_debug`, even without the frame-handle accesses added here. I didn't dig into the exact diff in codegen or runtime behavior that caused this but in any case, accessing `vm_store_context` via these two different paths (with one mut) appears to be unsound in any case. The fix here is to set the unwind state via the raw pointer in `CallThreadState` since that's the only path that the subsequent `unwind` has access to. Unrelated but useful: `ci/miri-provenance.test.sh` now accepts `MIRI_RUST_VERSION=+nightly` or whatnot, which is nice for running locally (I keep `stable` as my default toolchain).
cfallin
force-pushed
the
debug-frames-miri
branch
from
c43a590 to
4fb36d0
Compare
alexcrichton
approved these changes
Feb 12, 2026
Member
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Thanks! Do you think it'd be worth running through anything else in the debug API as well? Or does getting a module more-or-less exercise everything?
github-actions
bot
added
the
wasmtime:api
Member
Author
|
Getting the current module exercises almost everything (frame-walking, getting metadata, reading raw vmctx and converting back to proper borrow of instance, etc) but I added a few more reads for good measure. Thanks! |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Member
Author
|
@alexcrichton pinging for re-review here -- another miri test failure (not part of the Pulley provenance test) has pushed me back toward another approach that involves re-deriving the raw |
cfallin
force-pushed
the
debug-frames-miri
branch
from
6cb69d7 to
d27711a
Compare
…re-deriving the raw pointer in `CallThreadState`. prtest:full
cfallin
force-pushed
the
debug-frames-miri
branch
from
d27711a to
ad9103f
Compare
Member
Author
|
(There's still a failure on s390x only that I suspect is an endianness issue when running Pulley + guest-debug instrumentation -- will take a look) |
alexcrichton
approved these changes
Feb 12, 2026
Member
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
Looks reasonable to me yeah 👍
Member
Author
|
A little more detail: Pulley on s390x with guest-debug enabled (which hasn't been tested before -- only native s390x with guest-debug) is generating a big-endian vector store in the CLIF to compile to Pulley, which Cranelift's Pulley backend doesn't support. That's a nontrivial bit of implementation (lane swapping, etc). I'll instead refactor the instrumentation to work as e.g. object fields do, always little endian, in a separate PR. |
cfallin
added a commit
to cfallin/wasmtime
that referenced
this pull request
Feb 12, 2026
…reload vectors in guest-debugging slot and ABI clobbers.
When running Pulley on an s390x (or other big-endian) host, and enabling
guest-debugging instrumentationa very strange confluence of events
occurs:
- Pulley uses "native endian" of the host by default for loads and
stores.
- Patchable calls to debug hooks use the `preserve_all` ABI, which
spills all registers in the trampoline adapter (callee in this ABI),
including vector registers.
- Saving vector-typed locals/operand stack values to the debugger state
slot also uses vector stores.
- All of these stores were thus big-endian on big-endian hosts.
- Pulley's bytecode only supports little-endian vector loads/stores.
We were thus hitting an assert in Pulley codegen (the Cranelift
backend) when encountering a `VStore` VCode instruction with a
big-endian mode.
This PR makes two changes that avoid this issue:
- The ABI code for Pulley is careful to specify little-endian mode
explicitly for any vector load/store.
- The debug instrumentation code is refactored to use little-endian
explicitly for vector types *only*.
- (Why not for all types? Because we GC-root GC ref values, and
these need to be provided to the collector as mutable storage
cells, so need to be in native endianness.)
Test will come as part of bytecodealliance#12575 incorporating a
Pulley-with-guest-debugging test and running on s390x amongst our
platforms.
cfallin
added a commit
to cfallin/wasmtime
that referenced
this pull request
Feb 12, 2026
…reload vectors in guest-debugging slot and ABI clobbers.
When running Pulley on an s390x (or other big-endian) host, and enabling
guest-debugging instrumentation, a very strange confluence of events
occurs:
- Pulley uses "native endian" of the host by default for loads and
stores.
- Patchable calls to debug hooks use the `preserve_all` ABI, which
spills all registers in the trampoline adapter (callee in this ABI),
including vector registers.
- Saving vector-typed locals/operand stack values to the debugger state
slot also uses vector stores.
- All of these stores were thus big-endian on big-endian hosts.
- Pulley's bytecode only supports little-endian vector loads/stores.
We were thus hitting an assert in Pulley codegen (the Cranelift
backend) when encountering a `VStore` VCode instruction with a
big-endian mode.
This PR makes two changes that avoid this issue:
- The ABI code for Pulley is careful to specify little-endian mode
explicitly for any vector load/store.
- The debug instrumentation code is refactored to use little-endian
explicitly for vector types *only*.
- (Why not for all types? Because we GC-root GC ref values, and
these need to be provided to the collector as mutable storage
cells, so need to be in native endianness.)
Test will come as part of bytecodealliance#12575 incorporating a
Pulley-with-guest-debugging test and running on s390x amongst our
platforms.
prtest:full
Member
Author
|
s390x fix in #12585; will |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 12, 2026
…reload vectors in guest-debugging slot and ABI clobbers. (#12585) * Cranelift/Wasmtime/Pulley/Debugging: use little-endian mode to spill/reload vectors in guest-debugging slot and ABI clobbers. When running Pulley on an s390x (or other big-endian) host, and enabling guest-debugging instrumentation, a very strange confluence of events occurs: - Pulley uses "native endian" of the host by default for loads and stores. - Patchable calls to debug hooks use the `preserve_all` ABI, which spills all registers in the trampoline adapter (callee in this ABI), including vector registers. - Saving vector-typed locals/operand stack values to the debugger state slot also uses vector stores. - All of these stores were thus big-endian on big-endian hosts. - Pulley's bytecode only supports little-endian vector loads/stores. We were thus hitting an assert in Pulley codegen (the Cranelift backend) when encountering a `VStore` VCode instruction with a big-endian mode. This PR makes two changes that avoid this issue: - The ABI code for Pulley is careful to specify little-endian mode explicitly for any vector load/store. - The debug instrumentation code is refactored to use little-endian explicitly for vector types *only*. - (Why not for all types? Because we GC-root GC ref values, and these need to be provided to the collector as mutable storage cells, so need to be in native endianness.) Test will come as part of #12575 incorporating a Pulley-with-guest-debugging test and running on s390x amongst our platforms. prtest:full * Review feedback. * Re-bless Cranelift tests (explicit `little` flags on `preserve_all` tests).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The fix to
vm_store_contextprovenance inrecord_unwind/unwindis a little weird to me. I was seeing mut access to thevm_store_contextviastore.vm_store_context_mut()inrecord_unwind(before this diff) then access via the previously saved raw pointer in theCallThreadState, which was registered as invalid and I believe is indeed invalid. This was only manifesting when settingConfig::guest_debug, even without the frame-handle accesses added here. I didn't dig into the exact diff in codegen or runtime behavior that caused this but in any case, accessingvm_store_contextvia these two different paths (with one mut) appears to be unsound in any case. The fix here is to set the unwind state via the raw pointer inCallThreadStatesince that's the only path that the subsequentunwindhas access to.