[PATCH] D137806: [AST] Fix class layout when using external layout under MS ABI.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG29c92879171d: [AST] Fix class layout when using external
layout under MS ABI. (authored by zequanwu).
Changed prior to commit:
https://reviews.llvm.org/D137806?vs=474623=475880#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137806/new/
https://reviews.llvm.org/D137806
Files:
clang/lib/AST/RecordLayoutBuilder.cpp
clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
clang/test/CodeGenCXX/override-layout-virtual-base.cpp
Index: clang/test/CodeGenCXX/override-layout-virtual-base.cpp
===
--- clang/test/CodeGenCXX/override-layout-virtual-base.cpp
+++ clang/test/CodeGenCXX/override-layout-virtual-base.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck %s
+// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck --check-prefix=SIMPLE %s
+// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck %s
struct S1 {
int a;
@@ -8,14 +9,48 @@
virtual void foo() {}
};
-// CHECK: Type: struct S3
-// CHECK: FieldOffsets: [128]
+// SIMPLE: Type: struct S3
+// SIMPLE: FieldOffsets: [64]
struct S3 : S2 {
char b;
};
+struct S4 {
+};
+
+struct S5 : S4 {
+ virtual void foo() {}
+};
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK: 0 | struct S2
+// CHECK-NEXT: 0 | (S2 vftable pointer)
+// CHECK-NEXT: 8 | (S2 vbtable pointer)
+// CHECK-NEXT: 8 | struct S1 (virtual base)
+// CHECK-NEXT: 8 | int a
+// CHECK-NEXT:| [sizeof=8, align=8,
+// CHECK-NEXT:| nvsize=8, nvalign=8]
+// CHECK: *** Dumping AST Record Layout
+// CHECK: 0 | struct S3
+// CHECK-NEXT: 0 | struct S2 (primary base)
+// CHECK-NEXT: 0 | (S2 vftable pointer)
+// CHECK-NEXT: 8 | (S2 vbtable pointer)
+// CHECK-NEXT: 8 | char b
+// CHECK-NEXT: 16 | struct S1 (virtual base)
+// CHECK-NEXT: 16 | int a
+// CHECK-NEXT:| [sizeof=24, align=8,
+// CHECK-NEXT:| nvsize=16, nvalign=8]
+// CHECK: *** Dumping AST Record Layout
+// CHECK: 0 | struct S5
+// CHECK-NEXT: 0 | (S5 vftable pointer)
+// CHECK-NEXT: 0 | struct S4 (base) (empty)
+// CHECK-NEXT:| [sizeof=8, align=8,
+// CHECK-NEXT:| nvsize=8, nvalign=8]
+
void use_structs() {
S1 s1s[sizeof(S1)];
S2 s2s[sizeof(S2)];
S3 s3s[sizeof(S3)];
+ S4 s4s[sizeof(S4)];
+ S5 s5s[sizeof(S5)];
}
Index: clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
===
--- clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
+++ clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
@@ -6,3 +6,11 @@
Size:64
Alignment:64
FieldOffsets: []>
+
+*** Dumping AST Record Layout
+Type: struct S5
+
+Layout:
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2027,7 +2027,7 @@
// The align if the field is not packed. This is to check if the attribute
// was unnecessary (-Wpacked).
- CharUnits UnpackedFieldAlign = FieldAlign;
+ CharUnits UnpackedFieldAlign = FieldAlign;
CharUnits PackedFieldAlign = CharUnits::One();
CharUnits UnpackedFieldOffset = FieldOffset;
CharUnits OriginalFieldAlign = UnpackedFieldAlign;
@@ -3081,10 +3081,9 @@
VBPtrOffset += Offset;
if (UseExternalLayout) {
-// The class may have no bases or fields, but still have a vfptr
-// (e.g. it's an interface class). The size was not correctly set before
-// in this case.
-if (FieldOffsets.empty() && Bases.empty())
+// The class may have size 0 and a vfptr (e.g. it's an interface class). The
+// size was not correctly set before in this case.
+if (Size.isZero())
Size += Offset;
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D137806: [AST] Fix class layout when using external layout under MS ABI.
rnk accepted this revision. rnk 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/D137806/new/ https://reviews.llvm.org/D137806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D137806: [AST] Fix class layout when using external layout under MS ABI.
zequanwu added inline comments. Comment at: clang/test/CodeGenCXX/override-layout-virtual-base.cpp:25-43 +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | struct S2 +// CHECK-NEXT: 0 | (S2 vftable pointer) +// CHECK-NEXT: 8 | (S2 vbtable pointer) +// CHECK-NEXT: 8 | struct S1 (virtual base) +// CHECK-NEXT: 8 | int a +// CHECK-NEXT:| [sizeof=8, align=8, Pasted -fdump-record-layout result below before this change as reference: ``` *** Dumping AST Record Layout 0 | struct S1 0 | int a | [sizeof=4, align=4, | nvsize=4, nvalign=4] *** Dumping AST Record Layout 0 | struct S2 0 | (S2 vftable pointer) 8 | (S2 vbtable pointer) 16 | struct S1 (virtual base) 16 | int a | [sizeof=8, align=8, | nvsize=16, nvalign=8] *** Dumping AST Record Layout 0 | struct S3 0 | struct S2 (primary base) 0 | (S2 vftable pointer) 8 | (S2 vbtable pointer) 16 | char b 24 | struct S1 (virtual base) 24 | int a | [sizeof=32, align=8, | nvsize=24, nvalign=8] *** Dumping AST Record Layout 0 | struct S4 (empty) | [sizeof=1, align=1, | nvsize=0, nvalign=1] *** Dumping AST Record Layout 0 | struct S5 0 | (S5 vftable pointer) 0 | struct S4 (base) (empty) | [sizeof=8, align=8, | nvsize=0, nvalign=8] ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137806/new/ https://reviews.llvm.org/D137806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D137806: [AST] Fix class layout when using external layout under MS ABI.
zequanwu created this revision.
zequanwu added a reviewer: rnk.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D137806
Files:
clang/lib/AST/RecordLayoutBuilder.cpp
clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
clang/test/CodeGenCXX/override-layout-virtual-base.cpp
Index: clang/test/CodeGenCXX/override-layout-virtual-base.cpp
===
--- clang/test/CodeGenCXX/override-layout-virtual-base.cpp
+++ clang/test/CodeGenCXX/override-layout-virtual-base.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck %s
+// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck --check-prefix=SIMPLE %s
+// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck %s
struct S1 {
int a;
@@ -8,14 +9,49 @@
virtual void foo() {}
};
-// CHECK: Type: struct S3
-// CHECK: FieldOffsets: [128]
+// SIMPLE: Type: struct S3
+// SIMPLE: FieldOffsets: [64]
struct S3 : S2 {
char b;
};
+struct S4 {
+};
+
+struct S5 : S4 {
+ virtual void foo() {}
+};
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct S2
+// CHECK-NEXT: 0 | (S2 vftable pointer)
+// CHECK-NEXT: 8 | (S2 vbtable pointer)
+// CHECK-NEXT: 8 | struct S1 (virtual base)
+// CHECK-NEXT: 8 | int a
+// CHECK-NEXT:| [sizeof=8, align=8,
+// CHECK-NEXT:| nvsize=8, nvalign=8]
+// CHECK-NEXT:
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct S3
+// CHECK-NEXT: 0 | struct S2 (primary base)
+// CHECK-NEXT: 0 | (S2 vftable pointer)
+// CHECK-NEXT: 8 | (S2 vbtable pointer)
+// CHECK-NEXT: 8 | char b
+// CHECK-NEXT: 16 | struct S1 (virtual base)
+// CHECK-NEXT: 16 | int a
+// CHECK-NEXT:| [sizeof=24, align=8,
+// CHECK-NEXT:| nvsize=16, nvalign=8]
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT: 0 | struct S5
+// CHECK-NEXT: 0 | (S5 vftable pointer)
+// CHECK-NEXT: 0 | struct S4 (base) (empty)
+// CHECK-NEXT:| [sizeof=8, align=8,
+// CHECK-NEXT:| nvsize=8, nvalign=8]
+
void use_structs() {
S1 s1s[sizeof(S1)];
S2 s2s[sizeof(S2)];
S3 s3s[sizeof(S3)];
+ S4 s4s[sizeof(S4)];
+ S5 s5s[sizeof(S5)];
}
Index: clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
===
--- clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
+++ clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
@@ -6,3 +6,11 @@
Size:64
Alignment:64
FieldOffsets: []>
+
+*** Dumping AST Record Layout
+Type: struct S5
+
+Layout:
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2027,7 +2027,7 @@
// The align if the field is not packed. This is to check if the attribute
// was unnecessary (-Wpacked).
- CharUnits UnpackedFieldAlign = FieldAlign;
+ CharUnits UnpackedFieldAlign = FieldAlign;
CharUnits PackedFieldAlign = CharUnits::One();
CharUnits UnpackedFieldOffset = FieldOffset;
CharUnits OriginalFieldAlign = UnpackedFieldAlign;
@@ -3081,10 +3081,9 @@
VBPtrOffset += Offset;
if (UseExternalLayout) {
-// The class may have no bases or fields, but still have a vfptr
-// (e.g. it's an interface class). The size was not correctly set before
-// in this case.
-if (FieldOffsets.empty() && Bases.empty())
+// The class may have size 0 and a vfptr (e.g. it's an interface class). The
+// size was not correctly set before in this case.
+if (Size.isZero())
Size += Offset;
return;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
