Stream: git-wasmtime

Topic: wasmtime / PR #13783 x64: report an error instead of pani...

Wasmtime GitHub notifications bot (Jul 01 2026 at 02:13):

soumik15630m requested wasmtime-compiler-s390x-reviewers for a review on PR #13783.

Wasmtime GitHub notifications bot (Jul 01 2026 at 02:13):

soumik15630m opened PR #13783 from soumik15630m:fix-x64-frame-size-overflow to bytecodealliance:main:

Summary

x64 codegen panics with TryFromIntError when a function's stack frame
is large enough that offsets relative to it no longer fit in a 32-bit
immediate. This affects three places: the inline stack-probe loop
(guard_size * probe_count), clobber save/restore (RSP-relative
offsets), and incoming-argument access (RBP-relative offsets via
SyntheticAmode::IncomingArg::finalize).

This was reported downstream in
rust-lang/rustc_codegen_cranelift#1656, where a ~2GB stack allocation
([0; 536870787_usize], a [i32; N] array) triggers an ICE:
thread 'optimize module ...' panicked at cranelift/codegen/src/isa/x64/inst/emit.rs:373:18:
guard_size * probe_count is too large to fit in a 32-bit immediate: TryFromIntError(PosOverflow)

What this does

Adds ABIMachineSpec::validate_frame_layout, a new trait method with a
no-op default, overridden for x64. It's called from VCode::emit right
after compute_frame_layout, before any offset encoding takes place,
and checks:

On overflow, this returns CodegenError::Unsupported instead of
panicking. Only x64 overrides the new trait method; other backends are
unaffected (aarch64/riscv64/s390x/pulley needed a one-line ?
each to propagate emit's now-fallible return type, no behavior
change).

Testing

• Added unit tests in isa/x64/abi.rs covering: a normal frame
  (accepted), an oversized fixed_frame_storage_size (rejected), an
  oversized tail_args_size via the RBP-relative path (rejected), and
  a frame that's under i32::MAX on its own but pushed over by the
  guard-page rounding margin (rejected) — this last case specifically
  reproduces the original bug.

Known follow-up (not included here)

bjorn3's comment mentions "all stack load/store instruction
lowerings" more broadly. I traced and covered the two offset-encoding
paths that are actually reachable from the repro (gen_clobber_save/
gen_clobber_restore and SyntheticAmode::IncomingArg::finalize), but
I have not exhaustively audited every stack-relative offset computation
in the x64 backend for the same class of narrowing. Happy to widen this
if there's a specific path you'd like checked.

Separately, rustc_codegen_cranelift currently panics when it receives
any Err from Context::compile at
src/base.rs:224
rather than converting it to a rustc diagnostic. I plan to open a
follow-up PR there to handle CodegenError::Unsupported the same way
the existing CodegenError::Verifier arm already does.

Wasmtime GitHub notifications bot (Jul 01 2026 at 02:13):

soumik15630m requested alexcrichton for a review on PR #13783.

Wasmtime GitHub notifications bot (Jul 01 2026 at 02:13):

soumik15630m requested wasmtime-compiler-reviewers for a review on PR #13783.

Wasmtime GitHub notifications bot (Jul 01 2026 at 02:25):

soumik15630m edited PR #13783:

Summary

x64 codegen panics with TryFromIntError when a function's stack frame
is large enough that offsets relative to it no longer fit in a 32-bit
immediate. This affects three places: the inline stack-probe loop
(guard_size * probe_count), clobber save/restore (RSP-relative
offsets), and incoming-argument access (RBP-relative offsets via
SyntheticAmode::IncomingArg::finalize).

This was reported downstream in
rust-lang/rustc_codegen_cranelift#1656, where a ~2GB stack allocation
([0; 536870787_usize], a [i32; N] array) triggers an ICE:
thread 'optimize module ...' panicked at cranelift/codegen/src/isa/x64/inst/emit.rs:373:18:
guard_size * probe_count is too large to fit in a 32-bit immediate: TryFromIntError(PosOverflow)

What this does

Adds ABIMachineSpec::validate_frame_layout, a new trait method with a
no-op default, overridden for x64. It's called from VCode::emit right
after compute_frame_layout, before any offset encoding takes place,
and checks:

On overflow, this returns CodegenError::Unsupported instead of
panicking. Only x64 overrides the new trait method; other backends are
unaffected (aarch64/riscv64/s390x/pulley needed a one-line ?
each to propagate emit's now-fallible return type, no behavior
change).

Testing

• Added unit tests in isa/x64/abi.rs covering: a normal frame
  (accepted), an oversized fixed_frame_storage_size (rejected), an
  oversized tail_args_size via the RBP-relative path (rejected), and
  a frame that's under i32::MAX on its own but pushed over by the
  guard-page rounding margin (rejected) — this last case specifically
  reproduces the original bug.

Known follow-up (not included here)

bjorn3's comment mentions "all stack load/store instruction
lowerings" more broadly. I traced and covered the two offset-encoding
paths that are actually reachable from the repro (gen_clobber_save/
gen_clobber_restore and SyntheticAmode::IncomingArg::finalize), but
I have not exhaustively audited every stack-relative offset computation
in the x64 backend for the same class of narrowing. Happy to widen this
if there's a specific path you'd like checked.

Separately, rustc_codegen_cranelift currently panics when it receives
any Err from Context::compile at
src/base.rs:224
rather than converting it to a rustc diagnostic. I have open a PR rust-lang/rustc_codegen_cranelift/pull/1669 there to handle CodegenError::Unsupported the same way
the existing CodegenError::Verifier arm already does.

Wasmtime GitHub notifications bot (Jul 01 2026 at 04:03):

github-actions[bot] added the label cranelift on PR #13783.

Wasmtime GitHub notifications bot (Jul 01 2026 at 04:03):

github-actions[bot] added the label cranelift:area:machinst on PR #13783.

Wasmtime GitHub notifications bot (Jul 01 2026 at 04:03):

github-actions[bot] added the label cranelift:area:x64 on PR #13783.

Wasmtime GitHub notifications bot (Jul 01 2026 at 04:03):

github-actions[bot] added the label cranelift:area:aarch64 on PR #13783.

Wasmtime GitHub notifications bot (Jul 01 2026 at 06:09):

:memo: bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Jul 01 2026 at 06:09):

:speech_balloon: bjorn3 created PR review comment:

Should probably use ImplLimitExceeded instead.

Wasmtime GitHub notifications bot (Jul 01 2026 at 08:31):

:memo: bjorn3 submitted PR review.

Wasmtime GitHub notifications bot (Jul 01 2026 at 08:31):

:speech_balloon: bjorn3 created PR review comment:

This validation should probably be done in generic code (maybe the caller of compute_frame_layout?). The FrameLayout type uses 32bit ints, so the 4GB stack frame size applies to all targets.

Wasmtime GitHub notifications bot (Jul 01 2026 at 12:02):

:memo: soumik15630m submitted PR review.

Wasmtime GitHub notifications bot (Jul 01 2026 at 12:02):

:speech_balloon: soumik15630m created PR review comment:

Good call, thanks. ImplLimitExceeded doesn't carry a message though ...so is it okay?Happy to switch either way; want to make sure I'm not throwing away information you'd want to keep.

Wasmtime GitHub notifications bot (Jul 01 2026 at 12:03):

:memo: soumik15630m submitted PR review.

Wasmtime GitHub notifications bot (Jul 01 2026 at 12:03):

:speech_balloon: soumik15630m created PR review comment:

Sure ... will push a commit soon

Wasmtime GitHub notifications bot (Jul 01 2026 at 12:24):

soumik15630m updated PR #13783.

Wasmtime GitHub notifications bot (Jul 01 2026 at 12:28):

:memo: soumik15630m submitted PR review.

Wasmtime GitHub notifications bot (Jul 01 2026 at 12:28):

:speech_balloon: soumik15630m created PR review comment:

Pushed a commit adding a generic check in compute_frame_layout.

I kept the x64-specific validate_frame_layout check as well, since it's
catching a different, tighter condition: x64 encodes these offsets as
signed 32-bit immediates (i32::MAX, not u32::MAX), and the inline
probestack loop rounds the frame size up to the next guard-page boundary
before encoding it - so a frame just under i32::MAX can get pushed
over it by that rounding. The original repro (~2.147GB) is under
u32::MAX (~4.295GB), so the generic check alone doesn't catch it -
only the x64-specific one does.

Happy to remove the x64-specific check if you'd rather keep this simpler
and generic-only, but wanted to flag that it currently exists to catch
a real case the generic check can't - unless other backends share x64's
signed-immediate constraint, in which case maybe it'd make sense to
generalize the tighter check too rather than keep it x64-only. Let me
know which direction you'd prefer.

Wasmtime GitHub notifications bot (Jul 01 2026 at 12:31):

soumik15630m updated PR #13783.

Wasmtime GitHub notifications bot (Jul 01 2026 at 12:31):

soumik15630m requested bjorn3 for a review on PR #13783.

Wasmtime GitHub notifications bot (Jul 01 2026 at 18:55):

alexcrichton commented on PR #13783:

Thanks for the PR, but personally I'm pretty wary of adding a parallel path that calculates information about a frame that duplicates what's already in each backend. Frame handling/layout is pretty precise and requires extremely careful handling, and I wouldn't be confident that it could be accurately mirrored elsewhere in the system.

What I think should happen here is either (a) to propagate a result upwards at the time of a try_into or a checked_add happens (e.g. the try_into in this case, or (b) the case in question is updated. Given the line number it looks like this is failing to generate a stack probe, but we could enhance that logic to better support the stack probe loop with larger frame sizes.

Wasmtime GitHub notifications bot (Jul 02 2026 at 00:52):

soumik15630m commented on PR #13783:

@alexcrichton
Dug into this in detail. There are actually two different kinds of
32-bit narrowing happening here, and I think they need different
treatment:

1. RSP arithmetic (the subq_mi/addq_mi adjustments in the probe
loop and clobber save/restore) - this is genuinely fixable by widening.
r11/r15 are already reserved as scratch registers in this ABI, and
movabsq_oi already exists to materialize a 64-bit value into one. So
for these sites, we could fall back to "materialize into a scratch reg,
then register-to-register add/sub" when the value doesn't fit in i32.

2. Stack-relative memory addressing (Amode::imm_reg(offset, rsp),
used for touching probe pages, clobber load/store, incoming-arg access)

• this hits an actual x86-64 ISA limit, not a Cranelift shortcut: the
  addressing mode's displacement field is a hard 32-bit signed value at
  the instruction-encoding level. The fix is computing the absolute
  effective address into a register first, then addressing off that with
  a zero displacement.

One thing worth flagging on option (a): MachInstEmit::emit returns
() for all 5 backends, with no error path anywhere in MachBuffer.
So propagating a Result from the actual try_from/.expect() sites
in emit.rs would mean changing that trait's signature - which is
called once per instruction, everywhere, across every backend. That's
a much bigger surface than I first thought, and probably bigger than
(b), which stays contained to x64 frame setup/teardown.

Given that, (b) seems like the more proportionate direction, and I'm
happy to take it on. Let me know if that matches your thinking or if
I'm missing a narrower way to do (a).

Wasmtime GitHub notifications bot (Jul 02 2026 at 00:54):

soumik15630m edited a comment on PR #13783:

@alexcrichton
Thanks for the detailed feedback.
Dug into this in detail. There are actually two different kinds of
32-bit narrowing happening here, and I think they need different
treatment:

1. RSP arithmetic (the subq_mi/addq_mi adjustments in the probe
loop and clobber save/restore) - this is genuinely fixable by widening.
r11/r15 are already reserved as scratch registers in this ABI, and
movabsq_oi already exists to materialize a 64-bit value into one. So
for these sites, we could fall back to "materialize into a scratch reg,
then register-to-register add/sub" when the value doesn't fit in i32.

2. Stack-relative memory addressing (Amode::imm_reg(offset, rsp),
used for touching probe pages, clobber load/store, incoming-arg access)

• this hits an actual x86-64 ISA limit, not a Cranelift shortcut: the
  addressing mode's displacement field is a hard 32-bit signed value at
  the instruction-encoding level. The fix is computing the absolute
  effective address into a register first, then addressing off that with
  a zero displacement.

One thing worth flagging on option (a): MachInstEmit::emit returns
() for all 5 backends, with no error path anywhere in MachBuffer.
So propagating a Result from the actual try_from/.expect() sites
in emit.rs would mean changing that trait's signature - which is
called once per instruction, everywhere, across every backend. That's
a much bigger surface than I first thought, and probably bigger than
(b), which stays contained to x64 frame setup/teardown.

Given that, (b) seems like the more proportionate direction, and I'm
happy to take it on. Let me know if that matches your thinking or if
I'm missing a narrower way to do (a).

Wasmtime GitHub notifications bot (Jul 02 2026 at 19:41):

alexcrichton commented on PR #13783:

I'd like to confirm that you're aware of the Bytecode Alliance's AI tool usage policy and would ask that if you haven't seen that that you read over it.

My main concern here is still that I would want to make sure that there's not duplication in calculating or interpreting frame layout. I believe there should be a "mostly central" location that each backend handles all this and ideally a limit, if any, would fall out of that. It's true that returning an error during emission would require changing signatures and it would also affect other situations such as stack offsets.

I'll bring this up at next week's Cranelift meeting because I'm not sure how best to handle this. One option would be to just say "all backends must support up to N byte stack frames" and return a hard error for anything over N for all backends. That would notably be a Cranelift/cross-platform limitation rather than just the x64 backend. Alternatively if N is high enough (e.g. 4GiB) then it would require changes to the x64 backend (and possibly others). I think it'd be good to loop in others on this discussion though.

Wasmtime GitHub notifications bot (Jul 02 2026 at 19:50):

soumik15630m commented on PR #13783:

Sure I will go through the ai tool usuage policy ...

and yeah this issue needs a deeper discussion cause to me both the approach looks equally reasonable ..so i'm happy to wait for that discussion, or start on whichever direction comes out of it. Let me know how I can help move it forward.

Wasmtime GitHub notifications bot (Jul 02 2026 at 19:56):

cfallin commented on PR #13783:

(Ghost-of-cfallin popping in from PTO briefly to offer thoughts as original-person-who-inflicted-this-ABI-code-upon-us)

I think that while this is a real issue that should indeed be surfaced into ImplLimitExceeded, I agree with Alex that there is too much backend-specific logic here. Actually the whole thing seems overly complex. I'd prefer a maximum_frame_size trait method per backend, then validate that once in the platform-independent code. And we should set it to something reasonable like 1GiB rather than try to reason carefully about 2GiB-minus-epsilon and squeeze every little bit out of the layout; the latter is error-prone and brittle.

Wasmtime GitHub notifications bot (Jul 03 2026 at 16:07):

soumik15630m commented on PR #13783:

Sure , this idea looks much practical ...keeping the best of both worlds

I will wait though for the next internal discussion - before pushing another commit with this method
fixing the size needs the discussion right ?..but yeah 1gb seems reasonable

Thanks...

Wasmtime GitHub notifications bot (Jul 06 2026 at 03:01):

alexcrichton commented on PR #13783:

FWIW I've added this to Cranelift's next meeting agenda, this coming Wednesday, although I suspect the consensus there will be a 1G max across backends is also fine. If you want to have that ready to go I can review/approve afterwards

Wasmtime GitHub notifications bot (Jul 07 2026 at 18:55):

soumik15630m commented on PR #13783:

Sure , will push a commit .....

Wasmtime GitHub notifications bot (Jul 10 2026 at 10:06):

soumik15630m updated PR #13783.

Wasmtime GitHub notifications bot (Jul 10 2026 at 19:30):

:thumbs_up: alexcrichton submitted PR review:

Thanks! I suspect this won't be too controversial so I'm going to go ahead and land this, and when we discuss this in a meeting if there's any follow-ups I'm happy to handle them

Wasmtime GitHub notifications bot (Jul 10 2026 at 19:30):

alexcrichton added PR #13783 x64: report an error instead of panicking on stack frames larger than i32::MAX to the merge queue.

Wasmtime GitHub notifications bot (Jul 10 2026 at 19:56):

:check: alexcrichton merged PR #13783.

Wasmtime GitHub notifications bot (Jul 10 2026 at 19:56):

alexcrichton removed PR #13783 x64: report an error instead of panicking on stack frames larger than i32::MAX from the merge queue.

Wasmtime GitHub notifications bot (Jul 11 2026 at 17:09):

soumik15630m commented on PR #13783:

Sure ...Thanks!!

Last updated: Jul 29 2026 at 05:03 UTC