Skip to content

Commit adaf62c

Browse files
ahatanakaahatanaka
committed
[Sema] Reject array element types whose sizes aren't a multiple of their
alignments In the following code, the first element is aligned on a 16-byte boundary, but the remaining elements aren't: ``` typedef char int8_a16 __attribute__((aligned(16))); int8_a16 array[4]; ``` Currently clang doesn't reject the code, but it should since it can cause crashes at runtime. This patch also fixes assertion failures in CodeGen caused by the changes in https://reviews.llvm.org/D123649. Differential Revision: https://reviews.llvm.org/D133711
1 parent f47a5df commit adaf62c

15 files changed

+119
-49
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ code bases.
7070
results do not change before/after setting
7171
``-Werror=incompatible-function-pointer-types`` to avoid incompatibility with
7272
Clang 16.
73+
- Clang now disallows types whose sizes aren't a multiple of their alignments to
74+
be used as the element type of arrays.
7375

7476
.. code-block:: c
7577

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7703,6 +7703,8 @@ def warn_overaligned_type : Warning<
77037703
"type %0 requires %1 bytes of alignment and the default allocator only "
77047704
"guarantees %2 bytes">,
77057705
InGroup<OveralignedType>, DefaultIgnore;
7706+
def err_array_element_alignment : Error<
7707+
"size of array element of type %0 (%1 bytes) isn't a multiple of its alignment (%2 bytes)">;
77067708
def err_aligned_allocation_unavailable : Error<
77077709
"aligned %select{allocation|deallocation}0 function of type '%1' is "
77087710
"%select{only|not}4 available on %2%select{ %3 or newer|}4">;

‎clang/include/clang/Sema/Sema.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2442,6 +2442,10 @@ class Sema final {
24422442

24432443
bool isUsualDeallocationFunction(const CXXMethodDecl *FD);
24442444

2445+
// Check whether the size of array element of type \p EltTy is a multiple of
2446+
// its alignment and return false if it isn't.
2447+
bool checkArrayElementAlignment(QualType EltTy, SourceLocation Loc);
2448+
24452449
bool isCompleteType(SourceLocation Loc, QualType T,
24462450
CompleteTypeKind Kind = CompleteTypeKind::Default) {
24472451
return !RequireCompleteTypeImpl(Loc, T, Kind, nullptr);

‎clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2449,6 +2449,8 @@ bool Sema::CheckAllocatedType(QualType AllocType, SourceLocation Loc,
24492449
else if (RequireNonAbstractType(Loc, AllocType,
24502450
diag::err_allocation_of_abstract_type))
24512451
return true;
2452+
else if (!checkArrayElementAlignment(AllocType, Loc))
2453+
return true;
24522454
else if (AllocType->isVariablyModifiedType())
24532455
return Diag(Loc, diag::err_variably_modified_new_type)
24542456
<< AllocType;

‎clang/lib/Sema/SemaType.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2394,6 +2394,23 @@ static ExprResult checkArraySize(Sema &S, Expr *&ArraySize,
23942394
return R;
23952395
}
23962396

2397+
bool Sema::checkArrayElementAlignment(QualType EltTy, SourceLocation Loc) {
2398+
EltTy = Context.getBaseElementType(EltTy);
2399+
if (EltTy->isIncompleteType() || EltTy->isDependentType() ||
2400+
EltTy->isUndeducedType())
2401+
return true;
2402+
2403+
CharUnits Size = Context.getTypeSizeInChars(EltTy);
2404+
CharUnits Alignment = Context.getTypeAlignInChars(EltTy);
2405+
2406+
if (Size.isMultipleOf(Alignment))
2407+
return true;
2408+
2409+
Diag(Loc, diag::err_array_element_alignment)
2410+
<< EltTy << Size.getQuantity() << Alignment.getQuantity();
2411+
return false;
2412+
}
2413+
23972414
/// Build an array type.
23982415
///
23992416
/// \param T The type of each element in the array.
@@ -2477,6 +2494,9 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM,
24772494
return QualType();
24782495
}
24792496

2497+
if (!checkArrayElementAlignment(T, Loc))
2498+
return QualType();
2499+
24802500
// Do placeholder conversions on the array size expression.
24812501
if (ArraySize && ArraySize->hasPlaceholderType()) {
24822502
ExprResult Result = CheckPlaceholderExpr(ArraySize);

‎clang/test/CodeGenCXX/override-layout-packed-base.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,6 @@ class D : virtual B<0>, virtual B<1> {
3535
//#pragma pack(pop)
3636

3737
void use_structs() {
38-
C cs[sizeof(C)];
39-
D ds[sizeof(D)];
38+
C cs;
39+
D ds;
4040
}

‎clang/test/CodeGenCXX/ubsan-new-checks.cpp

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ struct S4 : public S3 {
1818
S4() : S3() {}
1919
};
2020

21-
typedef __attribute__((ext_vector_type(2), aligned(32))) float float32x2_t;
21+
typedef __attribute__((ext_vector_type(8), aligned(32))) float float32x8_t;
2222

2323
struct S5 {
24-
float32x2_t x;
24+
float32x8_t x;
2525
};
2626

2727
void *operator new (unsigned long, void *p) { return p; }
@@ -54,21 +54,21 @@ S2 *func_03() {
5454
return new S2[20];
5555
}
5656

57-
float32x2_t *func_04() {
57+
float32x8_t *func_04() {
5858
// CHECK-LABEL: define {{.*}} @_Z7func_04v
5959
// CHECK: and i64 %{{.*}}, 31, !nosanitize
6060
// CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
61-
// CHECK: ret <2 x float>*
62-
return new float32x2_t;
61+
// CHECK: ret <8 x float>*
62+
return new float32x8_t;
6363
}
6464

65-
float32x2_t *func_05() {
65+
float32x8_t *func_05() {
6666
// CHECK-LABEL: define {{.*}} @_Z7func_05v
6767
// CHECK: and i64 %{{.*}}, 31, !nosanitize
6868
// CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
6969
// CHECK-NOT: and i64 %{{.*}}, 31
70-
// CHECK: ret <2 x float>*
71-
return new float32x2_t[20];
70+
// CHECK: ret <8 x float>*
71+
return new float32x8_t[20];
7272
}
7373

7474
S3 *func_07() {
@@ -110,21 +110,21 @@ S2 *func_11(void *p) {
110110
return new(p) S2[10];
111111
}
112112

113-
float32x2_t *func_12() {
113+
float32x8_t *func_12() {
114114
// CHECK-LABEL: define {{.*}} @_Z7func_12v
115115
// CHECK: and i64 %{{.*}}, 31, !nosanitize
116116
// CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
117-
// CHECK: ret <2 x float>*
118-
return new float32x2_t;
117+
// CHECK: ret <8 x float>*
118+
return new float32x8_t;
119119
}
120120

121-
float32x2_t *func_13() {
121+
float32x8_t *func_13() {
122122
// CHECK-LABEL: define {{.*}} @_Z7func_13v
123123
// CHECK: and i64 %{{.*}}, 31, !nosanitize
124124
// CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
125125
// CHECK-NOT: and i64 %{{.*}}, 31
126-
// CHECK: ret <2 x float>*
127-
return new float32x2_t[20];
126+
// CHECK: ret <8 x float>*
127+
return new float32x8_t[20];
128128
}
129129

130130
S4 *func_14() {

‎clang/test/Layout/ms-aligned-array.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66
// FIXME: What about other type sugar, like _Atomic? This would only matter in a
77
// packed struct context.
88
struct __declspec(align(16)) AlignedStruct { int x; };
9-
typedef int __declspec(align(16)) AlignedInt;
9+
struct Struct2 {
10+
char c[16];
11+
};
12+
typedef struct Struct2 __declspec(align(16)) AlignedStruct2;
1013

1114
#define CHECK_SIZE(X, Align) \
1215
_Static_assert(__alignof(struct X) == Align, "should be aligned");
@@ -20,13 +23,13 @@ CHECK_SIZE(A, 16);
2023

2124
struct B {
2225
char b;
23-
AlignedInt a[1];
26+
AlignedStruct2 a[1];
2427
};
2528
CHECK_SIZE(B, 16);
2629

2730
struct C {
2831
char b;
29-
AlignedInt a[];
32+
AlignedStruct2 a[];
3033
};
3134
CHECK_SIZE(C, 16);
3235

@@ -39,14 +42,18 @@ CHECK_SIZE(C, 16);
3942
// CHECK-NEXT: 0 | struct AlignedStruct[1] a
4043
// CHECK-NEXT: | [sizeof=16, align=16]
4144
// CHECK: *** Dumping AST Record Layout
45+
// CHECK-NEXT: 0 | struct Struct2
46+
// CHECK-NEXT: 0 | char[16] c
47+
// CHECK-NEXT: | [sizeof=16, align=1]
48+
// CHECK: *** Dumping AST Record Layout
4249
// CHECK-NEXT: 0 | struct B
4350
// CHECK-NEXT: 0 | char b
44-
// CHECK-NEXT: 16 | AlignedInt[1] a
51+
// CHECK-NEXT: 16 | AlignedStruct2[1] a
4552
// CHECK-NEXT: | [sizeof=32, align=16]
4653
// CHECK: *** Dumping AST Record Layout
4754
// CHECK-NEXT: 0 | struct C
4855
// CHECK-NEXT: 0 | char b
49-
// CHECK-NEXT: 16 | AlignedInt[] a
56+
// CHECK-NEXT: 16 | AlignedStruct2[] a
5057
// CHECK-NEXT: | [sizeof=16, align=16]
5158

5259
#pragma pack(pop)

‎clang/test/Layout/ms-x86-misalignedarray.cpp

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,15 @@
1-
// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple i686-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>&1 \
2-
// RUN: | FileCheck %s
1+
// RUN: %clang_cc1 -fno-rtti -triple i686-pc-win32 -fsyntax-only -verify %s
32
// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>/dev/null \
43
// RUN: | FileCheck %s -check-prefix CHECK-X64
54

65
struct T0 { char c; };
76
struct T2 : virtual T0 { };
87
struct T3 { T2 a[1]; char c; };
98

10-
// CHECK: *** Dumping AST Record Layout
11-
// CHECK: *** Dumping AST Record Layout
12-
// CHECK: *** Dumping AST Record Layout
13-
// CHECK-NEXT: 0 | struct T3
14-
// CHECK-NEXT: 0 | T2[1] a
15-
// CHECK-NEXT: 5 | char c
16-
// CHECK-NEXT: | [sizeof=8, align=4
17-
// CHECK-NEXT: | nvsize=8, nvalign=4]
9+
#ifdef _ILP32
10+
// expected-error@-3 {{size of array element}}
11+
#endif
12+
1813
// CHECK-X64: *** Dumping AST Record Layout
1914
// CHECK-X64: *** Dumping AST Record Layout
2015
// CHECK-X64: *** Dumping AST Record Layout

‎clang/test/Sema/align-x86-64.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -verify %s
2-
// expected-no-diagnostics
32

43
// PR5637
54

65
typedef __attribute__((aligned(16))) struct {
76
unsigned long long w[3];
87
} UINT192;
98

10-
UINT192 ten2mk192M[] = {
9+
UINT192 ten2mk192M[] = { // expected-error {{size of array element}}
1110
{{0xcddd6e04c0592104ULL, 0x0fcf80dc33721d53ULL, 0xa7c5ac471b478423ULL}},
1211
{{0xcddd6e04c0592104ULL, 0x0fcf80dc33721d53ULL, 0xa7c5ac471b478423ULL}},
1312
{{0xcddd6e04c0592104ULL, 0x0fcf80dc33721d53ULL, 0xa7c5ac471b478423ULL}}

‎clang/test/Sema/align-x86.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
// RUN: %clang_cc1 -std=c11 -triple i386-apple-darwin9 -fsyntax-only -verify %s
2-
// expected-no-diagnostics
32

43
#define STATIC_ASSERT(cond) _Static_assert(cond, #cond)
54

@@ -38,10 +37,10 @@ typedef ALIGNED(2) struct {
3837
} aligned_before_struct;
3938

4039
STATIC_ASSERT(sizeof(aligned_before_struct) == 3);
41-
STATIC_ASSERT(sizeof(aligned_before_struct[1]) == 4);
42-
STATIC_ASSERT(sizeof(aligned_before_struct[2]) == 6);
43-
STATIC_ASSERT(sizeof(aligned_before_struct[2][1]) == 8);
44-
STATIC_ASSERT(sizeof(aligned_before_struct[1][2]) == 6);
40+
STATIC_ASSERT(sizeof(aligned_before_struct[1]) == 4); // expected-error {{size of array element}}
41+
STATIC_ASSERT(sizeof(aligned_before_struct[2]) == 6); // expected-error {{size of array element}}
42+
STATIC_ASSERT(sizeof(aligned_before_struct[2][1]) == 8); // expected-error {{size of array element}}
43+
STATIC_ASSERT(sizeof(aligned_before_struct[1][2]) == 6); // expected-error {{size of array element}}
4544

4645
typedef struct ALIGNED(2) {
4746
char a[3];

‎clang/test/Sema/attr-aligned.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,14 @@ struct E e;
4949
char e1[__alignof__(e) == 2 ?: -1] = {0};
5050
char e2[__alignof__(e.member) == 2 ?: -1] = {0};
5151

52-
typedef char overaligned_char __attribute__((aligned(16)));
53-
typedef overaligned_char array_with_overaligned_char[11];
52+
typedef struct { char c[16]; } S;
53+
typedef S overaligned_struct __attribute__((aligned(16)));
54+
typedef overaligned_struct array_with_overaligned_struct[11];
5455
typedef char array_with_align_attr[11] __attribute__((aligned(16)));
5556

56-
char f0[__alignof__(array_with_overaligned_char) == 16 ? 1 : -1] = { 0 };
57+
char f0[__alignof__(array_with_overaligned_struct) == 16 ? 1 : -1] = { 0 };
5758
char f1[__alignof__(array_with_align_attr) == 16 ? 1 : -1] = { 0 };
58-
array_with_overaligned_char F2;
59+
array_with_overaligned_struct F2;
5960
char f2[__alignof__(F2) == 16 ? 1 : -1] = { 0 };
6061
array_with_align_attr F3;
6162
char f3[__alignof__(F3) == 16 ? 1 : -1] = { 0 };

‎clang/test/SemaCXX/align-x86.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
// RUN: %clang_cc1 -std=c++11 -triple i386-apple-darwin9 -fsyntax-only -verify %s
2-
// expected-no-diagnostics
32

43
using size_t = decltype(sizeof(0));
54

@@ -46,10 +45,10 @@ typedef ALIGNED(2) struct {
4645
} aligned_before_struct;
4746

4847
static_assert(sizeof(aligned_before_struct) == 3, "");
49-
static_assert(sizeof(aligned_before_struct[1]) == 4, "");
50-
static_assert(sizeof(aligned_before_struct[2]) == 6, "");
51-
static_assert(sizeof(aligned_before_struct[2][1]) == 8, "");
52-
static_assert(sizeof(aligned_before_struct[1][2]) == 6, "");
48+
static_assert(sizeof(aligned_before_struct[1]) == 4, ""); // expected-error {{size of array element}}
49+
static_assert(sizeof(aligned_before_struct[2]) == 6, ""); // expected-error {{size of array element}}
50+
static_assert(sizeof(aligned_before_struct[2][1]) == 8, ""); // expected-error {{size of array element}}
51+
static_assert(sizeof(aligned_before_struct[1][2]) == 6, ""); // expected-error {{size of array element}}
5352

5453
typedef struct ALIGNED(2) {
5554
char a[3];

‎clang/test/SemaCXX/array-alignment.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// RUN: %clang_cc1 -fsyntax-only -verify %s
2+
3+
typedef char __attribute__((aligned(2))) AlignedChar;
4+
typedef AlignedChar arrayType0[4]; // expected-error {{size of array element}}
5+
6+
struct __attribute__((aligned(8))) AlignedStruct {
7+
int m0;
8+
};
9+
10+
struct __attribute__((packed)) PackedStruct {
11+
char m0;
12+
int i0;
13+
};
14+
15+
typedef PackedStruct AlignedPackedStruct __attribute__((aligned(4)));
16+
typedef AlignedPackedStruct arrayType1[4]; // expected-error {{(5 bytes) isn't a multiple of its alignment (4 bytes)}}
17+
18+
AlignedChar a0[1]; // expected-error {{size of array element}}
19+
AlignedStruct a1[1];
20+
AlignedPackedStruct a2[1]; // expected-error {{size of array element}}
21+
22+
struct S {
23+
AlignedChar m0[1]; // expected-error {{size of array element}}
24+
AlignedStruct m1[1];
25+
AlignedPackedStruct m2[1]; // expected-error {{size of array element}}
26+
};
27+
28+
void test(char *p) {
29+
auto p0 = (AlignedChar(*)[1])p; // expected-error {{size of array element}}
30+
auto r0 = (AlignedChar(&)[1])(*p); // expected-error {{size of array element}}
31+
auto p1 = new AlignedChar[1]; // expected-error {{size of array element}}
32+
auto p2 = (AlignedStruct(*)[1])p;
33+
auto p3 = new AlignedStruct[1];
34+
auto p4 = (AlignedPackedStruct(*)[1])p; // expected-error {{size of array element}}
35+
auto p5 = new AlignedPackedStruct[1]; // expected-error {{size of array element}}
36+
}

‎clang/test/SemaCXX/warn-new-overaligned.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,13 @@ void helper() {
1919
}
2020

2121
namespace test2 {
22+
struct S {
23+
char c[256];
24+
};
25+
2226
class Test {
23-
typedef int __attribute__((aligned(256))) aligned_int;
24-
aligned_int high_contention_data[10];
27+
typedef S __attribute__((aligned(256))) alignedS;
28+
alignedS high_contention_data[10];
2529
};
2630

2731
void helper() {

0 commit comments

Comments
 (0)