Stream: git-wasmtime

Topic: wasmtime / PR #14235 aarch64: fix apple_aarch64 stack slo...

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2026 at 14:22):

dsecurity49 opened PR #14235 from dsecurity49:fix-apple-aarch64-stack-slot-overflow to bytecodealliance:main:

Problem

Fixes #13973

When a stack-spilled argument has a sext or uext extension under the apple_aarch64 calling convention, get_ext_mode returns the extension, which causes:

However, the slot-size condition used an || that allowed sub-word slots (e.g. 2 bytes for i16 sext) whenever args_or_rets == Args, regardless of whether an extension was present. This meant the caller wrote 8 bytes into a 2-byte slot, silently corrupting the 6 adjacent stack bytes.

Reproduction

The included repro_i16.clif demonstrates the issue. Before this fix:

FAIL ./repro_i16.clif: run
Failed test: run: %caller() == 1234605616436508552, actual: 1234605619298697215

After this fix:

1 tests   <- pass

Fix

Remove the args_or_rets == ArgsOrRets::Args branch from the condition. Sub-word slots are only safe when param.extension == ArgumentExtension::None, because only then will the caller store exactly size bytes and the callee load exactly size bytes.

-let size = if (is_apple_cc || is_winch_return)
-    && (args_or_rets == ArgsOrRets::Args
-        || param.extension == ir::ArgumentExtension::None)
+let size = if (is_apple_cc || is_winch_return)
+    && param.extension == ir::ArgumentExtension::None

Tested natively on AArch64 Linux (Android/Termux, Qualcomm TRINKET).

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2026 at 14:22):

dsecurity49 requested cfallin for a review on PR #14235.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2026 at 14:22):

dsecurity49 requested wasmtime-compiler-reviewers for a review on PR #14235.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2026 at 14:22):

dsecurity49 requested alexcrichton for a review on PR #14235.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2026 at 14:22):

dsecurity49 requested wasmtime-default-reviewers for a review on PR #14235.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2026 at 17:44):

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 30 2026 at 17:44):

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2026 at 12:14):

dsecurity49 edited PR #14235:

Fixes #13973

For apple_aarch64, get_ext_mode returns specified extensions (sext/uext), which causes caller/callee to load and store full 64-bit words for stack arguments. The slot size condition previously allowed sub-word sizes for spilled arguments whenever args_or_rets == Args, causing 8-byte stores to overwrite adjacent stack slots.

This updates the check to only allow sub-word stack slots when param.extension == None.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 31 2026 at 20:58):

cfallin commented on PR #14235:

If I understand this right, you are asserting that the Apple aarch64 convention actually does 8-align args just because they are extended to 64 bits (in registers). That doesn't sound right to me but I'm happy to be corrected; could you link the documentation you're using as a reference for this change? Thanks!

(If the extension only applies to in-register values instead, as I suspect, then the correct fix is instead to do narrow (true-width) loads/stores when values live on the stack, I think.)

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

dsecurity49 commented on PR #14235:

You are absolutely correct. The Apple ABI dictates that stack arguments smaller than 8 bytes occupy only their natural size (e.g., 2 bytes for an i16). The extension to 64 bits only applies to arguments passed in registers.

Because get_ext_mode in cranelift/codegen/src/isa/aarch64/abi.rs currently returns specified for AppleAarch64 regardless of the argument's location (register vs. stack), the generic code in machinst/abi.rs forces a 64-bit load/store for stack slots when an extension is specified.

My previous fix just forced the stack slot to be 8 bytes to absorb the 8-byte store generated by machinst/abi.rs. But as you pointed out, the true fix is to do a narrow (true-width) load/store when the value lives on the stack.

Since M::get_ext_mode(call_conv, extension) doesn't currently receive context about whether the argument is a Reg or Stack slot, what is the preferred way to handle this in Cranelift?

  1. Should we pass a location flag to get_ext_mode so it can return ArgumentExtension::None for stack slots on AppleAarch64?
  2. Or should the generic code in machinst/abi.rs just ignore ext when loading/storing from a Stack slot on little-endian platforms (though the comment at line 1591 mentions it is needed for big-endian)?

Happy to update the PR with whichever architectural approach you prefer!

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

dsecurity49 deleted a comment on PR #14235:

You are absolutely correct. The Apple ABI dictates that stack arguments smaller than 8 bytes occupy only their natural size (e.g., 2 bytes for an i16). The extension to 64 bits only applies to arguments passed in registers.

Because get_ext_mode in cranelift/codegen/src/isa/aarch64/abi.rs currently returns specified for AppleAarch64 regardless of the argument's location (register vs. stack), the generic code in machinst/abi.rs forces a 64-bit load/store for stack slots when an extension is specified.

My previous fix just forced the stack slot to be 8 bytes to absorb the 8-byte store generated by machinst/abi.rs. But as you pointed out, the true fix is to do a narrow (true-width) load/store when the value lives on the stack.

Since M::get_ext_mode(call_conv, extension) doesn't currently receive context about whether the argument is a Reg or Stack slot, what is the preferred way to handle this in Cranelift?

  1. Should we pass a location flag to get_ext_mode so it can return ArgumentExtension::None for stack slots on AppleAarch64?
  2. Or should the generic code in machinst/abi.rs just ignore ext when loading/storing from a Stack slot on little-endian platforms (though the comment at line 1591 mentions it is needed for big-endian)?

Happy to update the PR with whichever architectural approach you prefer!

view this post on Zulip Wasmtime GitHub notifications bot (Sep 01 2026 at 06:00):

dsecurity49 requested wasmtime-compiler-s390x-reviewers for a review on PR #14235.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 01 2026 at 06:00):

dsecurity49 updated PR #14235.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 01 2026 at 06:00):

dsecurity49 updated PR #14235.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 01 2026 at 06:06):

dsecurity49 updated PR #14235.

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

dsecurity49 commented on PR #14235:

you're right. The extension should only apply to registers. get_ext_mode didn't know the location though, so it was forcing 8-byte loads/stores for anything extended.

I've updated the PR to thread an ABIArgLocation into get_ext_mode so we can correctly return None for stack args on apple AArch64. I also added a precise-output test for it.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 01 2026 at 09:46):

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

view this post on Zulip Wasmtime GitHub notifications bot (Sep 01 2026 at 09:46):

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

view this post on Zulip Wasmtime GitHub notifications bot (Sep 01 2026 at 16:25):

alexcrichton unassigned alexcrichton from PR #14235 aarch64: fix apple_aarch64 stack slot size for sext/uext spilled args.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2026 at 03:54):

cfallin commented on PR #14235:

Thanks! I'll repeat a question from earlier:

could you link the documentation you're using as a reference for this change? Thanks!

Both for my own use and ideally linked in a comment so we can have a canonical source going forward.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2026 at 05:41):

dsecurity49 updated PR #14235.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2026 at 05:55):

dsecurity49 commented on PR #14235:

Source: https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2026 at 14:59):

:thumbs_up: cfallin submitted PR review:

Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2026 at 14:59):

cfallin added PR #14235 aarch64: fix apple_aarch64 stack slot size for sext/uext spilled args to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2026 at 15:26):

:check: cfallin merged PR #14235.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 03 2026 at 15:26):

cfallin removed PR #14235 aarch64: fix apple_aarch64 stack slot size for sext/uext spilled args from the merge queue.

Last updated: Sep 20 2026 at 18:08 UTC