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

diff --git a/lib/CodeGen/CGRecordLayoutBuilder.cpp b/lib/CodeGen/CGRecordLayoutBuilder.cpp
index b7ab169a64f95e49d9c00b99cbd6eb4b8eff12b9..eb2d524ef77e1a71bf6895c9bc324a94ba51bec2 100644 (file)
--- 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 1f6212949fde307f8d7aa3e8aa28195b431116b4..c6aaf0f0460807304dd90f1f0cfb5ac6df41f418 100644 (file)
--- 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 {};\r
+       struct D { int a; };\r
+       struct B : virtual D, A { };\r
+       struct C : B, A { void f() {} };\r
+       C c;\r
+}