r207280 - Fixed Assert In CGRecordLowering

[Warren Hunt]

Author: whunt
Date: Fri Apr 25 16:56:30 2014
New Revision: 207280

URL: http://llvm.org/viewvc/llvm-project?rev=207280&view=rev
Log:
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.


Modified:
    cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
    cfe/trunk/test/CodeGenCXX/class-layout.cpp

Modified: cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp?rev=207280&r1=207279&r2=207280&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp Fri Apr 25 16:56:30 2014
@@ -414,10 +414,11 @@ CGRecordLowering::accumulateBitFields(Re
 
 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())

Modified: cfe/trunk/test/CodeGenCXX/class-layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/class-layout.cpp?rev=207280&r1=207279&r2=207280&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/class-layout.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/class-layout.cpp Fri Apr 25 16:56:30 2014
@@ -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;
+}