cfallin opened PR #14648 from cfallin:fix-preserve-all to bytecodealliance:main:
The
preserve_allABI specifies that a function may not clobber any registers (all registers are callee-saved).However, Cranelift has two features that cause code to be inserted into the prologue that may sometimes clobber registers: stack-limit checks, and stack probes. Both of these happen before clobbered registers are saved, so normally use caller-saved (volatile) registers. In
preserve_all, no such registers exist. We previously used volatiles consistent with SysV/tail, erroneously assuming they would be volatile in all ABIs.(In Wasmtime,
preserve_allis needed for the guest-debug breakpoint trampoline to which calls are patched in from sequence-point NOP areas. We don't spill registers around these areas, so all registers really must be preserved.)This PR fixes the interaction two ways:
- Stack limits are simply disallowed in
preserve_allfunctions. (This is compatible with Wasmtime's trampoline, the onepreserve_alluse-case we have in-tree.)- Stack probes are only supported with an "unrolled" strategy, where we decrement RSP and store to it in one-page-sized steps. This strategy does not use any registers (other than RSP) so is still safe in this context.
(Note that practically speaking, this has no current impact on Wasmtime because, in that one use-case above, the trampoline has effectively no stack frame so does not emit any stack probes. But the theoretical problem still exists and should be handled or rejected.)
<!--
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.htmlPlease review the Bytecode Alliance's AI tool usage policy at
https://github.com/bytecodealliance/governance/blob/main/AI_TOOL_POLICY.mdPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
cfallin requested fitzgen for a review on PR #14648.
cfallin requested wasmtime-compiler-reviewers for a review on PR #14648.
cfallin updated PR #14648.
cfallin edited PR #14648:
The
preserve_allABI specifies that a function may not clobber any registers (all registers are callee-saved).However, Cranelift has two features that cause code to be inserted into the prologue that may sometimes clobber registers: stack-limit checks, and stack probes. Both of these happen before clobbered registers are saved, so normally use caller-saved (volatile) registers. In
preserve_all, no such registers exist. We previously used volatiles consistent with SysV/tail, erroneously assuming they would be volatile in all ABIs.(In Wasmtime,
preserve_allis needed for the guest-debug breakpoint trampoline to which calls are patched in from sequence-point NOP areas. We don't spill registers around these areas, so all registers really must be preserved.)This PR fixes the interaction two ways:
- Stack limits are simply disallowed in
preserve_allfunctions. (This is compatible with Wasmtime's trampoline, the onepreserve_alluse-case we have in-tree.)- Stack probes are only supported with an "unrolled" strategy, where we decrement RSP and store to it in one-page-sized steps. This strategy does not use any registers (other than RSP) so is still safe in this context.
(Note that practically speaking, this has no current impact on Wasmtime because, in that one use-case above, the trampoline has effectively no stack frame so does not emit any stack probes. But the theoretical problem still exists and should be handled or rejected.)
This PR also fixes something else found while looking at
preserve_all: on Pulley, we need 16 bytes, not 8, to save a 128-bit vector register (!). That is a correctness bug reachable from Wasmtime, but only when doing guest debugging on Pulley, neither of which is tier-1.<!--
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.htmlPlease review the Bytecode Alliance's AI tool usage policy at
https://github.com/bytecodealliance/governance/blob/main/AI_TOOL_POLICY.mdPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
:thumbs_up: alexcrichton submitted PR review.
alexcrichton has enabled auto merge for PR #14648.
alexcrichton added PR #14648 Cranelift: fix the interaction of preserve_all and stack probes+limits. to the merge queue
github-actions[bot] added the label cranelift on PR #14648.
github-actions[bot] added the label cranelift:area:x64 on PR #14648.
:check: alexcrichton merged PR #14648.
alexcrichton removed PR #14648 Cranelift: fix the interaction of preserve_all and stack probes+limits. from the merge queue
Last updated: Oct 11 2026 at 04:10 UTC