[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Fixed in 0ca1051bf 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Please try to avoid uploading multiple independent patches into the same 
revision; it's confusing for anyone who comes looking for the review of the 
initial patch, and only finds the followup.

That said, fix looks fine, sure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 462938.
ahatanak added a comment.

Check whether the allocated type is an array type before calling 
`checkArrayElementAlignment`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/array-alignment.cpp


Index: clang/test/SemaCXX/array-alignment.cpp
===
--- clang/test/SemaCXX/array-alignment.cpp
+++ clang/test/SemaCXX/array-alignment.cpp
@@ -33,4 +33,5 @@
   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}}
+  auto p6 = new AlignedPackedStruct;
 }
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2079,6 +2079,9 @@
   if (CheckAllocatedType(AllocType, TypeRange.getBegin(), TypeRange))
 return ExprError();
 
+  if (ArraySize && !checkArrayElementAlignment(AllocType, 
TypeRange.getBegin()))
+return ExprError();
+
   // In ARC, infer 'retaining' for the allocated
   if (getLangOpts().ObjCAutoRefCount &&
   AllocType.getObjCLifetime() == Qualifiers::OCL_None &&
@@ -2449,8 +2452,6 @@
   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;


Index: clang/test/SemaCXX/array-alignment.cpp
===
--- clang/test/SemaCXX/array-alignment.cpp
+++ clang/test/SemaCXX/array-alignment.cpp
@@ -33,4 +33,5 @@
   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}}
+  auto p6 = new AlignedPackedStruct;
 }
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -2079,6 +2079,9 @@
   if (CheckAllocatedType(AllocType, TypeRange.getBegin(), TypeRange))
 return ExprError();
 
+  if (ArraySize && !checkArrayElementAlignment(AllocType, TypeRange.getBegin()))
+return ExprError();
+
   // In ARC, infer 'retaining' for the allocated
   if (getLangOpts().ObjCAutoRefCount &&
   AllocType.getObjCLifetime() == Qualifiers::OCL_None &&
@@ -2449,8 +2452,6 @@
   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;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Thanks for reporting the bug.

`Sema::CheckAllocatedType` can be called when the allocated type isn't an 
array, so `checkArrayElementAlignment` shouldn't be called in that function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

This also fires in non-array cases:

  $ cat /tmp/a.cc
  struct T0 { char c; };
  struct T2 : virtual T0 { };
  
  T2* f() { return new T2(); }
  
  $ /work/llvm-project/build/bin/clang++ -c -target i686-pc-win32 /tmp/a.cc
  /tmp/a.cc:4:22: error: size of array element of type 'T2' (5 bytes) isn't a 
multiple of its alignment (4 bytes)
  T2* f() { return new T2(); }
   ^
  1 error generated.

I don't think we want to error here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

GCC also rejects those tests so https://reviews.llvm.org/D134417 to disable 
them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

This has caused 2 test suite failures across our bots, for example: 
https://lab.llvm.org/buildbot/#/builders/174/builds/13518 
`GCC-C-execute-pr36093.test` / `GCC-C-execute-pr43783.test`

Pretty likely the tests are doing something strange, I will look into it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Akira Hatanaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGadaf62ced2a1: [Sema] Reject array element types whose sizes 
arent a multiple of their (authored by ahatanak).

Changed prior to commit:
  https://reviews.llvm.org/D133711?vs=460735=461921#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenCXX/override-layout-packed-base.cpp
  clang/test/CodeGenCXX/ubsan-new-checks.cpp
  clang/test/Layout/ms-aligned-array.c
  clang/test/Layout/ms-x86-misalignedarray.cpp
  clang/test/Sema/align-x86-64.c
  clang/test/Sema/align-x86.c
  clang/test/Sema/attr-aligned.c
  clang/test/SemaCXX/align-x86.cpp
  clang/test/SemaCXX/array-alignment.cpp
  clang/test/SemaCXX/warn-new-overaligned.cpp

Index: clang/test/SemaCXX/warn-new-overaligned.cpp
===
--- clang/test/SemaCXX/warn-new-overaligned.cpp
+++ clang/test/SemaCXX/warn-new-overaligned.cpp
@@ -19,9 +19,13 @@
 }
 
 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() {
Index: clang/test/SemaCXX/array-alignment.cpp
===
--- /dev/null
+++ 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}}
+}
Index: clang/test/SemaCXX/align-x86.cpp
===
--- clang/test/SemaCXX/align-x86.cpp
+++ 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 @@
 } 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];
Index: clang/test/Sema/attr-aligned.c
===
--- clang/test/Sema/attr-aligned.c
+++ clang/test/Sema/attr-aligned.c
@@ -49,13 +49,14 @@
 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 };

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.

Sound like a good plan. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I'm going to commit this in a day or two if there are no more comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 460735.
ahatanak added a comment.

- Fix failing CodeGen test.
- Add release note to the Potentially Breaking Changes section.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenCXX/override-layout-packed-base.cpp
  clang/test/CodeGenCXX/ubsan-new-checks.cpp
  clang/test/Layout/ms-aligned-array.c
  clang/test/Layout/ms-x86-misalignedarray.cpp
  clang/test/Sema/align-x86-64.c
  clang/test/Sema/align-x86.c
  clang/test/Sema/attr-aligned.c
  clang/test/SemaCXX/align-x86.cpp
  clang/test/SemaCXX/array-alignment.cpp
  clang/test/SemaCXX/warn-new-overaligned.cpp

Index: clang/test/SemaCXX/warn-new-overaligned.cpp
===
--- clang/test/SemaCXX/warn-new-overaligned.cpp
+++ clang/test/SemaCXX/warn-new-overaligned.cpp
@@ -19,9 +19,13 @@
 }
 
 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() {
Index: clang/test/SemaCXX/array-alignment.cpp
===
--- /dev/null
+++ 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}}
+}
Index: clang/test/SemaCXX/align-x86.cpp
===
--- clang/test/SemaCXX/align-x86.cpp
+++ 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 @@
 } 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];
Index: clang/test/Sema/attr-aligned.c
===
--- clang/test/Sema/attr-aligned.c
+++ clang/test/Sema/attr-aligned.c
@@ -49,13 +49,14 @@
 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) 

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133711#3790933 , @rnk wrote:

> This seems like a new error that will be pretty easy to run into, especially 
> in the funky MSVC virtual base case. @aaron.ballman proposed some kind of new 
> breaking change announcement mechanism. Can you add a release note and send 
> an announcement following that process? I think it's just posting to 
> Discourse announcements.

I just landed the change to the release notes so there's now a potentially 
breaking changes section to put the note under. We're not quite ready to post 
to announcements yet (that channel is locked so we need to remove the 
gatekeeping mechanism there), but when I'm able to post to announcements, I 
plan to announce everything already in the release notes anyway. So I'd say 
adding `clang-vendors` like @efriedma did and putting in the release notes is 
great for now.

Note, precommit CI found a relevant failure:

   TEST 'Clang :: 
CodeGenCXX/override-layout-packed-base.cpp' FAILED 
  
  Script:
  
  --
  
  : 'RUN: at line 1';   
/var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 
-internal-isystem 
/var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/16.0.0/include 
-nostdsysteminc -triple i686-windows-msvc -w -fdump-record-layouts-simple 
-foverride-record-layout=/var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
 
/var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/override-layout-packed-base.cpp
 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck 
/var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/override-layout-packed-base.cpp
  
  --
  
  Exit Code: 1
  
  
  
  Command Output (stderr):
  
  --
  
  
/var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/override-layout-packed-base.cpp:38:7:
 error: size of array element of type 'C' (11 bytes) isn't a multiple of its 
alignment (4 bytes)
  
C cs[sizeof(C)];
  
^
  
  
/var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/override-layout-packed-base.cpp:39:7:
 error: size of array element of type 'D' (15 bytes) isn't a multiple of its 
alignment (4 bytes)
  
D ds[sizeof(D)];
  
^
  
  2 errors generated.
  
  
  
  --
  
  
  
  


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

This seems like a new error that will be pretty easy to run into, especially in 
the funky MSVC virtual base case. @aaron.ballman proposed some kind of new 
breaking change announcement mechanism. Can you add a release note and send an 
announcement following that process? I think it's just posting to Discourse 
announcements.

Overall this seems like the right thing to do.




Comment at: clang/test/Layout/ms-x86-misalignedarray.cpp:10
+#ifdef _ILP32
+// expected-error@-3 {{size of array element}}
+#endif

I think what I had in mind when I said let's not change record layout is, let's 
keep doing what we were doing if we are targeting MSVC. But, what you've done 
is principled, so if we can ship it, it would be for the best.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 460207.
ahatanak retitled this revision from "[Sema] Reject array element types whose 
alignments are larger than their sizes" to "[Sema] Reject array element types 
whose sizes aren't a multiple of their alignments".
ahatanak added a comment.

Check whether the size is a multiple of the alignment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133711/new/

https://reviews.llvm.org/D133711

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenCXX/ubsan-new-checks.cpp
  clang/test/Layout/ms-aligned-array.c
  clang/test/Layout/ms-x86-misalignedarray.cpp
  clang/test/Sema/align-x86-64.c
  clang/test/Sema/align-x86.c
  clang/test/Sema/attr-aligned.c
  clang/test/SemaCXX/align-x86.cpp
  clang/test/SemaCXX/array-alignment.cpp
  clang/test/SemaCXX/warn-new-overaligned.cpp

Index: clang/test/SemaCXX/warn-new-overaligned.cpp
===
--- clang/test/SemaCXX/warn-new-overaligned.cpp
+++ clang/test/SemaCXX/warn-new-overaligned.cpp
@@ -19,9 +19,13 @@
 }
 
 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() {
Index: clang/test/SemaCXX/array-alignment.cpp
===
--- /dev/null
+++ 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}}
+}
Index: clang/test/SemaCXX/align-x86.cpp
===
--- clang/test/SemaCXX/align-x86.cpp
+++ 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 @@
 } 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];
Index: clang/test/Sema/attr-aligned.c
===
--- clang/test/Sema/attr-aligned.c
+++ clang/test/Sema/attr-aligned.c
@@ -49,13 +49,14 @@
 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