[clang] 8da5b90 - [MS] Fix packed struct layout for arrays of aligned non-record types

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2020-04-14T18:34:52-07:00
New Revision: 8da5b9083691b557f50f72ab099598bb291aec5f

URL: https://github.com/llvm/llvm-project/commit/8da5b9083691b557f50f72ab099598bb291aec5f
DIFF: https://github.com/llvm/llvm-project/commit/8da5b9083691b557f50f72ab099598bb291aec5f.diff

LOG: [MS] Fix packed struct layout for arrays of aligned non-record types

In particular, this affects Clang's vectors. Users encounter this issue
when a struct contains an __m128 type.

Fixes PR45420

Reviewed By: rjmccall

Differential Revision: https://reviews.llvm.org/D77754

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

Modified: 
    clang/lib/AST/ASTContext.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 9181cce07cb4..d3f3162dc31f 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1892,24 +1892,24 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const {
 
   case Type::IncompleteArray:
   case Type::VariableArray:
-    Width = 0;
-    Align = getTypeAlign(cast<ArrayType>(T)->getElementType());
-    break;
-
   case Type::ConstantArray: {
-    const auto *CAT = cast<ConstantArrayType>(T);
+    // Model non-constant sized arrays as size zero, but track the alignment.
+    uint64_t Size = 0;
+    if (const auto *CAT = dyn_cast<ConstantArrayType>(T))
+      Size = CAT->getSize().getZExtValue();
 
-    TypeInfo EltInfo = getTypeInfo(CAT->getElementType());
-    uint64_t Size = CAT->getSize().getZExtValue();
+    TypeInfo EltInfo = getTypeInfo(cast<ArrayType>(T)->getElementType());
     assert((Size == 0 || EltInfo.Width <= (uint64_t)(-1) / Size) &&
            "Overflow in array type bit size evaluation");
     Width = EltInfo.Width * Size;
     Align = EltInfo.Align;
+    AlignIsRequired = EltInfo.AlignIsRequired;
     if (!getTargetInfo().getCXXABI().isMicrosoft() ||
         getTargetInfo().getPointerWidth(0) == 64)
       Width = llvm::alignTo(Width, Align);
     break;
   }
+
   case Type::ExtVector:
   case Type::Vector: {
     const auto *VT = cast<VectorType>(T);

diff  --git a/clang/test/Layout/ms-aligned-array.c b/clang/test/Layout/ms-aligned-array.c
new file mode 100644
index 000000000000..81fbbbb62753
--- /dev/null
+++ b/clang/test/Layout/ms-aligned-array.c
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple x86_64-pc-win32 -fms-extensions -fdump-record-layouts -fsyntax-only %s 2>/dev/null \
+// RUN:            | FileCheck %s -check-prefix CHECK
+
+// Before PR45420, we would only find the alignment on this record. Afterwards,
+// we can see the alignment on the typedef through the array type.
+// 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;
+
+#define CHECK_SIZE(X, Align) \
+  _Static_assert(__alignof(struct X) == Align, "should be aligned");
+
+#pragma pack(push, 2)
+
+struct A {
+  struct AlignedStruct a[1];
+};
+CHECK_SIZE(A, 16);
+
+struct B {
+  char b;
+  AlignedInt a[1];
+};
+CHECK_SIZE(B, 16);
+
+struct C {
+  char b;
+  AlignedInt a[];
+};
+CHECK_SIZE(C, 16);
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:          0 | struct AlignedStruct
+// CHECK-NEXT:          0 |   int x
+// CHECK-NEXT:            | [sizeof=16, align=16]
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:          0 | struct A
+// CHECK-NEXT:          0 |   struct AlignedStruct [1] a
+// CHECK-NEXT:            | [sizeof=16, align=16]
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:          0 | struct B
+// CHECK-NEXT:          0 |   char b
+// CHECK-NEXT:         16 |   AlignedInt [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:            | [sizeof=16, align=16]
+
+#pragma pack(pop)
+