cfallin requested wasmtime-compiler-reviewers for a review on PR #12842.
cfallin opened PR #12842 from cfallin:machbuffer-cjump to bytecodealliance:main:
In #12811 it was reported that riscv64 compressed jumps (
c.jinstructions), with a +/- 2048-byte range, could cause panics when combined with queued-up/deferred constants in a constant pool during binary emission.Our
MachBufferhandles single-pass machine code emission, resolution of labels, and upgrading of label ranges via "veneers" (jumps that a shorter jump can reach that themselves have a longer range). We track a pending "deadline" of all unresolved branches, and when the deadline is too close (including the max size of all veneers yet to be emitted), we emit an "island" of all veneers to resolve the deadline.After its initial design, we added support for deferred traps and constants to the
MachBuffer. These worked by emitting their contents before the "island" of veneers, which turns out to be slightly nicer for code layout in some cases.Unfortunately the full implications of those additions weren't realized against the invariants of the deadline-resolution algorithm. In particular, when a new branch is added with a very short range (e.g.,
c.j), it is possible that there are already too many queued-up traps/constants for the range of that just-emitted branch to reach even the first possible veneer site if we start an island right away.Thus it is strictly necessary to emit the veneers before constants/traps. Unfortunately this requires some alterations to other aspects of label resolution as well: in particular, we can't resolve fixups for label references to constants before we emit those constants, and likewise for traps. Note that we do a fixpoint loop over emitting island(s) at the end of emission, so all constants/traps will be emitted and label references to them will be resolved eventually; just in the opposite order, now.
No compile test because the particular reduced testcase in #12811 only worked in the
release-36.0.0branch, and not onmain, and it was too hard to tweak the test to hit the right case onmainas well. In lieu of that, I've added a unit test directly to theMachBufferimplementation to exercise this case.Fixes #12811.
<!--
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 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 #12842.
github-actions[bot] added the label cranelift on PR #12842.
github-actions[bot] added the label cranelift:area:machinst on PR #12842.
fitzgen submitted PR review.
cfallin updated PR #12842.
cfallin has enabled auto merge for PR #12842.
cfallin added PR #12842 Cranelift: rework MachBuffer to handle very short-deadline jumps. to the merge queue
cfallin merged PR #12842.
cfallin removed PR #12842 Cranelift: rework MachBuffer to handle very short-deadline jumps. from the merge queue
Last updated: Apr 12 2026 at 23:10 UTC