From 38e053b96c010552b36d46101e2f46b646f76200 Mon Sep 17 00:00:00 2001 From: kalsprite Date: Fri, 14 Aug 2026 23:02:04 -0700 Subject: [PATCH] i386: cap cmplx / quaternion alignment; abi test: cover a narrow member in front of complex --- src/types.cpp | 25 ++++++++++++++++--------- tests/abi/gen.odin | 31 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/types.cpp b/src/types.cpp index b51e44a45..8e7fea777 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -4353,6 +4353,18 @@ gb_internal i64 type_align_of(Type *t) { } +// The largest alignment the target permits. The i386 System V psABI caps every scalar at 4, unlike +// Windows. Anything that derives its alignment from a COMPONENT rather than from its own size has +// to be capped here too. +gb_internal i64 type_target_max_align(void) { + i64 max_align = build_context.max_align; + if (build_context.metrics.arch == TargetArch_i386 && + build_context.metrics.os != TargetOs_windows) { + max_align = gb_min(max_align, 4); + } + return max_align; +} + gb_internal i64 type_align_of_internal(Type *t, TypePath *path) { GB_ASSERT(path != nullptr); if (t->failure) { @@ -4377,10 +4389,11 @@ gb_internal i64 type_align_of_internal(Type *t, TypePath *path) { case Basic_uintptr: case Basic_rawptr: return build_context.ptr_size; + // A complex aligns to one component and a quaternion to one of its four. case Basic_complex32: case Basic_complex64: case Basic_complex128: - return type_size_of_internal(t, path) / 2; + return gb_min(type_size_of_internal(t, path) / 2, type_target_max_align()); case Basic_quaternion64: case Basic_quaternion128: case Basic_quaternion256: - return type_size_of_internal(t, path) / 4; + return gb_min(type_size_of_internal(t, path) / 4, type_target_max_align()); } } break; @@ -4531,13 +4544,7 @@ gb_internal i64 type_align_of_internal(Type *t, TypePath *path) { // NOTE(bill): Things that are bigger than build_context.ptr_size, are actually comprised of smaller types // TODO(bill): Is this correct for 128-bit types (integers)? - i64 max_align = build_context.max_align; - if (build_context.metrics.arch == TargetArch_i386 && - build_context.metrics.os != TargetOs_windows) { - // the i386 System V psABI aligns every scalar to at most 4, unlike Windows - max_align = gb_min(max_align, 4); - } - return gb_clamp(next_pow2(type_size_of_internal(t, path)), 1, max_align); + return gb_clamp(next_pow2(type_size_of_internal(t, path)), 1, type_target_max_align()); } gb_internal i64 *type_set_offsets_of(Slice const &fields, bool is_packed, bool is_raw_union, i64 min_field_align, i64 max_field_align) { diff --git a/tests/abi/gen.odin b/tests/abi/gen.odin index 611b5343d..a793faf30 100644 --- a/tests/abi/gen.odin +++ b/tests/abi/gen.odin @@ -285,6 +285,9 @@ build :: proc() { // control on anything that reads signedness where it should not {"u8"}, {"u16"}, {"u32"}, {"u64"}, {"u8", "u32"}, {"u16", "u64"}, {"u32", "f32"}, {"u64", "f64"}, + // a narrow member in FRONT of a complex: the complex aligns to its component, so a wrong + // component alignment moves it and changes the struct's size. Nothing else here reaches that. + {"i8", "c64"}, {"i8", "c128"}, {"i16", "c128"}, } for tags in combos { odin_members := make([]string, len(tags), context.temp_allocator) @@ -390,6 +393,33 @@ build :: proc() { tp("%s(%s)", scalar(q.tag).odin, v), } } + // the same aggregate with a narrow member in front, which is what catches a + // wrong component alignment: it moves the quaternion and resizes the struct + off_fields := make([]Leaf, 5) + off_getters := make([][2]string, 5) + off_fields[0] = leaf("a", "i8", 0) + off_getters[0] = {"{}.a", tp("i8(%s)", val(0, "i8"))} + for i in 0 ..< 4 { + v := val(i, q.tag) + off_fields[i + 1] = leaf2("", tp("{}.q.%s", lanes[i]), q.tag, v) + off_getters[i + 1] = { + tp("%s({}.q)", accessors[i]), + tp("%s(%s)", scalar(q.tag).odin, v), + } + } + add( + tp("off_%s", q.name), + tp("struct { a: i8, q: %s }", q.odin), + tp("struct { int8_t a; struct { %s x, y, z, w; } q; }", q.c_elem), + off_fields, + tier = q.tier, + odin_set = strs( + tp("{}.a = %s", val(0, "i8")), + tp("{}.q = quaternion(x=%s, y=%s, z=%s, w=%s)", + val(0, q.tag), val(1, q.tag), val(2, q.tag), val(3, q.tag)), + ), + odin_get = off_getters, + ) add( tp("bs_%s", q.name), q.odin, @@ -1247,6 +1277,7 @@ ABI_MUTATE :: #config(ABI_MUTATE, false) // // It is a single defect, but very noisy in the test results as ~75% trip it. // Turn it on with ` + "`-define:ABI_VARARGS=true`" + ` to measure it. +ABI_VARARGS :: #config(ABI_VARARGS, false)