[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

clang/docs/ReleaseNotes.rst

@@ -70,6 +70,8 @@ code bases.
   results do not change before/after setting
   ``-Werror=incompatible-function-pointer-types`` to avoid incompatibility with
   Clang 16.
+- Clang now disallows types whose sizes aren't a multiple of their alignments to
+  be used as the element type of arrays.
 
   .. code-block:: c
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7703,6 +7703,8 @@ def warn_overaligned_type : Warning<
   "type %0 requires %1 bytes of alignment and the default allocator only "
   "guarantees %2 bytes">,
   InGroup<OveralignedType>, DefaultIgnore;
+def err_array_element_alignment : Error<
+  "size of array element of type %0 (%1 bytes) isn't a multiple of its alignment (%2 bytes)">;
 def err_aligned_allocation_unavailable : Error<
   "aligned %select{allocation|deallocation}0 function of type '%1' is "
   "%select{only|not}4 available on %2%select{ %3 or newer|}4">;

clang/include/clang/Sema/Sema.h

@@ -2442,6 +2442,10 @@ public:
 
   bool isUsualDeallocationFunction(const CXXMethodDecl *FD);
 
+  // Check whether the size of array element of type \p EltTy is a multiple of
+  // its alignment and return false if it isn't.
+  bool checkArrayElementAlignment(QualType EltTy, SourceLocation Loc);
+
   bool isCompleteType(SourceLocation Loc, QualType T,
                       CompleteTypeKind Kind = CompleteTypeKind::Default) {
     return !RequireCompleteTypeImpl(Loc, T, Kind, nullptr);

clang/lib/Sema/SemaExprCXX.cpp

@@ -2449,6 +2449,8 @@ bool Sema::CheckAllocatedType(QualType AllocType, SourceLocation Loc,
   else if (RequireNonAbstractType(Loc, AllocType,
                                   diag::err_allocation_of_abstract_type))
     return true;
+  else if (!checkArrayElementAlignment(AllocType, Loc))
+    return true;
   else if (AllocType->isVariablyModifiedType())
     return Diag(Loc, diag::err_variably_modified_new_type)
              << AllocType;

clang/lib/Sema/SemaType.cpp

@@ -2394,6 +2394,23 @@ static ExprResult checkArraySize(Sema &S, Expr *&ArraySize,
   return R;
 }
 
+bool Sema::checkArrayElementAlignment(QualType EltTy, SourceLocation Loc) {
+  EltTy = Context.getBaseElementType(EltTy);
+  if (EltTy->isIncompleteType() || EltTy->isDependentType() ||
+      EltTy->isUndeducedType())
+    return true;
+
+  CharUnits Size = Context.getTypeSizeInChars(EltTy);
+  CharUnits Alignment = Context.getTypeAlignInChars(EltTy);
+
+  if (Size.isMultipleOf(Alignment))
+    return true;
+
+  Diag(Loc, diag::err_array_element_alignment)
+      << EltTy << Size.getQuantity() << Alignment.getQuantity();
+  return false;
+}
+
 /// Build an array type.
 ///
 /// \param T The type of each element in the array.
@@ -2477,6 +2494,9 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM,
     return QualType();
   }
 
+  if (!checkArrayElementAlignment(T, Loc))
+    return QualType();
+
   // Do placeholder conversions on the array size expression.
   if (ArraySize && ArraySize->hasPlaceholderType()) {
     ExprResult Result = CheckPlaceholderExpr(ArraySize);

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

@@ -35,6 +35,6 @@ class D : virtual B<0>, virtual B<1> {
 //#pragma pack(pop)
 
 void use_structs() {
-  C cs[sizeof(C)];
-  D ds[sizeof(D)];
+  C cs;
+  D ds;
 }

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

@@ -18,10 +18,10 @@ struct S4 : public S3 {
   S4() : S3() {}
 };
 
-typedef __attribute__((ext_vector_type(2), aligned(32))) float float32x2_t;
+typedef __attribute__((ext_vector_type(8), aligned(32))) float float32x8_t;
 
 struct S5 {
-  float32x2_t x;
+  float32x8_t x;
 };
 
 void *operator new (unsigned long, void *p) { return p; }
@@ -54,21 +54,21 @@ S2 *func_03() {
   return new S2[20];
 }
 
-float32x2_t *func_04() {
+float32x8_t *func_04() {
   // CHECK-LABEL: define {{.*}} @_Z7func_04v
   // CHECK:       and i64 %{{.*}}, 31, !nosanitize
   // CHECK:       icmp eq i64 %{{.*}}, 0, !nosanitize
-  // CHECK:       ret <2 x float>*
-  return new float32x2_t;
+  // CHECK:       ret <8 x float>*
+  return new float32x8_t;
 }
 
-float32x2_t *func_05() {
+float32x8_t *func_05() {
   // CHECK-LABEL: define {{.*}} @_Z7func_05v
   // CHECK:       and i64 %{{.*}}, 31, !nosanitize
   // CHECK:       icmp eq i64 %{{.*}}, 0, !nosanitize
   // CHECK-NOT:   and i64 %{{.*}}, 31
-  // CHECK:       ret <2 x float>*
-  return new float32x2_t[20];
+  // CHECK:       ret <8 x float>*
+  return new float32x8_t[20];
 }
 
 S3 *func_07() {
@@ -110,21 +110,21 @@ S2 *func_11(void *p) {
   return new(p) S2[10];
 }
 
-float32x2_t *func_12() {
+float32x8_t *func_12() {
   // CHECK-LABEL: define {{.*}} @_Z7func_12v
   // CHECK:       and i64 %{{.*}}, 31, !nosanitize
   // CHECK:       icmp eq i64 %{{.*}}, 0, !nosanitize
-  // CHECK:       ret <2 x float>*
-  return new float32x2_t;
+  // CHECK:       ret <8 x float>*
+  return new float32x8_t;
 }
 
-float32x2_t *func_13() {
+float32x8_t *func_13() {
   // CHECK-LABEL: define {{.*}} @_Z7func_13v
   // CHECK:       and i64 %{{.*}}, 31, !nosanitize
   // CHECK:       icmp eq i64 %{{.*}}, 0, !nosanitize
   // CHECK-NOT:   and i64 %{{.*}}, 31
-  // CHECK:       ret <2 x float>*
-  return new float32x2_t[20];
+  // CHECK:       ret <8 x float>*
+  return new float32x8_t[20];
 }
 
 S4 *func_14() {

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

@@ -6,7 +6,10 @@
 // FIXME: What about other type sugar, like _Atomic? This would only matter in a
 // packed struct context.
 struct __declspec(align(16)) AlignedStruct { int x; };
-typedef int  __declspec(align(16)) AlignedInt;
+struct Struct2 {
+  char c[16];
+};
+typedef struct Struct2 __declspec(align(16)) AlignedStruct2;
 
 #define CHECK_SIZE(X, Align) \
   _Static_assert(__alignof(struct X) == Align, "should be aligned");
@@ -20,13 +23,13 @@ CHECK_SIZE(A, 16);
 
 struct B {
   char b;
-  AlignedInt a[1];
+  AlignedStruct2 a[1];
 };
 CHECK_SIZE(B, 16);
 
 struct C {
   char b;
-  AlignedInt a[];
+  AlignedStruct2 a[];
 };
 CHECK_SIZE(C, 16);
 
@@ -39,14 +42,18 @@ CHECK_SIZE(C, 16);
 // CHECK-NEXT:          0 |   struct AlignedStruct[1] a
 // CHECK-NEXT:            | [sizeof=16, align=16]
 // CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:          0 | struct Struct2
+// CHECK-NEXT:          0 |   char[16] c
+// CHECK-NEXT:            | [sizeof=16, align=1]
+// CHECK: *** Dumping AST Record Layout
 // CHECK-NEXT:          0 | struct B
 // CHECK-NEXT:          0 |   char b
-// CHECK-NEXT:         16 |   AlignedInt[1] a
+// CHECK-NEXT:         16 |   AlignedStruct2[1] a
 // CHECK-NEXT:            | [sizeof=32, align=16]
 // CHECK: *** Dumping AST Record Layout
 // CHECK-NEXT:          0 | struct C
 // CHECK-NEXT:          0 |   char b
-// CHECK-NEXT:         16 |   AlignedInt[] a
+// CHECK-NEXT:         16 |   AlignedStruct2[] a
 // CHECK-NEXT:            | [sizeof=16, align=16]
 
 #pragma pack(pop)

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

@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple i686-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>&1 \
-// RUN:            | FileCheck %s
+// RUN: %clang_cc1 -fno-rtti -triple i686-pc-win32 -fsyntax-only -verify %s
 // RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>/dev/null \
 // RUN:            | FileCheck %s -check-prefix CHECK-X64
 
@@ -7,14 +6,10 @@ struct T0 { char c; };
 struct T2 : virtual T0 { };
 struct T3 { T2 a[1]; char c; };
 
-// CHECK: *** Dumping AST Record Layout
-// CHECK: *** Dumping AST Record Layout
-// CHECK: *** Dumping AST Record Layout
-// CHECK-NEXT:    0 | struct T3
-// CHECK-NEXT:    0 |   T2[1] a
-// CHECK-NEXT:    5 |   char c
-// CHECK-NEXT:      | [sizeof=8, align=4
-// CHECK-NEXT:      |  nvsize=8, nvalign=4]
+#ifdef _ILP32
+// expected-error@-3 {{size of array element}}
+#endif
+
 // CHECK-X64: *** Dumping AST Record Layout
 // CHECK-X64: *** Dumping AST Record Layout
 // CHECK-X64: *** Dumping AST Record Layout

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

@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 // PR5637
 
@@ -7,7 +6,7 @@ typedef __attribute__((aligned(16))) struct {
   unsigned long long w[3];
 } UINT192;
 
-UINT192 ten2mk192M[] = {
+UINT192 ten2mk192M[] = { // expected-error {{size of array element}}
     {{0xcddd6e04c0592104ULL, 0x0fcf80dc33721d53ULL, 0xa7c5ac471b478423ULL}},
     {{0xcddd6e04c0592104ULL, 0x0fcf80dc33721d53ULL, 0xa7c5ac471b478423ULL}},
     {{0xcddd6e04c0592104ULL, 0x0fcf80dc33721d53ULL, 0xa7c5ac471b478423ULL}}

clang/test/Sema/align-x86.c

@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c11 -triple i386-apple-darwin9 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 #define STATIC_ASSERT(cond) _Static_assert(cond, #cond)
 
@@ -38,10 +37,10 @@ typedef ALIGNED(2) struct {
 } aligned_before_struct;
 
 STATIC_ASSERT(sizeof(aligned_before_struct)       == 3);
-STATIC_ASSERT(sizeof(aligned_before_struct[1])    == 4);
-STATIC_ASSERT(sizeof(aligned_before_struct[2])    == 6);
-STATIC_ASSERT(sizeof(aligned_before_struct[2][1]) == 8);
-STATIC_ASSERT(sizeof(aligned_before_struct[1][2]) == 6);
+STATIC_ASSERT(sizeof(aligned_before_struct[1])    == 4); // expected-error {{size of array element}}
+STATIC_ASSERT(sizeof(aligned_before_struct[2])    == 6); // expected-error {{size of array element}}
+STATIC_ASSERT(sizeof(aligned_before_struct[2][1]) == 8); // expected-error {{size of array element}}
+STATIC_ASSERT(sizeof(aligned_before_struct[1][2]) == 6); // expected-error {{size of array element}}
 
 typedef struct ALIGNED(2) {
   char a[3];

clang/test/Sema/attr-aligned.c

@@ -49,13 +49,14 @@ struct E e;
 char e1[__alignof__(e) == 2 ?: -1] = {0};
 char e2[__alignof__(e.member) == 2 ?: -1] = {0};
 
-typedef char overaligned_char __attribute__((aligned(16)));
-typedef overaligned_char array_with_overaligned_char[11];
+typedef struct { char c[16]; } S;
+typedef S overaligned_struct __attribute__((aligned(16)));
+typedef overaligned_struct array_with_overaligned_struct[11];
 typedef char array_with_align_attr[11] __attribute__((aligned(16)));
 
-char f0[__alignof__(array_with_overaligned_char) == 16 ? 1 : -1] = { 0 };
+char f0[__alignof__(array_with_overaligned_struct) == 16 ? 1 : -1] = { 0 };
 char f1[__alignof__(array_with_align_attr) == 16 ? 1 : -1] = { 0 };
-array_with_overaligned_char F2;
+array_with_overaligned_struct F2;
 char f2[__alignof__(F2) == 16 ? 1 : -1] = { 0 };
 array_with_align_attr F3;
 char f3[__alignof__(F3) == 16 ? 1 : -1] = { 0 };

clang/test/SemaCXX/align-x86.cpp

@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -std=c++11 -triple i386-apple-darwin9 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 using size_t = decltype(sizeof(0));
 
@@ -46,10 +45,10 @@ typedef ALIGNED(2) struct {
 } aligned_before_struct;
 
 static_assert(sizeof(aligned_before_struct)       == 3, "");
-static_assert(sizeof(aligned_before_struct[1])    == 4, "");
-static_assert(sizeof(aligned_before_struct[2])    == 6, "");
-static_assert(sizeof(aligned_before_struct[2][1]) == 8, "");
-static_assert(sizeof(aligned_before_struct[1][2]) == 6, "");
+static_assert(sizeof(aligned_before_struct[1])    == 4, ""); // expected-error {{size of array element}}
+static_assert(sizeof(aligned_before_struct[2])    == 6, ""); // expected-error {{size of array element}}
+static_assert(sizeof(aligned_before_struct[2][1]) == 8, ""); // expected-error {{size of array element}}
+static_assert(sizeof(aligned_before_struct[1][2]) == 6, ""); // expected-error {{size of array element}}
 
 typedef struct ALIGNED(2) {
   char a[3];

clang/test/SemaCXX/array-alignment.cpp

@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+typedef char __attribute__((aligned(2))) AlignedChar;
+typedef AlignedChar arrayType0[4]; // expected-error {{size of array element}}
+
+struct __attribute__((aligned(8))) AlignedStruct {
+  int m0;
+};
+
+struct __attribute__((packed)) PackedStruct {
+  char m0;
+  int i0;
+};
+
+typedef PackedStruct AlignedPackedStruct __attribute__((aligned(4)));
+typedef AlignedPackedStruct arrayType1[4]; // expected-error {{(5 bytes) isn't a multiple of its alignment (4 bytes)}}
+
+AlignedChar a0[1]; // expected-error {{size of array element}}
+AlignedStruct a1[1];
+AlignedPackedStruct a2[1]; // expected-error {{size of array element}}
+
+struct S {
+  AlignedChar m0[1]; // expected-error {{size of array element}}
+  AlignedStruct m1[1];
+  AlignedPackedStruct m2[1]; // expected-error {{size of array element}}
+};
+
+void test(char *p) {
+  auto p0 = (AlignedChar(*)[1])p;    // expected-error {{size of array element}}
+  auto r0 = (AlignedChar(&)[1])(*p); // expected-error {{size of array element}}
+  auto p1 = new AlignedChar[1];      // expected-error {{size of array element}}
+  auto p2 = (AlignedStruct(*)[1])p;
+  auto p3 = new AlignedStruct[1];
+  auto p4 = (AlignedPackedStruct(*)[1])p; // expected-error {{size of array element}}
+  auto p5 = new AlignedPackedStruct[1]; // expected-error {{size of array element}}
+}

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

@@ -19,9 +19,13 @@ void helper() {
 }
 
 namespace test2 {
+struct S {
+  char c[256];
+};
+
 class Test {
-  typedef int __attribute__((aligned(256))) aligned_int;
-  aligned_int high_contention_data[10];
+  typedef S __attribute__((aligned(256))) alignedS;
+  alignedS high_contention_data[10];
 };
 
 void helper() {