From 5c3d8fe8533f402f41eb79e85604af685b5c235a Mon Sep 17 00:00:00 2001
From: Aleksandr Urakov <aleksandr.urakov@jetbrains.com>
Date: Tue, 23 Oct 2018 08:23:22 +0000
Subject: [PATCH] [AST] Do not align virtual bases in
`MicrosoftRecordLayoutBuilder` when an external layout is used
Summary:
The patch removes alignment of virtual bases when an external layout is used.
We have two cases:
- the external layout source has an information about virtual bases offsets,
so we just use them;
- the external source has no information about virtual bases offsets. In this
case we can't predict where the base will be located. If we will align it but
there will be something like `#pragma pack(push, 1)` really, then likely our
layout will not fit into the real structure size, and then some asserts will
hit. The asserts look reasonable, so I don't think that we need to remove
them. May be it would be better instead don't align fields / bases etc.
(so treat it always as `#pragma pack(push, 1)`) when an external layout source
is used but no info about a field location is presented.
This one is related to D49871
Reviewers: rnk, rsmith, zturner, mstorsjo, majnemer
Reviewed By: rnk
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D53497
llvm-svn: 345012
---
clang/lib/AST/RecordLayoutBuilder.cpp | 11 +++++-----
.../Inputs/override-layout-packed-base.layout | 10 ++++++++++
.../override-layout-packed-base.cpp | 20 ++++++++++++++++---
3 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 6f71d5b83e62..673fa3f4eeab 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2829,15 +2829,14 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
CharUnits BaseOffset;
// Respect the external AST source base offset, if present.
- bool FoundBase = false;
if (UseExternalLayout) {
- FoundBase = External.getExternalVBaseOffset(BaseDecl, BaseOffset);
- if (FoundBase)
- assert(BaseOffset >= Size && "base offset already allocated");
- }
- if (!FoundBase)
+ if (!External.getExternalVBaseOffset(BaseDecl, BaseOffset))
+ BaseOffset = Size;
+ } else
BaseOffset = Size.alignTo(Info.Alignment);
+ assert(BaseOffset >= Size && "base offset already allocated");
+
VBases.insert(std::make_pair(BaseDecl,
ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp)));
Size = BaseOffset + BaseLayout.getNonVirtualSize();
diff --git a/clang/test/CodeGenCXX/Inputs/override-layout-packed-base.layout b/clang/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
index 949215ab849d..2ebcb98819b7 100644
--- a/clang/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
+++ b/clang/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
@@ -3,16 +3,26 @@
Type: class B<0>
Layout: <ASTRecordLayout
+ Size:40
FieldOffsets: [0, 32]>
*** Dumping AST Record Layout
Type: class B<1>
Layout: <ASTRecordLayout
+ Size:40
FieldOffsets: [0, 32]>
*** Dumping AST Record Layout
Type: class C
Layout: <ASTRecordLayout
+ Size:88
FieldOffsets: [80]>
+
+*** Dumping AST Record Layout
+Type: class D
+
+Layout: <ASTRecordLayout
+ Size:120
+ FieldOffsets: [32]>
diff --git a/clang/test/CodeGenCXX/override-layout-packed-base.cpp b/clang/test/CodeGenCXX/override-layout-packed-base.cpp
index 26ed4b5d815a..03255f2f6ebf 100644
--- a/clang/test/CodeGenCXX/override-layout-packed-base.cpp
+++ b/clang/test/CodeGenCXX/override-layout-packed-base.cpp
@@ -1,26 +1,40 @@
-// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-packed-base.layout %s | FileCheck %s
+// RUN: %clang_cc1 -triple i686-windows-msvc -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-packed-base.layout %s | FileCheck %s
+
+//#pragma pack(push, 1)
// CHECK: Type: class B<0>
+// CHECK: Size:40
// CHECK: FieldOffsets: [0, 32]
// CHECK: Type: class B<1>
+// CHECK: Size:40
// CHECK: FieldOffsets: [0, 32]
-//#pragma pack(push, 1)
template<int I>
class B {
int _b1;
char _b2;
};
-//#pragma pack(pop)
// CHECK: Type: class C
+// CHECK: Size:88
// CHECK: FieldOffsets: [80]
class C : B<0>, B<1> {
char _c;
};
+// CHECK: Type: class D
+// CHECK: Size:120
+// CHECK: FieldOffsets: [32]
+
+class D : virtual B<0>, virtual B<1> {
+ char _d;
+};
+
+//#pragma pack(pop)
+
void use_structs() {
C cs[sizeof(C)];
+ D ds[sizeof(D)];
}