This revision was automatically updated to reflect the committed changes.
Closed by commit rG8da5b9083691: [MS] Fix packed struct layout for arrays of
aligned non-record types (authored by rnk).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77754/new/
https://reviews.llvm.org/D77754
Files:
clang/lib/AST/ASTContext.cpp
clang/test/Layout/ms-aligned-array.c
Index: clang/test/Layout/ms-aligned-array.c
===================================================================
--- /dev/null
+++ 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)
+
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -1892,24 +1892,24 @@
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);
Index: clang/test/Layout/ms-aligned-array.c
===================================================================
--- /dev/null
+++ 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)
+
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -1892,24 +1892,24 @@
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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
