Fixed Assert In CGRecordLowering

Prior to this patch, CGRecordLower assumed that virtual bases could not 
be placed before the nvsize of an object.  This isn't true in Itanium 
mode, virtual bases are placed at dsize rather than vnsize and in the 
case of zero sized non-virtual bases nvsize can be larger than dsize.  
This patch fixes CGRecordLowering to avoid an assert and to clip 
bitfields properly in this case.  A test case is included.



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

2 files changed, 32 insertions, 6 deletions

diff --git a/lib/CodeGen/CGRecordLayoutBuilder.cpp b/lib/CodeGen/CGRecordLayoutBuilder.cpp
index b7ab169a64..eb2d524ef7 100644
--- a/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -414,10 +414,11 @@ CGRecordLowering::accumulateBitFields(RecordDecl::field_iterator Field,
 
 void CGRecordLowering::accumulateBases() {
   // If we've got a primary virtual base, we need to add it with the bases.
-  if (Layout.isPrimaryBaseVirtual())
-    Members.push_back(StorageInfo(
-      CharUnits::Zero(),
-      getStorageType(Layout.getPrimaryBase())));
+  if (Layout.isPrimaryBaseVirtual()) {
+    const CXXRecordDecl *BaseDecl = Layout.getPrimaryBase();
+    Members.push_back(MemberInfo(CharUnits::Zero(), MemberInfo::Base,
+                                 getStorageType(BaseDecl), BaseDecl));
+  }
   // Accumulate the non-virtual bases.
   for (const auto &Base : RD->bases()) {
     if (Base.isVirtual())
@@ -440,8 +441,24 @@ void CGRecordLowering::accumulateVPtrs() {
 }
 
 void CGRecordLowering::accumulateVBases() {
-  Members.push_back(MemberInfo(Layout.getNonVirtualSize(),
-                               MemberInfo::Scissor, 0, RD));
+  CharUnits ScissorOffset = Layout.getNonVirtualSize();
+  // In the itanium ABI, it's possible to place a vbase at a dsize that is
+  // smaller than the nvsize.  Here we check to see if such a base is placed
+  // before the nvsize and set the scissor offset to that, instead of the
+  // nvsize.
+  if (!useMSABI())
+    for (const auto &Base : RD->vbases()) {
+      const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
+      if (BaseDecl->isEmpty())
+        continue;
+      // If the vbase is a primary virtual base of some base, then it doesn't
+      // get its own storage location but instead lives inside of that base.
+      if (Context.isNearlyEmpty(BaseDecl) && !hasOwnStorage(RD, BaseDecl))
+        continue;
+      ScissorOffset = std::min(ScissorOffset,
+                               Layout.getVBaseClassOffset(BaseDecl));
+    }
+  Members.push_back(MemberInfo(ScissorOffset, MemberInfo::Scissor, 0, RD));
   for (const auto &Base : RD->vbases()) {
     const CXXRecordDecl *BaseDecl = Base.getType()->getAsCXXRecordDecl();
     if (BaseDecl->isEmpty())
diff --git a/test/CodeGenCXX/class-layout.cpp b/test/CodeGenCXX/class-layout.cpp
index 1f6212949f..c6aaf0f046 100644
--- a/test/CodeGenCXX/class-layout.cpp
+++ b/test/CodeGenCXX/class-layout.cpp
@@ -91,3 +91,12 @@ namespace Test7 {
   B* b;
   #pragma pack ()
 }
+
+// Shouldn't crash.
+namespace Test8 {
+	struct A {};

+	struct D { int a; };

+	struct B : virtual D, A { };

+	struct C : B, A { void f() {} };

+	C c;

+}