Author: Akash Manna Date: 2026-09-01T13:20:42+08:00 New Revision: edfb6e89d94135c6ce5e2f5f5674812968a1c307
URL: https://github.com/llvm/llvm-project/commit/edfb6e89d94135c6ce5e2f5f5674812968a1c307 DIFF: https://github.com/llvm/llvm-project/commit/edfb6e89d94135c6ce5e2f5f5674812968a1c307.diff LOG: [Clang][CodeGen][X86] Fix crash on __int128 bit-field access units (#216777) Fixes #202205 The x86-64 SysV classifier skipped every unnamed bit-field as padding, so an eightbyte holding nothing but a non-zero-width unnamed bit-field stayed `NO_CLASS`. GCC treats that storage as INTEGER. The crash falls out of this: a run of `__int128` bit-fields is lowered to a single `i128` access unit spanning both eightbytes, but with only one of them classified INTEGER the `i128` gets queried at offset 8 and hits `assert(IROffset == 0)` in `GetINTEGERTypeAtOffset` — or `assert(Hi == Integer)` in the callers, depending on which eightbyte holds the named field. It also silently diverges from GCC on ordinary shapes like `struct { long : 64; long a; }`, which clang passed in one register where GCC uses two. The fix skips only zero-length bit-fields and classifies the rest like named ones, matching GCC. Both eightbytes of an `__int128` bit-field run then come out `INTEGER`, so the existing asserts hold unchanged. The experimental ABI library mirrors the same classifier, so it gets the same one-line change; the new union test exercises it through `-fexperimental-abi-lowering`. The tests cover the `__int128` case in both field orders for arguments and returns, and also the `non-i128` shapes whose classification changes. LLM tools were used for this contribution. I've reviewed, built, and tested the change myself before pushing to GitHub. Added: Modified: clang/docs/ReleaseNotes.md clang/lib/CodeGen/Targets/X86.cpp clang/test/CodeGen/X86/x86_64-arguments.c clang/test/CodeGen/X86/x86_64-union-abi.c llvm/lib/ABI/Targets/X86.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index a616bd41f3560..82772714b5d46 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -96,6 +96,13 @@ features cannot lower the translation-unit ABI level; - On MIPS N32/N64, an `__int128` now correctly start in an even-numbered register or 16-byte aligned stack slot, matching GCC. +- On x86-64 System V, a non-zero-width unnamed bit-field now classifies the + eightbytes it occupies as INTEGER, like a named bit-field, matching GCC. + Aggregates where this changes the classification may be passed or returned + diff erently -- a struct holding a run of `__int128` bit-fields, for example, + now travels in the two integer registers the ABI assigns it. This also fixes + a crash when such a struct was passed or returned. (#GH202205) + ### AST Dumping Potentially Breaking Changes ### Clang Frontend Potentially Breaking Changes diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp index 1dc3bd0740baa..d60d71775a9d1 100644 --- a/clang/lib/CodeGen/Targets/X86.cpp +++ b/clang/lib/CodeGen/Targets/X86.cpp @@ -2207,8 +2207,9 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo, uint64_t Offset = OffsetBase + Layout.getFieldOffset(idx); bool BitField = i->isBitField(); - // Ignore padding bit-fields. - if (BitField && i->isUnnamedBitField()) + // Ignore zero-length bit-fields. Other unnamed bit-fields are real + // storage and classify like named ones, matching GCC. + if (BitField && i->isZeroLengthBitField()) continue; // AMD64-ABI 3.2.3p2: Rule 1. If the size of an object is larger than @@ -2249,7 +2250,7 @@ void X86_64ABIInfo::classify(QualType Ty, uint64_t OffsetBase, Class &Lo, // structure to be passed in memory even if unaligned, and // therefore they can straddle an eightbyte. if (BitField) { - assert(!i->isUnnamedBitField()); + assert(!i->isZeroLengthBitField()); uint64_t Offset = OffsetBase + Layout.getFieldOffset(idx); uint64_t Size = i->getBitWidthValue(); diff --git a/clang/test/CodeGen/X86/x86_64-arguments.c b/clang/test/CodeGen/X86/x86_64-arguments.c index 580f9487395d3..b56792dd50cdd 100644 --- a/clang/test/CodeGen/X86/x86_64-arguments.c +++ b/clang/test/CodeGen/X86/x86_64-arguments.c @@ -590,6 +590,58 @@ _BitInt(128) f74(__uint128_t b, __uint128_t c, __uint128_t d, long e, _BitInt(12 return a; } +// check that non-zero-width unnamed bit-fields classify INTEGER like named +// ones, so a run of (u)int128_t bit-fields is passed and returned as an i128 +struct s75 { + __uint128_t : 124; + __uint128_t a : 4; +}; +// CHECK-LABEL: define{{.*}} i128 @f75() +struct s75 f75(void) { + return (struct s75){0}; +} +// CHECK-LABEL: define{{.*}} void @f76(i128 %a.coerce) +void f76(struct s75 a) { +} + +struct s77 { + __uint128_t a : 4; + __uint128_t : 124; +}; +// CHECK-LABEL: define{{.*}} i128 @f77() +struct s77 f77(void) { + return (struct s77){0}; +} +// CHECK-LABEL: define{{.*}} void @f78(i128 %a.coerce) +void f78(struct s77 a) { +} + +// an unnamed bit-field filling the low eightbyte makes it INTEGER +struct s79 { + long : 64; + long a; +}; +// CHECK-LABEL: define{{.*}} { i64, i64 } @f79() +struct s79 f79(void) { + return (struct s79){0}; +} +// CHECK-LABEL: define{{.*}} void @f80(i64 %a.coerce0, i64 %a.coerce1) +void f80(struct s79 a) { +} + +// an unnamed bit-field in the high eightbyte is INTEGER while the low is SSE +struct s81 { + double d; + int : 32; +}; +// CHECK-LABEL: define{{.*}} { double, i32 } @f81() +struct s81 f81(void) { + return (struct s81){0}; +} +// CHECK-LABEL: define{{.*}} void @f82(double %a.coerce0, i32 %a.coerce1) +void f82(struct s81 a) { +} + /// The synthesized __va_list_tag does not have file/line fields. // CHECK: = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "__va_list_tag", // CHECK-NOT: file: diff --git a/clang/test/CodeGen/X86/x86_64-union-abi.c b/clang/test/CodeGen/X86/x86_64-union-abi.c index a9b5c60f7eeaa..3bc60464c02a8 100644 --- a/clang/test/CodeGen/X86/x86_64-union-abi.c +++ b/clang/test/CodeGen/X86/x86_64-union-abi.c @@ -76,3 +76,22 @@ void take_wide_unnamed(union WideUnnamedBitfield u); void call_wide_unnamed(union WideUnnamedBitfield u) { take_wide_unnamed(u); } // CHECK-DAG: declare void @take_wide_unnamed(i64) + +// A non-zero-width unnamed bitfield is INTEGER, which beats the double's SSE +// in the merge, so the union travels in a GPR. +union DoubleUnnamedBitfield { + double d; + long : 64; +}; + +void take_double_unnamed(union DoubleUnnamedBitfield u); +void call_double_unnamed(union DoubleUnnamedBitfield u) { + take_double_unnamed(u); +} + +// CHECK-DAG: declare void @take_double_unnamed(i64) + +union DoubleUnnamedBitfield ret_double_unnamed(void); +void call_ret_double_unnamed(void) { ret_double_unnamed(); } + +// CHECK-DAG: declare i64 @ret_double_unnamed() diff --git a/llvm/lib/ABI/Targets/X86.cpp b/llvm/lib/ABI/Targets/X86.cpp index 306068110c849..e496c4bf4bec5 100644 --- a/llvm/lib/ABI/Targets/X86.cpp +++ b/llvm/lib/ABI/Targets/X86.cpp @@ -560,7 +560,9 @@ void X86_64TargetInfo::classify(const Type *T, uint64_t OffsetBase, Class &Lo, uint64_t Offset = OffsetBase + Field.OffsetInBits; bool BitField = Field.IsBitField; - if (BitField && Field.IsUnnamedBitfield) + // Ignore zero-length bit-fields. Other unnamed bit-fields are real + // storage and classify like named ones, matching GCC. + if (BitField && Field.BitFieldWidth == 0) continue; if (Size > 128 && _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
