[clang] [ARM, AArch64] Fix ABI bugs with over-sized bitfields (PR #126774)
Oliver Stannard via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 20 05:59:27 PST 2025
https://github.com/ostannard updated https://github.com/llvm/llvm-project/pull/126774
>From 223eb6f3153bce087306f54c398034ff7a64c1d1 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Tue, 11 Feb 2025 17:40:55 +0000
Subject: [PATCH 1/4] Add tests showing bugs
---
clang/test/CodeGen/aapcs-align.cpp | 43 +++++++++++++++++++
clang/test/CodeGen/aapcs64-align.cpp | 64 ++++++++++++++++++++++++++++
2 files changed, 107 insertions(+)
diff --git a/clang/test/CodeGen/aapcs-align.cpp b/clang/test/CodeGen/aapcs-align.cpp
index 4f393d9e6b7f3..01b5ea5a69fdc 100644
--- a/clang/test/CodeGen/aapcs-align.cpp
+++ b/clang/test/CodeGen/aapcs-align.cpp
@@ -6,6 +6,11 @@
extern "C" {
+// CHECK: @sizeof_OverSizedBitfield ={{.*}} global i32 8
+// CHECK: @alignof_OverSizedBitfield ={{.*}} global i32 8
+// CHECK: @sizeof_VeryOverSizedBitfield ={{.*}} global i32 16
+// CHECK: @alignof_VeryOverSizedBitfield ={{.*}} global i32 8
+
// Base case, nothing interesting.
struct S {
int x, y;
@@ -138,4 +143,42 @@ void g6() {
// CHECK: call void @f6m(i32 noundef 1, i32 noundef 2, i32 noundef 3, i32 noundef 4, i32 noundef 5, [4 x i32] [i32 6, i32 7, i32 0, i32 undef])
// CHECK: declare void @f6(i32 noundef, [4 x i32])
// CHECK: declare void @f6m(i32 noundef, i32 noundef, i32 noundef, i32 noundef, i32 noundef, [4 x i32])
+
+// Over-sized bitfield, which results in a 64-bit container type, so 64-bit
+// alignment.
+struct OverSizedBitfield {
+ int x : 64;
+};
+
+unsigned sizeof_OverSizedBitfield = sizeof(OverSizedBitfield);
+unsigned alignof_OverSizedBitfield = alignof(OverSizedBitfield);
+
+// CHECK: define{{.*}} void @g7
+// CHECK: call void @f7(i32 noundef 1, [2 x i32] [i32 42, i32 0])
+// CHECK: declare void @f7(i32 noundef, [2 x i32])
+void f7(int a, OverSizedBitfield b);
+void g7() {
+ OverSizedBitfield s = {42};
+ f7(1, s);
+}
+
+// There are no 128-bit fundamental data types defined by AAPCS32, so this gets
+// a 64-bit container plus 64 bits of padding, giving it a size of 16 bytes and
+// alignment of 8 bytes.
+struct VeryOverSizedBitfield {
+ int x : 128;
+};
+
+unsigned sizeof_VeryOverSizedBitfield = sizeof(VeryOverSizedBitfield);
+unsigned alignof_VeryOverSizedBitfield = alignof(VeryOverSizedBitfield);
+
+// CHECK: define{{.*}} void @g8
+// CHECK: call void @f8(i32 noundef 1, [4 x i32] [i32 42, i32 0, i32 0, i32 0])
+// CHECK: declare void @f8(i32 noundef, [4 x i32])
+void f8(int a, VeryOverSizedBitfield b);
+void g8() {
+ VeryOverSizedBitfield s = {42};
+ f8(1, s);
+}
+
}
diff --git a/clang/test/CodeGen/aapcs64-align.cpp b/clang/test/CodeGen/aapcs64-align.cpp
index 7a8151022852e..e9d22135836e4 100644
--- a/clang/test/CodeGen/aapcs64-align.cpp
+++ b/clang/test/CodeGen/aapcs64-align.cpp
@@ -5,6 +5,13 @@
extern "C" {
+// CHECK: @sizeof_OverSizedBitfield ={{.*}} global i32 8
+// CHECK: @alignof_OverSizedBitfield ={{.*}} global i32 8
+// CHECK: @sizeof_VeryOverSizedBitfield ={{.*}} global i32 16
+// CHECK: @alignof_VeryOverSizedBitfield ={{.*}} global i32 8
+// CHECK: @sizeof_RidiculouslyOverSizedBitfield ={{.*}} global i32 32
+// CHECK: @alignof_RidiculouslyOverSizedBitfield ={{.*}} global i32 8
+
// Base case, nothing interesting.
struct S {
long x, y;
@@ -161,5 +168,62 @@ int test_bitint8(){
}
// CHECK: ret i32 1
+// Over-sized bitfield, which results in a 64-bit container type, so 64-bit
+// alignment.
+struct OverSizedBitfield {
+ int x : 64;
+};
+
+unsigned sizeof_OverSizedBitfield = sizeof(OverSizedBitfield);
+unsigned alignof_OverSizedBitfield = alignof(OverSizedBitfield);
+
+// CHECK: define{{.*}} void @g7
+// CHECK: call void @f7(i32 noundef 1, i64 42)
+// CHECK: declare void @f7(i32 noundef, i64)
+void f7(int a, OverSizedBitfield b);
+void g7() {
+ OverSizedBitfield s = {42};
+ f7(1, s);
+}
+
+// There are no 128-bit fundamental data types defined by AAPCS32, so this gets
+// a 64-bit container plus 64 bits of padding, giving it a size of 16 bytes and
+// alignment of 8 bytes.
+struct VeryOverSizedBitfield {
+ int x : 128;
+};
+
+unsigned sizeof_VeryOverSizedBitfield = sizeof(VeryOverSizedBitfield);
+unsigned alignof_VeryOverSizedBitfield = alignof(VeryOverSizedBitfield);
+
+// CHECK: define{{.*}} void @g8
+// CHECK: call void @f8(i32 noundef 1, [2 x i64] [i64 42, i64 0])
+// CHECK: declare void @f8(i32 noundef, [2 x i64])
+void f8(int a, VeryOverSizedBitfield b);
+void g8() {
+ VeryOverSizedBitfield s = {42};
+ f8(1, s);
+}
+
+// There are no bigger fundamental data types, so this gets a 128-bit container
+// and 128 bits of padding, giving the struct a size of 32 bytes, and an
+// alignment of 16 bytes. This is over the PCS size limit of 16 bytes, so it
+// will be passed indirectly.
+struct RidiculouslyOverSizedBitfield {
+ int x : 256;
+};
+
+unsigned sizeof_RidiculouslyOverSizedBitfield = sizeof(RidiculouslyOverSizedBitfield);
+unsigned alignof_RidiculouslyOverSizedBitfield = alignof(RidiculouslyOverSizedBitfield);
+
+// CHECK: define{{.*}} void @g9
+// CHECK: call void @f9(i32 noundef 1, ptr noundef nonnull %agg.tmp)
+// CHECK: declare void @f9(i32 noundef, ptr noundef)
+void f9(int a, RidiculouslyOverSizedBitfield b);
+void g9() {
+ RidiculouslyOverSizedBitfield s = {42};
+ f9(1, s);
+}
+
}
>From 75e6e376b72b8bd47c45539d584b03e4d1163d16 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Tue, 11 Feb 2025 17:37:57 +0000
Subject: [PATCH 2/4] [AArch64] Allow 128-bit containers for over-sized
bitfields
AAPCS64 defines the bitfield layout algorithm for over-sized bitfields
as picking a container which is the fundamental integer data type with
the largest size less than or equal to the bit-field width. Since
AAPCS64 has a 128-bit integer fundamental data type, we need to consider
Int128 as a container type for AArch64.
---
clang/include/clang/Basic/TargetInfo.h | 8 ++++++++
clang/lib/AST/RecordLayoutBuilder.cpp | 9 ++++++---
clang/lib/Basic/TargetInfo.cpp | 1 +
clang/lib/Basic/Targets/AArch64.cpp | 4 ++++
clang/test/CodeGen/aapcs64-align.cpp | 4 ++--
.../debug-info-structured-binding-bitfield.cpp | 4 ++--
6 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 070cc792ca7db..db23afa6d6f0b 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -199,6 +199,10 @@ struct TransferrableTargetInfo {
/// zero length bitfield, regardless of the zero length bitfield type.
unsigned ZeroLengthBitfieldBoundary;
+ /// The largest container size which should be used for an over-sized
+ /// bitfield, in bits.
+ unsigned LargestOverSizedBitfieldContainer;
+
/// If non-zero, specifies a maximum alignment to truncate alignment
/// specified in the aligned attribute of a static variable to this value.
unsigned MaxAlignedAttribute;
@@ -954,6 +958,10 @@ class TargetInfo : public TransferrableTargetInfo,
return ZeroLengthBitfieldBoundary;
}
+ unsigned getLargestOverSizedBitfieldContainer() const {
+ return LargestOverSizedBitfieldContainer;
+ }
+
/// Get the maximum alignment in bits for a static variable with
/// aligned attribute.
unsigned getMaxAlignedAttribute() const { return MaxAlignedAttribute; }
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index ae6d299024c6d..c461c3a12c8fd 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1469,15 +1469,18 @@ void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
// sizeof(T')*8 <= n.
QualType IntegralPODTypes[] = {
- Context.UnsignedCharTy, Context.UnsignedShortTy, Context.UnsignedIntTy,
- Context.UnsignedLongTy, Context.UnsignedLongLongTy
+ Context.UnsignedCharTy, Context.UnsignedShortTy,
+ Context.UnsignedIntTy, Context.UnsignedLongTy,
+ Context.UnsignedLongLongTy, Context.UnsignedInt128Ty,
};
QualType Type;
+ uint64_t MaxSize =
+ Context.getTargetInfo().getLargestOverSizedBitfieldContainer();
for (const QualType &QT : IntegralPODTypes) {
uint64_t Size = Context.getTypeSize(QT);
- if (Size > FieldSize)
+ if (Size > FieldSize || Size > MaxSize)
break;
Type = QT;
diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
index c0bf4e686cf03..0699ec686e4e6 100644
--- a/clang/lib/Basic/TargetInfo.cpp
+++ b/clang/lib/Basic/TargetInfo.cpp
@@ -141,6 +141,7 @@ TargetInfo::TargetInfo(const llvm::Triple &T) : Triple(T) {
UseLeadingZeroLengthBitfield = true;
UseExplicitBitFieldAlignment = true;
ZeroLengthBitfieldBoundary = 0;
+ LargestOverSizedBitfieldContainer = 64;
MaxAlignedAttribute = 0;
HalfFormat = &llvm::APFloat::IEEEhalf();
FloatFormat = &llvm::APFloat::IEEEsingle();
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index fad8d773bfc52..3633bab6e0df9 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -261,6 +261,10 @@ AArch64TargetInfo::AArch64TargetInfo(const llvm::Triple &Triple,
assert(UseBitFieldTypeAlignment && "bitfields affect type alignment");
UseZeroLengthBitfieldAlignment = true;
+ // AAPCS64 allows any "fundamental integer data type" to be used for
+ // over-sized bitfields, which includes 128-bit integers.
+ LargestOverSizedBitfieldContainer = 128;
+
HasUnalignedAccess = true;
// AArch64 targets default to using the ARM C++ ABI.
diff --git a/clang/test/CodeGen/aapcs64-align.cpp b/clang/test/CodeGen/aapcs64-align.cpp
index e9d22135836e4..740b7e1cb7a69 100644
--- a/clang/test/CodeGen/aapcs64-align.cpp
+++ b/clang/test/CodeGen/aapcs64-align.cpp
@@ -8,9 +8,9 @@ extern "C" {
// CHECK: @sizeof_OverSizedBitfield ={{.*}} global i32 8
// CHECK: @alignof_OverSizedBitfield ={{.*}} global i32 8
// CHECK: @sizeof_VeryOverSizedBitfield ={{.*}} global i32 16
-// CHECK: @alignof_VeryOverSizedBitfield ={{.*}} global i32 8
+// CHECK: @alignof_VeryOverSizedBitfield ={{.*}} global i32 16
// CHECK: @sizeof_RidiculouslyOverSizedBitfield ={{.*}} global i32 32
-// CHECK: @alignof_RidiculouslyOverSizedBitfield ={{.*}} global i32 8
+// CHECK: @alignof_RidiculouslyOverSizedBitfield ={{.*}} global i32 16
// Base case, nothing interesting.
struct S {
diff --git a/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp b/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp
index e475f032f5ce3..b7aad6a5bcd21 100644
--- a/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp
+++ b/clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp
@@ -248,8 +248,8 @@ struct S15 {
};
// CHECK-LABEL: define dso_local void @_Z4fS15v
-// CHECK: alloca %struct.S15, align 8
-// CHECK-NEXT: [[TMP0:%.*]] = alloca %struct.S15, align 8
+// CHECK: alloca %struct.S15, align 16
+// CHECK-NEXT: [[TMP0:%.*]] = alloca %struct.S15, align 16
// CHECK: #dbg_declare(ptr [[TMP0]], [[S15_A:![0-9]+]], !DIExpression(DW_OP_LLVM_extract_bits_sext, 0, 32),
// CHECK-NEXT: #dbg_declare(ptr [[TMP0]], [[S15_B:![0-9]+]], !DIExpression(DW_OP_plus_uconst, 16, DW_OP_LLVM_extract_bits_zext, 0, 32),
//
>From 01b4a28226fbbc842a67720c96e02fadb96844b8 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Tue, 11 Feb 2025 17:43:49 +0000
Subject: [PATCH 3/4] [ARM,AArch64] Over-sized bitfields affect unadjusted
alignment
The container type picked for an over-sized bitfield already contributes
to the alignment of the structure, but it should also contribute to the
"unadjusted alignment" which is used by the ARM and AArch64 PCS.
---
clang/lib/AST/RecordLayoutBuilder.cpp | 1 +
clang/test/CodeGen/aapcs-align.cpp | 8 ++++----
clang/test/CodeGen/aapcs64-align.cpp | 4 ++--
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index c461c3a12c8fd..d9380c4b6ae96 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1523,6 +1523,7 @@ void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
setSize(std::max(getSizeInBits(), getDataSizeInBits()));
// Remember max struct/class alignment.
+ UnadjustedAlignment = std::max(UnadjustedAlignment, TypeAlign);
UpdateAlignment(TypeAlign);
}
diff --git a/clang/test/CodeGen/aapcs-align.cpp b/clang/test/CodeGen/aapcs-align.cpp
index 01b5ea5a69fdc..c7bc5ba0bbfef 100644
--- a/clang/test/CodeGen/aapcs-align.cpp
+++ b/clang/test/CodeGen/aapcs-align.cpp
@@ -154,8 +154,8 @@ unsigned sizeof_OverSizedBitfield = sizeof(OverSizedBitfield);
unsigned alignof_OverSizedBitfield = alignof(OverSizedBitfield);
// CHECK: define{{.*}} void @g7
-// CHECK: call void @f7(i32 noundef 1, [2 x i32] [i32 42, i32 0])
-// CHECK: declare void @f7(i32 noundef, [2 x i32])
+// CHECK: call void @f7(i32 noundef 1, [1 x i64] [i64 42])
+// CHECK: declare void @f7(i32 noundef, [1 x i64])
void f7(int a, OverSizedBitfield b);
void g7() {
OverSizedBitfield s = {42};
@@ -173,8 +173,8 @@ unsigned sizeof_VeryOverSizedBitfield = sizeof(VeryOverSizedBitfield);
unsigned alignof_VeryOverSizedBitfield = alignof(VeryOverSizedBitfield);
// CHECK: define{{.*}} void @g8
-// CHECK: call void @f8(i32 noundef 1, [4 x i32] [i32 42, i32 0, i32 0, i32 0])
-// CHECK: declare void @f8(i32 noundef, [4 x i32])
+// CHECK: call void @f8(i32 noundef 1, [2 x i64] [i64 42, i64 0])
+// CHECK: declare void @f8(i32 noundef, [2 x i64])
void f8(int a, VeryOverSizedBitfield b);
void g8() {
VeryOverSizedBitfield s = {42};
diff --git a/clang/test/CodeGen/aapcs64-align.cpp b/clang/test/CodeGen/aapcs64-align.cpp
index 740b7e1cb7a69..9578772dace75 100644
--- a/clang/test/CodeGen/aapcs64-align.cpp
+++ b/clang/test/CodeGen/aapcs64-align.cpp
@@ -197,8 +197,8 @@ unsigned sizeof_VeryOverSizedBitfield = sizeof(VeryOverSizedBitfield);
unsigned alignof_VeryOverSizedBitfield = alignof(VeryOverSizedBitfield);
// CHECK: define{{.*}} void @g8
-// CHECK: call void @f8(i32 noundef 1, [2 x i64] [i64 42, i64 0])
-// CHECK: declare void @f8(i32 noundef, [2 x i64])
+// CHECK: call void @f8(i32 noundef 1, i128 42)
+// CHECK: declare void @f8(i32 noundef, i128)
void f8(int a, VeryOverSizedBitfield b);
void g8() {
VeryOverSizedBitfield s = {42};
>From 8ce77cc5498093f60b27358fc6cc199824a10cc1 Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Thu, 20 Feb 2025 13:54:19 +0000
Subject: [PATCH 4/4] Update comment in test
---
clang/test/CodeGen/aapcs64-align.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/clang/test/CodeGen/aapcs64-align.cpp b/clang/test/CodeGen/aapcs64-align.cpp
index 9578772dace75..e69faf231936c 100644
--- a/clang/test/CodeGen/aapcs64-align.cpp
+++ b/clang/test/CodeGen/aapcs64-align.cpp
@@ -186,9 +186,9 @@ void g7() {
f7(1, s);
}
-// There are no 128-bit fundamental data types defined by AAPCS32, so this gets
-// a 64-bit container plus 64 bits of padding, giving it a size of 16 bytes and
-// alignment of 8 bytes.
+// AAPCS64 does have a 128-bit integer fundamental data type, so this gets a
+// 128-bit container with 128-bit alignment. This is just within the limit of
+// what can be passed directly.
struct VeryOverSizedBitfield {
int x : 128;
};
More information about the cfe-commits
mailing list