[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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@345012 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp
index 6f71d5b83e62e4aa2bb876a5b61d256197c1ff3b..673fa3f4eeaba84ea6ed45053281b302a281637e 100644 (file)
--- a/lib/AST/RecordLayoutBuilder.cpp
+++ b/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/test/CodeGenCXX/Inputs/override-layout-packed-base.layout b/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
index 949215ab849da88ef465e0c979d2e369b4fd9a5d..2ebcb98819b7b4b9d3865a9ca85f7514b196b523 100644 (file)
--- a/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
+++ b/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/test/CodeGenCXX/override-layout-packed-base.cpp b/test/CodeGenCXX/override-layout-packed-base.cpp
index 26ed4b5d815a5329140dcf372bddc54d9cb70217..03255f2f6ebfcf519ca57590d3f83fccba7b9f78 100644 (file)
--- a/test/CodeGenCXX/override-layout-packed-base.cpp
+++ b/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)];
 }