kit

kit
git clone https://git.ryansepassi.com/git/kit.git
Log | Files | Refs | README

commit 8d02200922ca9a783e34528570d43c1acbef7124
parent 1c77dc34f3deae2c10b151591972398f24ffee0d
Author: Ryan Sepassi <rsepassi@gmail.com>
Date:   Wed, 17 Jun 2026 12:38:22 -0700

arm32: fix AAPCS32 mixed fp/int struct-param ABI (8-aligned aggregate split)

A struct passed by value whose natural alignment is 8 (e.g. struct{double d;
long i;}) was miscompiled: the caller and callee disagreed on its placement, so
`take(int pre, struct S s, double post)` returned garbage (parse case
6_5_2_2_06_struct_param_mixed_fp_int: O0 got 229, O1 got 5, want 42).

Root causes, all in the AAPCS32 arg walk:
- The ABI classifier (abi_aapcs32.c classify_aggregate) dropped the aggregate's
  natural alignment, marking every part align=4. The 8-byte even-register /
  8-byte-stack signal the i64/double scalar pair carries on parts[0].align was
  therefore absent for an 8-aligned record.
- Two divergent predicates: the caller size walk used the *type* alignment
  (rounds to even, straddle-correct), while the callee bind + arg-advance used
  `arm_arg_is_aligned_pair` (parts[0].align>=8 AND nparts==2) — false for the
  4-part struct, so the callee never rounded NCRN to even.
- The caller *emit* added a "spill the whole arg" override that forced any arg
  not fitting the remaining even-register pair entirely onto the stack. That is
  the i64/double no-straddle rule; an aggregate must straddle the reg/stack
  boundary (AAPCS C.5).

Fix, correct-by-construction and unified:
- classify_aggregate surfaces the record's natural alignment on parts[0].align
  (arm_param_abi mirrors it for synthesized variadic args).
- One `ai`-derived predicate, arm_arg_needs_even, replaces both
  arm_arg_is_aligned_pair and arm_arg_needs_8align, used by all four walk sites
  (callee bind, arg-advance, caller size, caller emit).
- Drop the no-straddle override. Round-to-even + naive per-part placement is
  correct for both shapes: an 8-byte scalar pair fits-or-fully-spills (its 2
  parts never straddle after rounding), a larger aggregate straddles per C.5.

arm32 toy corpus unchanged at 423/0/2; the struct case now passes O0+O1.

Also triage 4 ILP32 wrong-result parse cases (long is 32-bit on arm32):
- far_slot_large_frame, builtin_generic_overflow: made data-model-agnostic
  (long long / __LONG_MAX__) so they exercise the 8-byte far slot / long mul
  overflow and pass on arm32 AND the LP64 arches — the backend handles both.
- 6_5_64_unsigned_size_division, rv64_atomic_widths_orders: .arm32.skip
  (64-bit-size_t idiom / rv64 64-bit atomics), matching the existing rv32 skips.

Diffstat:
Msrc/abi/abi_aapcs32.c | 8++++++++
Msrc/arch/arm32/native.c | 95++++++++++++++++++++++++++++++++++++++-----------------------------------------
Atest/parse/cases/6_5_64_unsigned_size_division.arm32.skip | 1+
Mtest/parse/cases/builtin_generic_overflow.c | 10++++++----
Mtest/parse/cases/far_slot_large_frame.c | 10++++++----
Atest/parse/cases/rv64_atomic_widths_orders.arm32.skip | 1+
6 files changed, 68 insertions(+), 57 deletions(-)

diff --git a/src/abi/abi_aapcs32.c b/src/abi/abi_aapcs32.c @@ -88,6 +88,14 @@ static void classify_aggregate(TargetABI* a, KitCgTypeId t, ABIArgInfo* out, parts[i].src_offset = off; off += chunk; } + /* Surface the aggregate's natural alignment on the first part so the native + * AAPCS walk applies the round-NCRN-to-even / 8-byte-stack rule to an + * 8-byte-aligned record (e.g. struct{double;long;}) exactly as it does to an + * i64/double scalar pair. The remaining parts keep 4-byte (word) alignment; + * only parts[0].align drives the even/8-align decision. An aggregate that + * does not fully fit straddles the reg/stack boundary (AAPCS C.5) — the + * native walk realizes that without a no-straddle override. */ + parts[0].align = ti.align ? ti.align : ARM32_GPR_BYTES; out->kind = ABI_ARG_DIRECT; out->flags = ABI_AF_NONE; out->parts = parts; diff --git a/src/arch/arm32/native.c b/src/arch/arm32/native.c @@ -1536,15 +1536,22 @@ static KitCgTypeId arm_part_scalar_type(const ABIArgPart* part) { } } -/* AAPCS pair rule: an 8-byte scalar argument (i64/double — surfaced by the ABI - * as a 2-word DIRECT arg whose parts carry align=8, the slot-exceeding marker) - * must occupy an even/odd core-register pair (r0:r1 or r2:r3); the cursor rounds - * up to an even index first. Returns 1 if `ai`'s first part demands that. A - * 4-byte-aligned two-word aggregate (struct{int;int;}) has align=4 and is NOT - * pair-aligned, so the shape (nparts==2) alone is insufficient — the alignment - * is the discriminator. */ -static int arm_arg_is_aligned_pair(const ABIArgInfo* ai) { - return ai->kind == ABI_ARG_DIRECT && ai->nparts == 2u && ai->parts && +/* AAPCS even/8-align rule: an argument whose natural alignment is 8 bytes + * rounds NCRN up to an even core register (r0:r1 or r2:r3) and 8-byte-aligns its + * stack slot. This covers both an i64/double scalar pair AND an 8-byte-aligned + * aggregate (e.g. struct{double;long;}). The ABI classifier surfaces that + * alignment on the first part (scalar pair lanes carry align=8; an aggregate's + * parts[0].align is the record's natural alignment; the synthesized variadic + * form below mirrors this), so parts[0].align is the single discriminator — + * uniform for named and variadic args and independent of part count. + * + * This predicate is ONLY the even/8-align rule. Whether an argument may split + * across the reg/stack boundary is not a separate flag: round-to-even makes an + * 8-byte scalar pair fit-or-fully-spill (its 2 parts never straddle), while a + * larger aggregate straddles per AAPCS C.5 — the naive per-part walks below + * realize both with no special case. */ +static int arm_arg_needs_even(const ABIArgInfo* ai) { + return ai->kind == ABI_ARG_DIRECT && ai->nparts > 0u && ai->parts && ai->parts[0].align >= 2u * ARM32_GPR_BYTES; } @@ -1554,7 +1561,7 @@ static void arm_bind_native_param(NativeTarget* t, const CGParamDesc* p, const ABIFuncInfo* abi = abi_cg_func_info(t->c->abi, a->func->fn_type); const ABIArgInfo* ai = p->index < abi->nparams ? &abi->params[p->index] : NULL; int to_reg = dst.kind == NATIVE_LOC_REG; - int pair = ai && arm_arg_is_aligned_pair(ai); + int even = ai && arm_arg_needs_even(ai); u32 i; if (!ai || ai->kind == ABI_ARG_IGNORE) return; if (ai->kind == ABI_ARG_INDIRECT) { @@ -1597,16 +1604,12 @@ static void arm_bind_native_param(NativeTarget* t, const CGParamDesc* p, arm_copy_bytes(t, d_addr, from, access); return; } - /* AAPCS: round NCRN up to even before an 8-byte aligned pair, and if the pair - * does not fully fit in the remaining core registers it passes ENTIRELY on the - * (8-byte-aligned) stack — it never straddles the r3/stack boundary. */ - if (pair) { - a->next_param_int = align_up_u32(a->next_param_int, 2u); - if (a->next_param_int + 2u > 4u) { - a->next_param_int = 4u; - a->next_param_stack = align_up_u32(a->next_param_stack, 2u * ARM32_GPR_BYTES); - } - } + /* AAPCS: round NCRN up to even before an 8-byte-aligned argument. The parts + * then fill core registers r0..r3; any that do not fit spill to the (8-byte- + * aligned for part 0) incoming stack window. An 8-byte scalar pair fully fits + * or fully spills (round-to-even guarantees it); a larger aggregate straddles + * the reg/stack boundary (AAPCS C.5). */ + if (even) a->next_param_int = align_up_u32(a->next_param_int, 2u); for (i = 0; i < ai->nparts; ++i) { const ABIArgPart* part = &ai->parts[i]; KitCgTypeId lty = arm_part_scalar_type(part); @@ -1616,8 +1619,12 @@ static void arm_bind_native_param(NativeTarget* t, const CGParamDesc* p, } else { /* Incoming stack arg: above the saved register block = [r7 + N + k], * where N = saved_block_bytes (8 for the bare {r7,lr} -O0 prologue; larger - * when the -O1 known-frame prologue also PUSHes callee-saves). */ + * when the -O1 known-frame prologue also PUSHes callee-saves). The first + * stack part of an 8-byte-aligned arg lands on an 8-byte boundary. */ NativeAddr sa; + if (i == 0 && even) + a->next_param_stack = + align_up_u32(a->next_param_stack, 2u * ARM32_GPR_BYTES); src = native_loc_reg(lty, NATIVE_REG_INT, ARM_SCRATCH); memset(&sa, 0, sizeof sa); sa.base_kind = NATIVE_ADDR_BASE_REG; @@ -1656,6 +1663,7 @@ static void arm_bind_native_param(NativeTarget* t, const CGParamDesc* p, * fit in r0..r3, passes ENTIRELY on the 8-byte-aligned stack (no straddle); * - every other part takes the next core register, else a 4-byte stack slot. */ static void arm_arg_advance(const ABIArgInfo* ai, u32* next_int, u32* stack) { + int even = arm_arg_needs_even(ai); u32 p; if (ai->kind == ABI_ARG_IGNORE) return; if (ai->kind == ABI_ARG_INDIRECT) { @@ -1663,14 +1671,10 @@ static void arm_arg_advance(const ABIArgInfo* ai, u32* next_int, u32* stack) { else *stack += 4u; return; } - if (arm_arg_is_aligned_pair(ai)) { - *next_int = align_up_u32(*next_int, 2u); - if (*next_int + 2u > 4u) { - *next_int = 4u; - *stack = align_up_u32(*stack, 2u * ARM32_GPR_BYTES); - } - } + if (even) *next_int = align_up_u32(*next_int, 2u); for (p = 0; p < ai->nparts; ++p) { + if (p == 0 && even && *next_int >= 4u) + *stack = align_up_u32(*stack, 2u * ARM32_GPR_BYTES); if (*next_int < 4u) (*next_int)++; else *stack += 4u; } @@ -1701,6 +1705,10 @@ static const ABIArgInfo* arm_param_abi(NativeTarget* t, const ABIFuncInfo* abi, parts[p].align = 4u; parts[p].src_offset = off; } + /* Mirror the named classifier: parts[0].align carries the argument's natural + * alignment so the AAPCS even/8-align walk treats a synthesized variadic + * i64/double (or 8-aligned aggregate) the same as a named one. */ + parts[0].align = align; scratch->nparts = (u16)nparts; scratch->parts = parts; return scratch; @@ -1717,15 +1725,6 @@ static const ABIArgInfo* arm_param_abi(NativeTarget* t, const ABIFuncInfo* abi, return scratch; } -/* An AAPCS32 8-byte argument (i64 / soft double) requires even-register pair - * alignment in the core registers and 8-byte stack alignment. Detect it from - * the argument's natural alignment (8) — robust for both the named two-part - * classification and the synthesized variadic form. */ -static int arm_arg_needs_8align(NativeTarget* t, const NativeCallDesc* desc, - u32 i) { - return native_type_align(t, desc->args[i].type) >= 8u; -} - /* Outgoing stack-argument bytes for a call: int parts beyond r0..r3. */ static u32 arm_call_stack_size(NativeTarget* t, const NativeCallDesc* desc) { const ABIFuncInfo* abi = abi_cg_func_info(t->c->abi, desc->fn_type); @@ -1735,7 +1734,7 @@ static u32 arm_call_stack_size(NativeTarget* t, const NativeCallDesc* desc) { if (!abi) return 0; for (i = 0; i < desc->nargs; ++i) { const ABIArgInfo* ai = arm_param_abi(t, abi, desc, i, &scratch); - int eight = arm_arg_needs_8align(t, desc, i); + int even = arm_arg_needs_even(ai); if (ai->kind == ABI_ARG_IGNORE) continue; if (ai->kind == ABI_ARG_INDIRECT) { if (next_int < 4u) next_int++; @@ -1745,9 +1744,9 @@ static u32 arm_call_stack_size(NativeTarget* t, const NativeCallDesc* desc) { } continue; } - if (eight) next_int = align_up_u32(next_int, 2u); /* even-pair */ + if (even) next_int = align_up_u32(next_int, 2u); /* even-pair */ for (p = 0; p < ai->nparts; ++p) { - if (p == 0 && eight && next_int >= 4u) stack = align_up_u32(stack, 8u); + if (p == 0 && even && next_int >= 4u) stack = align_up_u32(stack, 8u); if (next_int < 4u) next_int++; else stack += 4u; } @@ -1934,7 +1933,7 @@ static void arm_plan_call(NativeTarget* t, const NativeCallDesc* desc, ABIArgInfo scratch; for (i = 0; i < desc->nargs; ++i) { const ABIArgInfo* ai = arm_param_abi(t, abi, desc, i, &scratch); - int eight = arm_arg_needs_8align(t, desc, i); + int even = arm_arg_needs_even(ai); if (ai->kind == ABI_ARG_IGNORE) continue; if (ai->kind == ABI_ARG_INDIRECT) { /* Pass a large aggregate by reference: a pointer to the caller's copy @@ -1956,13 +1955,11 @@ static void arm_plan_call(NativeTarget* t, const NativeCallDesc* desc, } continue; } - /* AAPCS32: an 8-byte arg starts in an even register pair; if it cannot fit - * (an odd reg remains, or none), the whole value goes to 8-byte-aligned - * stack. */ - if (eight) { - next_int = align_up_u32(next_int, 2u); - if (next_int + ai->nparts > 4u) next_int = 4u; /* spill the whole arg */ - } + /* AAPCS32: round NCRN up to even for an 8-byte-aligned arg, then fill core + * registers r0..r3; parts that do not fit spill to the (8-byte-aligned for + * part 0) outgoing stack. An 8-byte scalar pair fits-or-fully-spills; + * a larger aggregate straddles the reg/stack boundary (AAPCS C.5). */ + if (even) next_int = align_up_u32(next_int, 2u); for (p = 0; p < ai->nparts; ++p) { const ABIArgPart* part = &ai->parts[p]; KitCgTypeId lty = arm_part_scalar_type(part); @@ -1976,7 +1973,7 @@ static void arm_plan_call(NativeTarget* t, const NativeCallDesc* desc, } else { NativeLoc tmp = native_loc_reg(lty, NATIVE_REG_INT, arg_stage); arm_load_part(t, tmp, desc->args[i], part->src_offset, part->size); - if (p == 0 && eight) stack = align_up_u32(stack, 8u); + if (p == 0 && even) stack = align_up_u32(stack, 8u); else stack = align_up_u32(stack, 4u); arm_store_outgoing(t, stack, tmp, part->size, tail); stack += 4u; diff --git a/test/parse/cases/6_5_64_unsigned_size_division.arm32.skip b/test/parse/cases/6_5_64_unsigned_size_division.arm32.skip @@ -0,0 +1 @@ +This case is the 64-bit size_t overflow-check idiom: it asserts that 2^30 elements of a 24-byte struct do NOT exceed MAX_SIZE_T (so the LIMIT_N clamp is a no-op and limit == n). That holds only when size_t/unsigned long is 64-bit (LP64). On arm32 ILP32 unsigned long is 32-bit, MAX_SIZE_T/sizeof == 0xFFFFFFFF/24 < 2^30, so the clamp legitimately fires and limit != n (kit and clang both return 10). Data-model-dependent, like the i128_* cases; same skip as rv32. diff --git a/test/parse/cases/builtin_generic_overflow.c b/test/parse/cases/builtin_generic_overflow.c @@ -2,8 +2,10 @@ * operation type is inferred from the result pointer's pointee, dispatching to * the same per-type intrinsics as the explicit __builtin_smull_overflow family. * kit's own source (src/cg/control.c cg_checked_scaled_offset) uses these, so - * they must self-host. Returns the count of passing checks (expect 7). LP64 - * targets only (aa64/x64/rv64). */ + * they must self-host. Returns the count of passing checks (expect 7). The + * signed-`long` overflow check uses __LONG_MAX__ (LONG_MAX*2 overflows `long` + * under both LP64 and the ILP32 arm32/rv32 data model), so the case is + * data-model-agnostic. */ int test_main(void) { int ok = 0; @@ -14,8 +16,8 @@ int test_main(void) { /* signed mul, no overflow */ if (!__builtin_mul_overflow(3L, 4L, &lp) && lp == 12L) ok++; - /* signed mul, overflow */ - if (__builtin_mul_overflow((long)0x7fffffffffffffffL, 2L, &lp)) ok++; + /* signed mul, overflow (LONG_MAX*2 overflows `long` under LP64 and ILP32) */ + if (__builtin_mul_overflow((long)__LONG_MAX__, 2L, &lp)) ok++; /* signed add, no overflow */ if (!__builtin_add_overflow(100, 23, &ip) && ip == 123) ok++; /* signed int add, overflow */ diff --git a/test/parse/cases/far_slot_large_frame.c b/test/parse/cases/far_slot_large_frame.c @@ -6,22 +6,24 @@ * out-of-range scaled immediate (which previously tripped "far slot offset out * of positive scaled range" compiling src/api/package.c, a ~69KB frame). * x64/rv64 have no scaled-offset constraint, so this just confirms correctness - * everywhere. Returns the count of passing checks (expect 7). */ + * everywhere. `x` is `long long` (8 bytes on every data model, including the + * ILP32 arm32/rv32 targets) so the 8-byte far slot is exercised regardless of + * the width of `long`. Returns the count of passing checks (expect 7). */ int test_main(void) { - volatile long x; + volatile long long x; volatile int y; volatile char huge[40000]; int i; int ok = 0; long s = 0; - x = 0x1122334455667788L; + x = 0x1122334455667788LL; y = 0x0a0b0c0d; for (i = 0; i < 40000; i += 997) huge[i] = (char)(i & 0x7f); /* Read the far slots back. */ - if (x == 0x1122334455667788L) ok += 3; + if (x == 0x1122334455667788LL) ok += 3; if (y == 0x0a0b0c0d) ok += 4; /* Keep `huge` live so the big frame is not elided. */ diff --git a/test/parse/cases/rv64_atomic_widths_orders.arm32.skip b/test/parse/cases/rv64_atomic_widths_orders.arm32.skip @@ -0,0 +1 @@ +This rv64-targeted case exercises 64-bit atomics through `long`, relying on long being 64-bit (LP64) so i64_loc can hold values like 0x100000000. On arm32 ILP32 long is 32-bit: the store of the long long literal 0x100000000L truncates to 0, so the != comparison against the untruncated 64-bit literal fails and the case returns 9 (kit and clang agree). arm32's 32-bit atomic codegen (LDREX/STREX, fetch_add, CAS) is covered by builtin_06..25, which pass on arm32. Same skip as rv32.