[PATCH] Fix vbtable indices if a class shares the vbptr with a non-virtual base

[Timur Iskhodzhanov]

Hi rnk,

http://llvm-reviews.chandlerc.com/D2089

Files:
  lib/AST/VTableBuilder.cpp
  lib/CodeGen/MicrosoftVBTables.cpp
  test/CodeGenCXX/microsoft-abi-vbtables.cpp

Index: lib/AST/VTableBuilder.cpp
===================================================================
--- lib/AST/VTableBuilder.cpp
+++ lib/AST/VTableBuilder.cpp
@@ -2397,16 +2397,57 @@
   return CreateVTableLayout(Builder);
 }
 
-unsigned clang::GetVBTableIndex(const CXXRecordDecl *Derived,
-                                const CXXRecordDecl *VBase) {
-  unsigned VBTableIndex = 1; // Start with one to skip the self entry.
+static unsigned GetVBTableIndexOrZero(const CXXRecordDecl *Derived,
+                                      const CXXRecordDecl *VBase,
+                                      BasesSetVectorTy &VisitedBases) {
+  const ASTContext &Context = Derived->getASTContext();
+  const ASTRecordLayout &Layout = Context.getASTRecordLayout(Derived);
+
+  // First, see if the Derived class shared the vbptr with a non-virtual base.
+  for (CXXRecordDecl::base_class_const_iterator I = Derived->bases_begin(),
+       E = Derived->bases_end(); I != E; ++I) {
+    if (I->isVirtual())
+      continue;
+
+    const CXXRecordDecl *CurBase = I->getType()->getAsCXXRecordDecl();
+    CharUnits DerivedVBPtrOffset = Layout.getVBPtrOffset(),
+              BaseOffset = Layout.getBaseClassOffset(CurBase);
+    if (DerivedVBPtrOffset < BaseOffset)
+      continue;
+    const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(CurBase);
+    if (!BaseLayout.hasVBPtr() ||
+        DerivedVBPtrOffset != BaseOffset + BaseLayout.getVBPtrOffset())
+      continue;
+
+    // If the Derived class shares the vbptr with a non-virtual base,
+    // it inherits its vbase indices.
+    unsigned VBIndexInNVBase =
+        GetVBTableIndexOrZero(CurBase, VBase, VisitedBases);
+    if (VBIndexInNVBase)
+      return VBIndexInNVBase;
+    assert(VisitedBases.count(VBase) == 0);
+  }
+
+  // New vbases are added to the end of the vbtable.
+  // Skip the self entry and vbases visited in the non-virtual base, if any.
+  unsigned VBTableIndex = 1 + VisitedBases.size();
   for (CXXRecordDecl::base_class_const_iterator I = Derived->vbases_begin(),
        E = Derived->vbases_end(); I != E; ++I) {
-    if (I->getType()->getAsCXXRecordDecl() == VBase)
+    const CXXRecordDecl *CurVBase = I->getType()->getAsCXXRecordDecl();
+    if (CurVBase == VBase)
       return VBTableIndex;
-    ++VBTableIndex;
+    if (VisitedBases.insert(CurVBase))
+      ++VBTableIndex;
   }
-  llvm_unreachable("VBase must be a vbase of Derived");
+  return 0;
+}
+
+unsigned clang::GetVBTableIndex(const CXXRecordDecl *Derived,
+                                const CXXRecordDecl *VBase) {
+  BasesSetVectorTy VisitedBases;
+  unsigned ret = GetVBTableIndexOrZero(Derived, VBase, VisitedBases);
+  assert(ret > 0 && "VBase must be a vbase of Derived");
+  return ret;
 }
 
 namespace {
Index: lib/CodeGen/MicrosoftVBTables.cpp
===================================================================
--- lib/CodeGen/MicrosoftVBTables.cpp
+++ lib/CodeGen/MicrosoftVBTables.cpp
@@ -195,12 +195,11 @@
   const ASTRecordLayout &DerivedLayout =
     CGM.getContext().getASTRecordLayout(RD);
 
-  SmallVector<llvm::Constant *, 4> Offsets;
+  SmallVector<llvm::Constant *, 4> Offsets(1 + ReusingBase->getNumVBases(), 0);
 
   // The offset from ReusingBase's vbptr to itself always leads.
   CharUnits VBPtrOffset = BaseLayout.getVBPtrOffset();
-  Offsets.push_back(
-      llvm::ConstantInt::get(CGM.IntTy, -VBPtrOffset.getQuantity()));
+  Offsets[0] = llvm::ConstantInt::get(CGM.IntTy, -VBPtrOffset.getQuantity());
 
   // These are laid out in the same order as in Itanium, which is the same as
   // the order of the vbase iterator.
@@ -211,7 +210,9 @@
     assert(!Offset.isNegative());
     // Make it relative to the subobject vbptr.
     Offset -= VBPtrSubobject.getBaseOffset() + VBPtrOffset;
-    Offsets.push_back(llvm::ConstantInt::get(CGM.IntTy, Offset.getQuantity()));
+    llvm::Constant *&VBOffset = Offsets[GetVBTableIndex(ReusingBase, VBase)];
+    assert(VBOffset == 0 && "The same vbindex seen twice?");
+    VBOffset = llvm::ConstantInt::get(CGM.IntTy, Offset.getQuantity());
   }
 
   assert(Offsets.size() ==
Index: test/CodeGenCXX/microsoft-abi-vbtables.cpp
===================================================================
--- test/CodeGenCXX/microsoft-abi-vbtables.cpp
+++ test/CodeGenCXX/microsoft-abi-vbtables.cpp
@@ -410,3 +410,54 @@
 // CHECK-DAG: @"\01??_8B at Test21@@7B@" =
 // CHECK-DAG: @"\01??_8C at Test21@@7B@" =
 }
+
+namespace Test22 {
+struct A { int a; };
+struct B : virtual A { int b; };
+struct C { int c; };
+struct D : B, virtual C { int d; };
+D d;
+
+// CHECK-DAG: @"\01??_8D at Test22@@7B@" = linkonce_odr unnamed_addr constant [3 x i32] [i32 0, i32 12, i32 16]
+// CHECK-DAG: @"\01??_8B at Test22@@7B@" = linkonce_odr unnamed_addr constant [2 x i32] [i32 0, i32 8]
+}
+
+namespace Test23 {
+struct A { int a; };
+struct B : virtual A { int b; };
+struct C { int c; };
+// Note the unusual order of bases. It forces C to be laid out before A.
+struct D : virtual C, B { int d; };
+D d;
+
+// CHECK-DAG: @"\01??_8D at Test23@@7B@" = linkonce_odr unnamed_addr constant [3 x i32] [i32 0, i32 16, i32 12]
+// CHECK-DAG: @"\01??_8B at Test23@@7B@" = linkonce_odr unnamed_addr constant [2 x i32] [i32 0, i32 8]
+}
+
+namespace Test24 {
+struct A { int a; };
+struct B : virtual A { int b; };
+struct C { int c; };
+struct D : virtual C, B {
+  virtual void f();  // Issues a vfptr, but the vbptr is still shared with B.
+  int d;
+};
+D d;
+
+// CHECK-DAG: @"\01??_8D at Test24@@7B@" = linkonce_odr unnamed_addr constant [3 x i32] [i32 0, i32 16, i32 12]
+// CHECK-DAG: @"\01??_8B at Test24@@7B@" = linkonce_odr unnamed_addr constant [2 x i32] [i32 0, i32 8]
+}
+
+namespace Test25 {
+struct A { int a; };
+struct B : virtual A {
+  virtual void f();  // Issues a vfptr.
+  int b;
+};
+struct C { int c; };
+struct D : virtual C, B { int d; };
+D d;
+
+// CHECK-DAG: @"\01??_8D at Test25@@7B@" = linkonce_odr unnamed_addr constant [3 x i32] [i32 -4, i32 16, i32 12]
+// CHECK-DAG: @"\01??_8B at Test25@@7B@" = linkonce_odr unnamed_addr constant [2 x i32] [i32 -4, i32 8]
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2089.1.patch
Type: text/x-patch
Size: 6064 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131101/2f677aa8/attachment.bin>