Skip to content

Commit 8da5b90

Browse files
committed
[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
1 parent 33ffb62 commit 8da5b90

File tree

2 files changed

+60
-7
lines changed

2 files changed

+60
-7
lines changed

clang/lib/AST/ASTContext.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1892,24 +1892,24 @@ TypeInfo ASTContext::getTypeInfoImpl(const Type *T) const {
18921892

18931893
case Type::IncompleteArray:
18941894
case Type::VariableArray:
1895-
Width = 0;
1896-
Align = getTypeAlign(cast<ArrayType>(T)->getElementType());
1897-
break;
1898-
18991895
case Type::ConstantArray: {
1900-
const auto *CAT = cast<ConstantArrayType>(T);
1896+
// Model non-constant sized arrays as size zero, but track the alignment.
1897+
uint64_t Size = 0;
1898+
if (const auto *CAT = dyn_cast<ConstantArrayType>(T))
1899+
Size = CAT->getSize().getZExtValue();
19011900

1902-
TypeInfo EltInfo = getTypeInfo(CAT->getElementType());
1903-
uint64_t Size = CAT->getSize().getZExtValue();
1901+
TypeInfo EltInfo = getTypeInfo(cast<ArrayType>(T)->getElementType());
19041902
assert((Size == 0 || EltInfo.Width <= (uint64_t)(-1) / Size) &&
19051903
"Overflow in array type bit size evaluation");
19061904
Width = EltInfo.Width * Size;
19071905
Align = EltInfo.Align;
1906+
AlignIsRequired = EltInfo.AlignIsRequired;
19081907
if (!getTargetInfo().getCXXABI().isMicrosoft() ||
19091908
getTargetInfo().getPointerWidth(0) == 64)
19101909
Width = llvm::alignTo(Width, Align);
19111910
break;
19121911
}
1912+
19131913
case Type::ExtVector:
19141914
case Type::Vector: {
19151915
const auto *VT = cast<VectorType>(T);
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple x86_64-pc-win32 -fms-extensions -fdump-record-layouts -fsyntax-only %s 2>/dev/null \
2+
// RUN: | FileCheck %s -check-prefix CHECK
3+
4+
// Before PR45420, we would only find the alignment on this record. Afterwards,
5+
// we can see the alignment on the typedef through the array type.
6+
// FIXME: What about other type sugar, like _Atomic? This would only matter in a
7+
// packed struct context.
8+
struct __declspec(align(16)) AlignedStruct { int x; };
9+
typedef int __declspec(align(16)) AlignedInt;
10+
11+
#define CHECK_SIZE(X, Align) \
12+
_Static_assert(__alignof(struct X) == Align, "should be aligned");
13+
14+
#pragma pack(push, 2)
15+
16+
struct A {
17+
struct AlignedStruct a[1];
18+
};
19+
CHECK_SIZE(A, 16);
20+
21+
struct B {
22+
char b;
23+
AlignedInt a[1];
24+
};
25+
CHECK_SIZE(B, 16);
26+
27+
struct C {
28+
char b;
29+
AlignedInt a[];
30+
};
31+
CHECK_SIZE(C, 16);
32+
33+
// CHECK: *** Dumping AST Record Layout
34+
// CHECK-NEXT: 0 | struct AlignedStruct
35+
// CHECK-NEXT: 0 | int x
36+
// CHECK-NEXT: | [sizeof=16, align=16]
37+
// CHECK: *** Dumping AST Record Layout
38+
// CHECK-NEXT: 0 | struct A
39+
// CHECK-NEXT: 0 | struct AlignedStruct [1] a
40+
// CHECK-NEXT: | [sizeof=16, align=16]
41+
// CHECK: *** Dumping AST Record Layout
42+
// CHECK-NEXT: 0 | struct B
43+
// CHECK-NEXT: 0 | char b
44+
// CHECK-NEXT: 16 | AlignedInt [1] a
45+
// CHECK-NEXT: | [sizeof=32, align=16]
46+
// CHECK: *** Dumping AST Record Layout
47+
// CHECK-NEXT: 0 | struct C
48+
// CHECK-NEXT: 0 | char b
49+
// CHECK-NEXT: 16 | AlignedInt [] a
50+
// CHECK-NEXT: | [sizeof=16, align=16]
51+
52+
#pragma pack(pop)
53+

0 commit comments

Comments
 (0)