Skip to content

Commit 02e8fd7

Browse files
authored
[ARM,AArch64] Fix ABI bugs with over-sized bitfields (#126774)
This fixes two bugs in the ABI for over-sized bitfields for ARM and AArch64: 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. 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.
1 parent be51ef4 commit 02e8fd7

File tree

7 files changed

+129
-5
lines changed

7 files changed

+129
-5
lines changed

clang/include/clang/Basic/TargetInfo.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,10 @@ struct TransferrableTargetInfo {
199199
/// zero length bitfield, regardless of the zero length bitfield type.
200200
unsigned ZeroLengthBitfieldBoundary;
201201

202+
/// The largest container size which should be used for an over-sized
203+
/// bitfield, in bits.
204+
unsigned LargestOverSizedBitfieldContainer;
205+
202206
/// If non-zero, specifies a maximum alignment to truncate alignment
203207
/// specified in the aligned attribute of a static variable to this value.
204208
unsigned MaxAlignedAttribute;
@@ -954,6 +958,10 @@ class TargetInfo : public TransferrableTargetInfo,
954958
return ZeroLengthBitfieldBoundary;
955959
}
956960

961+
unsigned getLargestOverSizedBitfieldContainer() const {
962+
return LargestOverSizedBitfieldContainer;
963+
}
964+
957965
/// Get the maximum alignment in bits for a static variable with
958966
/// aligned attribute.
959967
unsigned getMaxAlignedAttribute() const { return MaxAlignedAttribute; }

clang/lib/AST/RecordLayoutBuilder.cpp

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,15 +1469,18 @@ void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
14691469
// sizeof(T')*8 <= n.
14701470

14711471
QualType IntegralPODTypes[] = {
1472-
Context.UnsignedCharTy, Context.UnsignedShortTy, Context.UnsignedIntTy,
1473-
Context.UnsignedLongTy, Context.UnsignedLongLongTy
1472+
Context.UnsignedCharTy, Context.UnsignedShortTy,
1473+
Context.UnsignedIntTy, Context.UnsignedLongTy,
1474+
Context.UnsignedLongLongTy, Context.UnsignedInt128Ty,
14741475
};
14751476

14761477
QualType Type;
1478+
uint64_t MaxSize =
1479+
Context.getTargetInfo().getLargestOverSizedBitfieldContainer();
14771480
for (const QualType &QT : IntegralPODTypes) {
14781481
uint64_t Size = Context.getTypeSize(QT);
14791482

1480-
if (Size > FieldSize)
1483+
if (Size > FieldSize || Size > MaxSize)
14811484
break;
14821485

14831486
Type = QT;
@@ -1520,6 +1523,7 @@ void ItaniumRecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
15201523
setSize(std::max(getSizeInBits(), getDataSizeInBits()));
15211524

15221525
// Remember max struct/class alignment.
1526+
UnadjustedAlignment = std::max(UnadjustedAlignment, TypeAlign);
15231527
UpdateAlignment(TypeAlign);
15241528
}
15251529

clang/lib/Basic/TargetInfo.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ TargetInfo::TargetInfo(const llvm::Triple &T) : Triple(T) {
141141
UseLeadingZeroLengthBitfield = true;
142142
UseExplicitBitFieldAlignment = true;
143143
ZeroLengthBitfieldBoundary = 0;
144+
LargestOverSizedBitfieldContainer = 64;
144145
MaxAlignedAttribute = 0;
145146
HalfFormat = &llvm::APFloat::IEEEhalf();
146147
FloatFormat = &llvm::APFloat::IEEEsingle();

clang/lib/Basic/Targets/AArch64.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,10 @@ AArch64TargetInfo::AArch64TargetInfo(const llvm::Triple &Triple,
261261
assert(UseBitFieldTypeAlignment && "bitfields affect type alignment");
262262
UseZeroLengthBitfieldAlignment = true;
263263

264+
// AAPCS64 allows any "fundamental integer data type" to be used for
265+
// over-sized bitfields, which includes 128-bit integers.
266+
LargestOverSizedBitfieldContainer = 128;
267+
264268
HasUnalignedAccess = true;
265269

266270
// AArch64 targets default to using the ARM C++ ABI.

clang/test/CodeGen/aapcs-align.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66

77
extern "C" {
88

9+
// CHECK: @sizeof_OverSizedBitfield ={{.*}} global i32 8
10+
// CHECK: @alignof_OverSizedBitfield ={{.*}} global i32 8
11+
// CHECK: @sizeof_VeryOverSizedBitfield ={{.*}} global i32 16
12+
// CHECK: @alignof_VeryOverSizedBitfield ={{.*}} global i32 8
13+
914
// Base case, nothing interesting.
1015
struct S {
1116
int x, y;
@@ -138,4 +143,42 @@ void g6() {
138143
// 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])
139144
// CHECK: declare void @f6(i32 noundef, [4 x i32])
140145
// CHECK: declare void @f6m(i32 noundef, i32 noundef, i32 noundef, i32 noundef, i32 noundef, [4 x i32])
146+
147+
// Over-sized bitfield, which results in a 64-bit container type, so 64-bit
148+
// alignment.
149+
struct OverSizedBitfield {
150+
int x : 64;
151+
};
152+
153+
unsigned sizeof_OverSizedBitfield = sizeof(OverSizedBitfield);
154+
unsigned alignof_OverSizedBitfield = alignof(OverSizedBitfield);
155+
156+
// CHECK: define{{.*}} void @g7
157+
// CHECK: call void @f7(i32 noundef 1, [1 x i64] [i64 42])
158+
// CHECK: declare void @f7(i32 noundef, [1 x i64])
159+
void f7(int a, OverSizedBitfield b);
160+
void g7() {
161+
OverSizedBitfield s = {42};
162+
f7(1, s);
163+
}
164+
165+
// There are no 128-bit fundamental data types defined by AAPCS32, so this gets
166+
// a 64-bit container plus 64 bits of padding, giving it a size of 16 bytes and
167+
// alignment of 8 bytes.
168+
struct VeryOverSizedBitfield {
169+
int x : 128;
170+
};
171+
172+
unsigned sizeof_VeryOverSizedBitfield = sizeof(VeryOverSizedBitfield);
173+
unsigned alignof_VeryOverSizedBitfield = alignof(VeryOverSizedBitfield);
174+
175+
// CHECK: define{{.*}} void @g8
176+
// CHECK: call void @f8(i32 noundef 1, [2 x i64] [i64 42, i64 0])
177+
// CHECK: declare void @f8(i32 noundef, [2 x i64])
178+
void f8(int a, VeryOverSizedBitfield b);
179+
void g8() {
180+
VeryOverSizedBitfield s = {42};
181+
f8(1, s);
182+
}
183+
141184
}

clang/test/CodeGen/aapcs64-align.cpp

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55

66
extern "C" {
77

8+
// CHECK: @sizeof_OverSizedBitfield ={{.*}} global i32 8
9+
// CHECK: @alignof_OverSizedBitfield ={{.*}} global i32 8
10+
// CHECK: @sizeof_VeryOverSizedBitfield ={{.*}} global i32 16
11+
// CHECK: @alignof_VeryOverSizedBitfield ={{.*}} global i32 16
12+
// CHECK: @sizeof_RidiculouslyOverSizedBitfield ={{.*}} global i32 32
13+
// CHECK: @alignof_RidiculouslyOverSizedBitfield ={{.*}} global i32 16
14+
815
// Base case, nothing interesting.
916
struct S {
1017
long x, y;
@@ -161,5 +168,62 @@ int test_bitint8(){
161168
}
162169
// CHECK: ret i32 1
163170

171+
// Over-sized bitfield, which results in a 64-bit container type, so 64-bit
172+
// alignment.
173+
struct OverSizedBitfield {
174+
int x : 64;
175+
};
176+
177+
unsigned sizeof_OverSizedBitfield = sizeof(OverSizedBitfield);
178+
unsigned alignof_OverSizedBitfield = alignof(OverSizedBitfield);
179+
180+
// CHECK: define{{.*}} void @g7
181+
// CHECK: call void @f7(i32 noundef 1, i64 42)
182+
// CHECK: declare void @f7(i32 noundef, i64)
183+
void f7(int a, OverSizedBitfield b);
184+
void g7() {
185+
OverSizedBitfield s = {42};
186+
f7(1, s);
187+
}
188+
189+
// AAPCS64 does have a 128-bit integer fundamental data type, so this gets a
190+
// 128-bit container with 128-bit alignment. This is just within the limit of
191+
// what can be passed directly.
192+
struct VeryOverSizedBitfield {
193+
int x : 128;
194+
};
195+
196+
unsigned sizeof_VeryOverSizedBitfield = sizeof(VeryOverSizedBitfield);
197+
unsigned alignof_VeryOverSizedBitfield = alignof(VeryOverSizedBitfield);
198+
199+
// CHECK: define{{.*}} void @g8
200+
// CHECK: call void @f8(i32 noundef 1, i128 42)
201+
// CHECK: declare void @f8(i32 noundef, i128)
202+
void f8(int a, VeryOverSizedBitfield b);
203+
void g8() {
204+
VeryOverSizedBitfield s = {42};
205+
f8(1, s);
206+
}
207+
208+
// There are no bigger fundamental data types, so this gets a 128-bit container
209+
// and 128 bits of padding, giving the struct a size of 32 bytes, and an
210+
// alignment of 16 bytes. This is over the PCS size limit of 16 bytes, so it
211+
// will be passed indirectly.
212+
struct RidiculouslyOverSizedBitfield {
213+
int x : 256;
214+
};
215+
216+
unsigned sizeof_RidiculouslyOverSizedBitfield = sizeof(RidiculouslyOverSizedBitfield);
217+
unsigned alignof_RidiculouslyOverSizedBitfield = alignof(RidiculouslyOverSizedBitfield);
218+
219+
// CHECK: define{{.*}} void @g9
220+
// CHECK: call void @f9(i32 noundef 1, ptr noundef nonnull %agg.tmp)
221+
// CHECK: declare void @f9(i32 noundef, ptr noundef)
222+
void f9(int a, RidiculouslyOverSizedBitfield b);
223+
void g9() {
224+
RidiculouslyOverSizedBitfield s = {42};
225+
f9(1, s);
226+
}
227+
164228
}
165229

clang/test/CodeGenCXX/debug-info-structured-binding-bitfield.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,8 +248,8 @@ struct S15 {
248248
};
249249

250250
// CHECK-LABEL: define dso_local void @_Z4fS15v
251-
// CHECK: alloca %struct.S15, align 8
252-
// CHECK-NEXT: [[TMP0:%.*]] = alloca %struct.S15, align 8
251+
// CHECK: alloca %struct.S15, align 16
252+
// CHECK-NEXT: [[TMP0:%.*]] = alloca %struct.S15, align 16
253253
// CHECK: #dbg_declare(ptr [[TMP0]], [[S15_A:![0-9]+]], !DIExpression(DW_OP_LLVM_extract_bits_sext, 0, 32),
254254
// CHECK-NEXT: #dbg_declare(ptr [[TMP0]], [[S15_B:![0-9]+]], !DIExpression(DW_OP_plus_uconst, 16, DW_OP_LLVM_extract_bits_zext, 0, 32),
255255
//

0 commit comments

Comments
 (0)