[PATCH] D46439: [Sema] Fix incorrect packed aligned structure layout

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D46439#1106929, @probinson wrote:

> This wouldn't be another layout/ABI breakage, would it?


Yes, albeit for what I'd expect to be an extremely rare case.


Repository:
  rC Clang

https://reviews.llvm.org/D46439



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

[PATCH] D46439: [Sema] Fix incorrect packed aligned structure layout

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

This wouldn't be another layout/ABI breakage, would it?


Repository:
  rC Clang

https://reviews.llvm.org/D46439



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

[PATCH] D46439: [Sema] Fix incorrect packed aligned structure layout

[Momchil Velikov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC332843: [Sema] Fix incorrect packed aligned structure layout 
(authored by chill, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D46439?vs=145687=147781#toc

Repository:
  rC Clang

https://reviews.llvm.org/D46439

Files:
  lib/Sema/SemaDecl.cpp
  test/Layout/itanium-pack-and-align.cpp


Index: test/Layout/itanium-pack-and-align.cpp
===
--- test/Layout/itanium-pack-and-align.cpp
+++ test/Layout/itanium-pack-and-align.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only 
-fdump-record-layouts %s \
+// RUN:| FileCheck %s
+
+struct S {
+  char x;
+  int y;
+} __attribute__((packed, aligned(8)));
+
+struct alignas(8) T {
+  char x;
+  int y;
+} __attribute__((packed));
+
+S s;
+T t;
+// CHECK:  0 | struct T
+// CHECK-NEXT:  0 |   char x
+// CHECK-NEXT:  1 |   int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NEXT:|  nvsize=8, nvalign=8]
+
+// CHECK:  0 | struct S
+// CHECK-NEXT:  0 |   char x
+// CHECK-NEXT:  1 |   int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NETX:|  nvsize=8, nvalign=8]
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15594,6 +15594,10 @@
 if (!Completed)
   Record->completeDefinition();
 
+// Handle attributes before checking the layout.
+if (Attr)
+  ProcessDeclAttributeList(S, Record, Attr);
+
 // We may have deferred checking for a deleted destructor. Check now.
 if (CXXRecordDecl *CXXRecord = dyn_cast(Record)) {
   auto *Dtor = CXXRecord->getDestructor();
@@ -15724,9 +15728,6 @@
   CDecl->setIvarRBraceLoc(RBrac);
 }
   }
-
-  if (Attr)
-ProcessDeclAttributeList(S, Record, Attr);
 }
 
 /// Determine whether the given integral value is representable within


Index: test/Layout/itanium-pack-and-align.cpp
===
--- test/Layout/itanium-pack-and-align.cpp
+++ test/Layout/itanium-pack-and-align.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only -fdump-record-layouts %s \
+// RUN:| FileCheck %s
+
+struct S {
+  char x;
+  int y;
+} __attribute__((packed, aligned(8)));
+
+struct alignas(8) T {
+  char x;
+  int y;
+} __attribute__((packed));
+
+S s;
+T t;
+// CHECK:  0 | struct T
+// CHECK-NEXT:  0 |   char x
+// CHECK-NEXT:  1 |   int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NEXT:|  nvsize=8, nvalign=8]
+
+// CHECK:  0 | struct S
+// CHECK-NEXT:  0 |   char x
+// CHECK-NEXT:  1 |   int y
+// CHECK-NEXT:| [sizeof=8, dsize=8, align=8,
+// CHECK-NETX:|  nvsize=8, nvalign=8]
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15594,6 +15594,10 @@
 if (!Completed)
   Record->completeDefinition();
 
+// Handle attributes before checking the layout.
+if (Attr)
+  ProcessDeclAttributeList(S, Record, Attr);
+
 // We may have deferred checking for a deleted destructor. Check now.
 if (CXXRecordDecl *CXXRecord = dyn_cast(Record)) {
   auto *Dtor = CXXRecord->getDestructor();
@@ -15724,9 +15728,6 @@
   CDecl->setIvarRBraceLoc(RBrac);
 }
   }
-
-  if (Attr)
-ProcessDeclAttributeList(S, Record, Attr);
 }
 
 /// Determine whether the given integral value is representable within
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46439: [Sema] Fix incorrect packed aligned structure layout

[Momchil Velikov via Phabricator via cfe-commits]

chill added a comment.

Thanks a lot!


https://reviews.llvm.org/D46439



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

[PATCH] D46439: [Sema] Fix incorrect packed aligned structure layout

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D46439



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