Fix vbtable indices when a class shares the vbptr with a non-virtual base

llvm-svn: 194082

clang/include/clang/AST/VTableBuilder.h

@@ -20,6 +20,7 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/Basic/ABI.h"
 #include "llvm/ADT/SetVector.h"
+#include "llvm/ADT/DenseSet.h"
 #include <utility>
 
 namespace clang {
@@ -358,16 +359,6 @@ public:
                                        const CXXRecordDecl *VBase);
 };
 
-/// \brief Computes the index of VBase in the vbtable of Derived.
-/// VBase must be a morally virtual base of Derived.  The vbtable is
-/// an array of i32 offsets.  The first entry is a self entry, and the rest are
-/// offsets from the vbptr to virtual bases.  The bases are ordered the same way
-/// our vbases are ordered: as they appear in a left-to-right depth-first search
-/// of the hierarchy.
-// FIXME: make this a static method of VBTableBuilder when we move it to AST.
-unsigned GetVBTableIndex(const CXXRecordDecl *Derived,
-                         const CXXRecordDecl *VBase);
-
 struct VFPtrInfo {
   typedef SmallVector<const CXXRecordDecl *, 1> BasePath;
 
@@ -411,7 +402,7 @@ struct VFPtrInfo {
   CharUnits VFPtrFullOffset;
 };
 
-class MicrosoftVFTableContext : public VTableContextBase {
+class MicrosoftVTableContext : public VTableContextBase {
 public:
   struct MethodVFTableLocation {
     /// If nonzero, holds the vbtable index of the virtual base with the vfptr.
@@ -467,16 +458,34 @@ private:
   typedef llvm::DenseMap<VFTableIdTy, const VTableLayout *> VFTableLayoutMapTy;
   VFTableLayoutMapTy VFTableLayouts;
 
+  typedef llvm::SmallSetVector<const CXXRecordDecl *, 8> BasesSetVectorTy;
+  void enumerateVFPtrs(const CXXRecordDecl *MostDerivedClass,
+                       const ASTRecordLayout &MostDerivedClassLayout,
+                       BaseSubobject Base, const CXXRecordDecl *LastVBase,
+                       const VFPtrInfo::BasePath &PathFromCompleteClass,
+                       BasesSetVectorTy &VisitedVBases,
+                       MicrosoftVTableContext::VFPtrListTy &Result);
+
+  void enumerateVFPtrs(const CXXRecordDecl *ForClass,
+                       MicrosoftVTableContext::VFPtrListTy &Result);
+
   void computeVTableRelatedInformation(const CXXRecordDecl *RD);
 
   void dumpMethodLocations(const CXXRecordDecl *RD,
                            const MethodVFTableLocationsTy &NewMethods,
                            raw_ostream &);
 
-public:
+  typedef std::pair<const CXXRecordDecl *, const CXXRecordDecl *> ClassPairTy;
-  MicrosoftVFTableContext(ASTContext &Context) : Context(Context) {}
+  typedef llvm::DenseMap<ClassPairTy, unsigned> VBTableIndicesTy;
+  VBTableIndicesTy VBTableIndices;
+  llvm::DenseSet<const CXXRecordDecl *> ComputedVBTableIndices;
 
-  ~MicrosoftVFTableContext() { llvm::DeleteContainerSeconds(VFTableLayouts); }
+  void computeVBTableRelatedInformation(const CXXRecordDecl *RD);
+
+public:
+  MicrosoftVTableContext(ASTContext &Context) : Context(Context) {}
+
+  ~MicrosoftVTableContext() { llvm::DeleteContainerSeconds(VFTableLayouts); }
 
   const VFPtrListTy &getVFPtrOffsets(const CXXRecordDecl *RD);
 
@@ -492,6 +501,19 @@ public:
       return 0;
     return VTableContextBase::getThunkInfo(GD);
   }
+
+  /// \brief Returns the index of VBase in the vbtable of Derived.
+  /// VBase must be a morally virtual base of Derived.
+  /// The vbtable is an array of i32 offsets.  The first entry is a self entry,
+  /// and the rest are offsets from the vbptr to virtual bases.
+  unsigned getVBTableIndex(const CXXRecordDecl *Derived,
+                           const CXXRecordDecl *VBase) {
+    computeVBTableRelatedInformation(Derived);
+    ClassPairTy Pair(Derived, VBase);
+    assert(VBTableIndices.count(Pair) == 1 &&
+           "VBase must be a vbase of Derived");
+    return VBTableIndices[Pair];
+  }
 };
 }
 

clang/lib/AST/VTableBuilder.cpp

@@ -2397,18 +2397,6 @@ VTableLayout *ItaniumVTableContext::createConstructionVTableLayout(
   return CreateVTableLayout(Builder);
 }
 
-unsigned clang::GetVBTableIndex(const CXXRecordDecl *Derived,
-                                const CXXRecordDecl *VBase) {
-  unsigned VBTableIndex = 1; // Start with one to skip the self entry.
-  for (CXXRecordDecl::base_class_const_iterator I = Derived->vbases_begin(),
-       E = Derived->vbases_end(); I != E; ++I) {
-    if (I->getType()->getAsCXXRecordDecl() == VBase)
-      return VBTableIndex;
-    ++VBTableIndex;
-  }
-  llvm_unreachable("VBase must be a vbase of Derived");
-}
-
 namespace {
 
 // Vtables in the Microsoft ABI are different from the Itanium ABI.
@@ -2451,12 +2439,15 @@ namespace {
 
 class VFTableBuilder {
 public:
-  typedef MicrosoftVFTableContext::MethodVFTableLocation MethodVFTableLocation;
+  typedef MicrosoftVTableContext::MethodVFTableLocation MethodVFTableLocation;
 
   typedef llvm::DenseMap<GlobalDecl, MethodVFTableLocation>
     MethodVFTableLocationsTy;
 
 private:
+  /// VTables - Global vtable information.
+  MicrosoftVTableContext &VTables;
+
   /// Context - The ASTContext which we will use for layout information.
   ASTContext &Context;
 
@@ -2591,8 +2582,10 @@ private:
   }
 
 public:
-  VFTableBuilder(const CXXRecordDecl *MostDerivedClass, VFPtrInfo Which)
+  VFTableBuilder(MicrosoftVTableContext &VTables,
-      : Context(MostDerivedClass->getASTContext()),
+                 const CXXRecordDecl *MostDerivedClass, VFPtrInfo Which)
+      : VTables(VTables),
+        Context(MostDerivedClass->getASTContext()),
         MostDerivedClass(MostDerivedClass),
         MostDerivedClassLayout(Context.getASTRecordLayout(MostDerivedClass)),
         WhichVFPtr(Which),
@@ -2889,7 +2882,7 @@ void VFTableBuilder::AddMethods(BaseSubobject Base, unsigned BaseDepth,
     // If we got here, MD is a method not seen in any of the sub-bases or
     // it requires return adjustment. Insert the method info for this method.
     unsigned VBIndex =
-        LastVBase ? GetVBTableIndex(MostDerivedClass, LastVBase) : 0;
+        LastVBase ? VTables.getVBTableIndex(MostDerivedClass, LastVBase) : 0;
     MethodInfo MI(VBIndex, Components.size());
 
     assert(!MethodInfoMap.count(MD) &&
@@ -2916,8 +2909,8 @@ void VFTableBuilder::AddMethods(BaseSubobject Base, unsigned BaseDepth,
         ReturnAdjustment.Virtual.Microsoft.VBPtrOffset =
             DerivedLayout.getVBPtrOffset().getQuantity();
         ReturnAdjustment.Virtual.Microsoft.VBIndex =
-            GetVBTableIndex(ReturnAdjustmentOffset.DerivedClass,
+            VTables.getVBTableIndex(ReturnAdjustmentOffset.DerivedClass,
-                            ReturnAdjustmentOffset.VirtualBase);
+                                    ReturnAdjustmentOffset.VirtualBase);
       }
     }
 
@@ -3087,13 +3080,13 @@ void VFTableBuilder::dumpLayout(raw_ostream &Out) {
 }
 }
 
-static void EnumerateVFPtrs(
+void MicrosoftVTableContext::enumerateVFPtrs(
-    ASTContext &Context, const CXXRecordDecl *MostDerivedClass,
+    const CXXRecordDecl *MostDerivedClass,
-    const ASTRecordLayout &MostDerivedClassLayout,
+    const ASTRecordLayout &MostDerivedClassLayout, BaseSubobject Base,
-    BaseSubobject Base, const CXXRecordDecl *LastVBase,
+    const CXXRecordDecl *LastVBase,
     const VFPtrInfo::BasePath &PathFromCompleteClass,
     BasesSetVectorTy &VisitedVBases,
-    MicrosoftVFTableContext::VFPtrListTy &Result) {
+    VFPtrListTy &Result) {
   const CXXRecordDecl *CurrentClass = Base.getBase();
   CharUnits OffsetInCompleteClass = Base.getBaseOffset();
   const ASTRecordLayout &CurrentClassLayout =
@@ -3101,7 +3094,7 @@ static void EnumerateVFPtrs(
 
   if (CurrentClassLayout.hasOwnVFPtr()) {
     if (LastVBase) {
-      uint64_t VBIndex = GetVBTableIndex(MostDerivedClass, LastVBase);
+      uint64_t VBIndex = getVBTableIndex(MostDerivedClass, LastVBase);
       assert(VBIndex > 0 && "vbases must have vbindex!");
       CharUnits VFPtrOffset =
           OffsetInCompleteClass -
@@ -3134,7 +3127,7 @@ static void EnumerateVFPtrs(
     NewPath.push_back(BaseDecl);
     BaseSubobject NextBase(BaseDecl, NextBaseOffset);
 
-    EnumerateVFPtrs(Context, MostDerivedClass, MostDerivedClassLayout, NextBase,
+    enumerateVFPtrs(MostDerivedClass, MostDerivedClassLayout, NextBase,
                     NextLastVBase, NewPath, VisitedVBases, Result);
   }
 }
@@ -3188,12 +3181,13 @@ CalculatePathToMangle(const CXXRecordDecl *RD, VFPtrInfo &VFPtr) {
   }
 }
 
-static void EnumerateVFPtrs(ASTContext &Context, const CXXRecordDecl *ForClass,
+void MicrosoftVTableContext::enumerateVFPtrs(
-                            MicrosoftVFTableContext::VFPtrListTy &Result) {
+    const CXXRecordDecl *ForClass,
+    MicrosoftVTableContext::VFPtrListTy &Result) {
   Result.clear();
   const ASTRecordLayout &ClassLayout = Context.getASTRecordLayout(ForClass);
   BasesSetVectorTy VisitedVBases;
-  EnumerateVFPtrs(Context, ForClass, ClassLayout,
+  enumerateVFPtrs(ForClass, ClassLayout,
                   BaseSubobject(ForClass, CharUnits::Zero()), 0,
                   VFPtrInfo::BasePath(), VisitedVBases, Result);
   if (Result.size() > 1) {
@@ -3202,7 +3196,7 @@ static void EnumerateVFPtrs(ASTContext &Context, const CXXRecordDecl *ForClass,
   }
 }
 
-void MicrosoftVFTableContext::computeVTableRelatedInformation(
+void MicrosoftVTableContext::computeVTableRelatedInformation(
     const CXXRecordDecl *RD) {
   assert(RD->isDynamicClass());
 
@@ -3213,12 +3207,12 @@ void MicrosoftVFTableContext::computeVTableRelatedInformation(
   const VTableLayout::AddressPointsMapTy EmptyAddressPointsMap;
 
   VFPtrListTy &VFPtrs = VFPtrLocations[RD];
-  EnumerateVFPtrs(Context, RD, VFPtrs);
+  enumerateVFPtrs(RD, VFPtrs);
 
   MethodVFTableLocationsTy NewMethodLocations;
   for (VFPtrListTy::iterator I = VFPtrs.begin(), E = VFPtrs.end();
        I != E; ++I) {
-    VFTableBuilder Builder(RD, *I);
+    VFTableBuilder Builder(*this, RD, *I);
 
     VFTableIdTy id(RD, I->VFPtrFullOffset);
     assert(VFTableLayouts.count(id) == 0);
@@ -3238,7 +3232,7 @@ void MicrosoftVFTableContext::computeVTableRelatedInformation(
     dumpMethodLocations(RD, NewMethodLocations, llvm::errs());
 }
 
-void MicrosoftVFTableContext::dumpMethodLocations(
+void MicrosoftVTableContext::dumpMethodLocations(
     const CXXRecordDecl *RD, const MethodVFTableLocationsTy &NewMethods,
     raw_ostream &Out) {
   // Compute the vtable indices for all the member functions.
@@ -3297,8 +3291,56 @@ void MicrosoftVFTableContext::dumpMethodLocations(
   }
 }
 
-const MicrosoftVFTableContext::VFPtrListTy &
+void MicrosoftVTableContext::computeVBTableRelatedInformation(
-MicrosoftVFTableContext::getVFPtrOffsets(const CXXRecordDecl *RD) {
+    const CXXRecordDecl *RD) {
+  if (ComputedVBTableIndices.count(RD))
+    return;
+  ComputedVBTableIndices.insert(RD);
+
+  const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
+  BasesSetVectorTy VisitedBases;
+
+  // First, see if the Derived class shared the vbptr
+  // with the first non-virtual base.
+  for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
+       E = RD->bases_end(); I != E; ++I) {
+    if (I->isVirtual())
+      continue;
+
+    const CXXRecordDecl *CurBase = I->getType()->getAsCXXRecordDecl();
+    CharUnits DerivedVBPtrOffset = Layout.getVBPtrOffset(),
+              BaseOffset = Layout.getBaseClassOffset(CurBase);
+    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.
+    computeVBTableRelatedInformation(CurBase);
+    for (CXXRecordDecl::base_class_const_iterator J = CurBase->vbases_begin(),
+         F = CurBase->vbases_end(); J != F; ++J) {
+      const CXXRecordDecl *SubVBase = J->getType()->getAsCXXRecordDecl();
+      assert(VBTableIndices.count(ClassPairTy(CurBase, SubVBase)));
+      VBTableIndices[ClassPairTy(RD, SubVBase)] =
+          VBTableIndices[ClassPairTy(CurBase, SubVBase)];
+      VisitedBases.insert(SubVBase);
+    }
+  }
+
+  // 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 = RD->vbases_begin(),
+       E = RD->vbases_end(); I != E; ++I) {
+    const CXXRecordDecl *CurVBase = I->getType()->getAsCXXRecordDecl();
+    if (VisitedBases.insert(CurVBase))
+      VBTableIndices[ClassPairTy(RD, CurVBase)] = VBTableIndex++;
+  }
+}
+
+const MicrosoftVTableContext::VFPtrListTy &
+MicrosoftVTableContext::getVFPtrOffsets(const CXXRecordDecl *RD) {
   computeVTableRelatedInformation(RD);
 
   assert(VFPtrLocations.count(RD) && "Couldn't find vfptr locations");
@@ -3306,8 +3348,8 @@ MicrosoftVFTableContext::getVFPtrOffsets(const CXXRecordDecl *RD) {
 }
 
 const VTableLayout &
-MicrosoftVFTableContext::getVFTableLayout(const CXXRecordDecl *RD,
+MicrosoftVTableContext::getVFTableLayout(const CXXRecordDecl *RD,
-                                          CharUnits VFPtrOffset) {
+                                         CharUnits VFPtrOffset) {
   computeVTableRelatedInformation(RD);
 
   VFTableIdTy id(RD, VFPtrOffset);
@@ -3315,8 +3357,8 @@ MicrosoftVFTableContext::getVFTableLayout(const CXXRecordDecl *RD,
   return *VFTableLayouts[id];
 }
 
-const MicrosoftVFTableContext::MethodVFTableLocation &
+const MicrosoftVTableContext::MethodVFTableLocation &
-MicrosoftVFTableContext::getMethodVFTableLocation(GlobalDecl GD) {
+MicrosoftVTableContext::getMethodVFTableLocation(GlobalDecl GD) {
   assert(cast<CXXMethodDecl>(GD.getDecl())->isVirtual() &&
          "Only use this method for virtual methods or dtors");
   if (isa<CXXDestructorDecl>(GD.getDecl()))

clang/lib/CodeGen/CGCXX.cpp

@@ -295,9 +295,9 @@ static llvm::Value *BuildAppleKextVirtualCall(CodeGenFunction &CGF,
   Ty = Ty->getPointerTo()->getPointerTo();
   VTable = CGF.Builder.CreateBitCast(VTable, Ty);
   assert(VTable && "BuildVirtualCall = kext vtbl pointer is null");
-  uint64_t VTableIndex = CGM.getVTableContext().getMethodVTableIndex(GD);
+  uint64_t VTableIndex = CGM.getItaniumVTableContext().getMethodVTableIndex(GD);
   uint64_t AddressPoint =
-    CGM.getVTableContext().getVTableLayout(RD)
+    CGM.getItaniumVTableContext().getVTableLayout(RD)
        .getAddressPoint(BaseSubobject(RD, CharUnits::Zero()));
   VTableIndex += AddressPoint;
   llvm::Value *VFuncPtr =

clang/lib/CodeGen/CGDebugInfo.cpp

@@ -1062,7 +1062,7 @@ CGDebugInfo::CreateCXXMemberFunction(const CXXMethodDecl *Method,
     // lookup if we have multiple or virtual inheritance.
     if (!isa<CXXDestructorDecl>(Method) &&
         !CGM.getTarget().getCXXABI().isMicrosoft())
-      VIndex = CGM.getVTableContext().getMethodVTableIndex(Method);
+      VIndex = CGM.getItaniumVTableContext().getMethodVTableIndex(Method);
     ContainingType = RecordTy;
   }
 
@@ -1168,7 +1168,7 @@ CollectCXXBases(const CXXRecordDecl *RD, llvm::DIFile Unit,
       // virtual base offset offset is -ve. The code generator emits dwarf
       // expression where it expects +ve number.
       BaseOffset =
-        0 - CGM.getVTableContext()
+        0 - CGM.getItaniumVTableContext()
                .getVirtualBaseOffsetOffset(RD, Base).getQuantity();
       BFlags = llvm::DIDescriptor::FlagVirtual;
     } else

clang/lib/CodeGen/CGRTTI.cpp

@@ -846,7 +846,7 @@ void RTTIBuilder::BuildVMIClassTypeInfo(const CXXRecordDecl *RD) {
     CharUnits Offset;
     if (Base->isVirtual())
       Offset = 
-        CGM.getVTableContext().getVirtualBaseOffsetOffset(RD, BaseDecl);
+        CGM.getItaniumVTableContext().getVirtualBaseOffsetOffset(RD, BaseDecl);
     else {
       const ASTRecordLayout &Layout = CGM.getContext().getASTRecordLayout(RD);
       Offset = Layout.getBaseClassOffset(BaseDecl);

clang/lib/CodeGen/CGVTT.cpp

@@ -65,8 +65,8 @@ CodeGenVTables::EmitVTTDefinition(llvm::GlobalVariable *VTT,
     uint64_t AddressPoint;
     if (VTTVT.getBase() == RD) {
       // Just get the address point for the regular vtable.
-      AddressPoint = VTContext.getVTableLayout(RD)
+      AddressPoint =
-                              .getAddressPoint(i->VTableBase);
+          ItaniumVTContext.getVTableLayout(RD).getAddressPoint(i->VTableBase);
       assert(AddressPoint != 0 && "Did not find vtable address point!");
     } else {
       AddressPoint = VTableAddressPoints[i->VTableIndex].lookup(i->VTableBase);

clang/lib/CodeGen/CGVTables.cpp

@@ -30,12 +30,12 @@ using namespace clang;
 using namespace CodeGen;
 
 CodeGenVTables::CodeGenVTables(CodeGenModule &CGM)
-  : CGM(CGM), VTContext(CGM.getContext()) {
+  : CGM(CGM), ItaniumVTContext(CGM.getContext()) {
   if (CGM.getTarget().getCXXABI().isMicrosoft()) {
     // FIXME: Eventually, we should only have one of V*TContexts available.
     // Today we use both in the Microsoft ABI as MicrosoftVFTableContext
     // is not completely supported in CodeGen yet.
-    VFTContext.reset(new MicrosoftVFTableContext(CGM.getContext()));
+    MicrosoftVTContext.reset(new MicrosoftVTableContext(CGM.getContext()));
   }
 }
 
@@ -439,10 +439,10 @@ void CodeGenVTables::EmitThunks(GlobalDecl GD)
     return;
 
   const VTableContextBase::ThunkInfoVectorTy *ThunkInfoVector;
-  if (VFTContext.isValid()) {
+  if (MicrosoftVTContext.isValid()) {
-    ThunkInfoVector = VFTContext->getThunkInfo(GD);
+    ThunkInfoVector = MicrosoftVTContext->getThunkInfo(GD);
   } else {
-    ThunkInfoVector = VTContext.getThunkInfo(GD);
+    ThunkInfoVector = ItaniumVTContext.getThunkInfo(GD);
   }
 
   if (!ThunkInfoVector)
@@ -581,9 +581,8 @@ CodeGenVTables::GenerateConstructionVTable(const CXXRecordDecl *RD,
     DI->completeClassData(Base.getBase());
 
   OwningPtr<VTableLayout> VTLayout(
-    VTContext.createConstructionVTableLayout(Base.getBase(),
+      ItaniumVTContext.createConstructionVTableLayout(
-                                             Base.getBaseOffset(),
+          Base.getBase(), Base.getBaseOffset(), BaseIsVirtual, RD));
-                                             BaseIsVirtual, RD));
 
   // Add the address points.
   AddressPoints = VTLayout->getAddressPoints();

clang/lib/CodeGen/CGVTables.h

@@ -31,10 +31,10 @@ namespace CodeGen {
 class CodeGenVTables {
   CodeGenModule &CGM;
 
-  // FIXME: Consider moving VTContext and VFTContext into respective CXXABI
+  // FIXME: Consider moving ItaniumVTContext and MicrosoftVTContext into
-  // classes?
+  // respective CXXABI classes?
-  ItaniumVTableContext VTContext;
+  ItaniumVTableContext ItaniumVTContext;
-  OwningPtr<MicrosoftVFTableContext> VFTContext;
+  OwningPtr<MicrosoftVTableContext> MicrosoftVTContext;
   
   /// VTableAddressPointsMapTy - Address points for a single vtable.
   typedef llvm::DenseMap<BaseSubobject, uint64_t> VTableAddressPointsMapTy;
@@ -72,9 +72,11 @@ public:
 
   CodeGenVTables(CodeGenModule &CGM);
 
-  ItaniumVTableContext &getVTableContext() { return VTContext; }
+  ItaniumVTableContext &getItaniumVTableContext() { return ItaniumVTContext; }
 
-  MicrosoftVFTableContext &getVFTableContext() { return *VFTContext.get(); }
+  MicrosoftVTableContext &getMicrosoftVTableContext() {
+    return *MicrosoftVTContext.get();
+  }
 
   /// getSubVTTIndex - Return the index of the sub-VTT for the base class of the
   /// given record decl.

clang/lib/CodeGen/CodeGenModule.h

@@ -523,12 +523,12 @@ public:
  
   CodeGenVTables &getVTables() { return VTables; }
 
-  ItaniumVTableContext &getVTableContext() {
+  ItaniumVTableContext &getItaniumVTableContext() {
-    return VTables.getVTableContext();
+    return VTables.getItaniumVTableContext();
   }
 
-  MicrosoftVFTableContext &getVFTableContext() {
+  MicrosoftVTableContext &getMicrosoftVTableContext() {
-    return VTables.getVFTableContext();
+    return VTables.getMicrosoftVTableContext();
   }
 
   llvm::MDNode *getTBAAInfo(QualType QTy);

clang/lib/CodeGen/ItaniumCXXABI.cpp

@@ -570,7 +570,7 @@ llvm::Constant *ItaniumCXXABI::BuildMemberPointer(const CXXMethodDecl *MD,
   // Get the function pointer (or index if this is a virtual function).
   llvm::Constant *MemPtr[2];
   if (MD->isVirtual()) {
-    uint64_t Index = CGM.getVTableContext().getMethodVTableIndex(MD);
+    uint64_t Index = CGM.getItaniumVTableContext().getMethodVTableIndex(MD);
 
     const ASTContext &Context = getContext();
     CharUnits PointerWidth =
@@ -780,7 +780,8 @@ ItaniumCXXABI::GetVirtualBaseClassOffset(CodeGenFunction &CGF,
                                          const CXXRecordDecl *BaseClassDecl) {
   llvm::Value *VTablePtr = CGF.GetVTablePtr(This, CGM.Int8PtrTy);
   CharUnits VBaseOffsetOffset =
-    CGM.getVTableContext().getVirtualBaseOffsetOffset(ClassDecl, BaseClassDecl);
+      CGM.getItaniumVTableContext().getVirtualBaseOffsetOffset(ClassDecl,
+                                                               BaseClassDecl);
 
   llvm::Value *VBaseOffsetPtr =
     CGF.Builder.CreateConstGEP1_64(VTablePtr, VBaseOffsetOffset.getQuantity(),
@@ -927,7 +928,7 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT,
   if (VTable->hasInitializer())
     return;
 
-  ItaniumVTableContext &VTContext = CGM.getVTableContext();
+  ItaniumVTableContext &VTContext = CGM.getItaniumVTableContext();
   const VTableLayout &VTLayout = VTContext.getVTableLayout(RD);
   llvm::GlobalVariable::LinkageTypes Linkage = CGM.getVTableLinkage(RD);
 
@@ -977,8 +978,9 @@ llvm::Value *ItaniumCXXABI::getVTableAddressPointInStructor(
   } else {
     llvm::Constant *VTable =
         CGM.getCXXABI().getAddrOfVTable(VTableClass, CharUnits());
-    uint64_t AddressPoint = CGM.getVTableContext().getVTableLayout(VTableClass)
+    uint64_t AddressPoint = CGM.getItaniumVTableContext()
-        .getAddressPoint(Base);
+                                .getVTableLayout(VTableClass)
+                                .getAddressPoint(Base);
     VTableAddressPoint =
         CGF.Builder.CreateConstInBoundsGEP2_64(VTable, 0, AddressPoint);
   }
@@ -991,8 +993,9 @@ llvm::Constant *ItaniumCXXABI::getVTableAddressPointForConstExpr(
   llvm::Constant *VTable = getAddrOfVTable(VTableClass, CharUnits());
 
   // Find the appropriate vtable within the vtable group.
-  uint64_t AddressPoint =
+  uint64_t AddressPoint = CGM.getItaniumVTableContext()
-    CGM.getVTableContext().getVTableLayout(VTableClass).getAddressPoint(Base);
+                              .getVTableLayout(VTableClass)
+                              .getAddressPoint(Base);
   llvm::Value *Indices[] = {
     llvm::ConstantInt::get(CGM.Int64Ty, 0),
     llvm::ConstantInt::get(CGM.Int64Ty, AddressPoint)
@@ -1018,7 +1021,7 @@ llvm::GlobalVariable *ItaniumCXXABI::getAddrOfVTable(const CXXRecordDecl *RD,
   Out.flush();
   StringRef Name = OutName.str();
 
-  ItaniumVTableContext &VTContext = CGM.getVTableContext();
+  ItaniumVTableContext &VTContext = CGM.getItaniumVTableContext();
   llvm::ArrayType *ArrayType = llvm::ArrayType::get(
       CGM.Int8PtrTy, VTContext.getVTableLayout(RD).getNumVTableComponents());
 
@@ -1036,7 +1039,7 @@ llvm::Value *ItaniumCXXABI::getVirtualFunctionPointer(CodeGenFunction &CGF,
   Ty = Ty->getPointerTo()->getPointerTo();
   llvm::Value *VTable = CGF.GetVTablePtr(This, Ty);
 
-  uint64_t VTableIndex = CGM.getVTableContext().getMethodVTableIndex(GD);
+  uint64_t VTableIndex = CGM.getItaniumVTableContext().getMethodVTableIndex(GD);
   llvm::Value *VFuncPtr =
       CGF.Builder.CreateConstInBoundsGEP1_64(VTable, VTableIndex, "vfn");
   return CGF.Builder.CreateLoad(VFuncPtr);

clang/lib/CodeGen/MicrosoftCXXABI.cpp

@@ -120,8 +120,8 @@ public:
   const CXXRecordDecl *getThisArgumentTypeForMethod(const CXXMethodDecl *MD) {
     MD = MD->getCanonicalDecl();
     if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD)) {
-      MicrosoftVFTableContext::MethodVFTableLocation ML =
+      MicrosoftVTableContext::MethodVFTableLocation ML =
-          CGM.getVFTableContext().getMethodVFTableLocation(MD);
+          CGM.getMicrosoftVTableContext().getMethodVFTableLocation(MD);
       // The vbases might be ordered differently in the final overrider object
       // and the complete object, so the "this" argument may sometimes point to
       // memory that has no particular type (e.g. past the complete object).
@@ -423,7 +423,9 @@ MicrosoftCXXABI::GetVirtualBaseClassOffset(CodeGenFunction &CGF,
   int64_t VBPtrChars = GetVBPtrOffsetFromBases(ClassDecl).getQuantity();
   llvm::Value *VBPtrOffset = llvm::ConstantInt::get(CGM.PtrDiffTy, VBPtrChars);
   CharUnits IntSize = getContext().getTypeSizeInChars(getContext().IntTy);
-  CharUnits VBTableChars = IntSize * GetVBTableIndex(ClassDecl, BaseClassDecl);
+  CharUnits VBTableChars =
+      IntSize *
+      CGM.getMicrosoftVTableContext().getVBTableIndex(ClassDecl, BaseClassDecl);
   llvm::Value *VBTableOffset =
     llvm::ConstantInt::get(CGM.IntTy, VBTableChars.getQuantity());
 
@@ -593,8 +595,8 @@ llvm::Value *MicrosoftCXXABI::adjustThisArgumentForVirtualCall(
     // with the base one, so look up the deleting one instead.
     LookupGD = GlobalDecl(DD, Dtor_Deleting);
   }
-  MicrosoftVFTableContext::MethodVFTableLocation ML =
+  MicrosoftVTableContext::MethodVFTableLocation ML =
-      CGM.getVFTableContext().getMethodVFTableLocation(LookupGD);
+      CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
 
   unsigned AS = cast<llvm::PointerType>(This->getType())->getAddressSpace();
   llvm::Type *charPtrTy = CGF.Int8Ty->getPointerTo(AS);
@@ -719,8 +721,8 @@ llvm::Value *MicrosoftCXXABI::adjustThisParameterInVirtualFunctionPrologue(
   // to the final overrider subobject before use.
   // See comments in the MicrosoftVFTableContext implementation for the details.
 
-  MicrosoftVFTableContext::MethodVFTableLocation ML =
+  MicrosoftVTableContext::MethodVFTableLocation ML =
-      CGM.getVFTableContext().getMethodVFTableLocation(LookupGD);
+      CGM.getMicrosoftVTableContext().getMethodVFTableLocation(LookupGD);
   CharUnits Adjustment = ML.VFTableOffset;
   if (ML.VBase) {
     const ASTRecordLayout &DerivedLayout =
@@ -801,11 +803,11 @@ void MicrosoftCXXABI::EmitConstructorCall(CodeGenFunction &CGF,
 
 void MicrosoftCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT,
                                             const CXXRecordDecl *RD) {
-  MicrosoftVFTableContext &VFTContext = CGM.getVFTableContext();
+  MicrosoftVTableContext &VFTContext = CGM.getMicrosoftVTableContext();
-  MicrosoftVFTableContext::VFPtrListTy VFPtrs = VFTContext.getVFPtrOffsets(RD);
+  MicrosoftVTableContext::VFPtrListTy VFPtrs = VFTContext.getVFPtrOffsets(RD);
   llvm::GlobalVariable::LinkageTypes Linkage = CGM.getVTableLinkage(RD);
 
-  for (MicrosoftVFTableContext::VFPtrListTy::iterator I = VFPtrs.begin(),
+  for (MicrosoftVTableContext::VFPtrListTy::iterator I = VFPtrs.begin(),
        E = VFPtrs.end(); I != E; ++I) {
     llvm::GlobalVariable *VTable = getAddrOfVTable(RD, I->VFPtrFullOffset);
     if (VTable->hasInitializer())
@@ -867,9 +869,9 @@ llvm::GlobalVariable *MicrosoftCXXABI::getAddrOfVTable(const CXXRecordDecl *RD,
 
   llvm::GlobalVariable *&VTable = I->second;
 
-  MicrosoftVFTableContext &VFTContext = CGM.getVFTableContext();
+  MicrosoftVTableContext &VTContext = CGM.getMicrosoftVTableContext();
-  const MicrosoftVFTableContext::VFPtrListTy &VFPtrs =
+  const MicrosoftVTableContext::VFPtrListTy &VFPtrs =
-      VFTContext.getVFPtrOffsets(RD);
+      VTContext.getVFPtrOffsets(RD);
 
   if (DeferredVFTables.insert(RD)) {
     // We haven't processed this record type before.
@@ -895,7 +897,7 @@ llvm::GlobalVariable *MicrosoftCXXABI::getAddrOfVTable(const CXXRecordDecl *RD,
 
     llvm::ArrayType *ArrayType = llvm::ArrayType::get(
         CGM.Int8PtrTy,
-        VFTContext.getVFTableLayout(RD, VFPtrs[J].VFPtrFullOffset)
+        VTContext.getVFTableLayout(RD, VFPtrs[J].VFPtrFullOffset)
             .getNumVTableComponents());
 
     SmallString<256> Name;
@@ -920,8 +922,8 @@ llvm::Value *MicrosoftCXXABI::getVirtualFunctionPointer(CodeGenFunction &CGF,
   llvm::Value *VPtr = adjustThisArgumentForVirtualCall(CGF, GD, This);
   llvm::Value *VTable = CGF.GetVTablePtr(VPtr, Ty);
 
-  MicrosoftVFTableContext::MethodVFTableLocation ML =
+  MicrosoftVTableContext::MethodVFTableLocation ML =
-      CGM.getVFTableContext().getMethodVFTableLocation(GD);
+      CGM.getMicrosoftVTableContext().getMethodVFTableLocation(GD);
   llvm::Value *VFuncPtr =
       Builder.CreateConstInBoundsGEP1_64(VTable, ML.Index, "vfn");
   return Builder.CreateLoad(VFuncPtr);

clang/lib/CodeGen/MicrosoftVBTables.cpp

@@ -195,15 +195,13 @@ void VBTableInfo::EmitVBTableDefinition(
   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(
+  Offsets[0] = llvm::ConstantInt::get(CGM.IntTy, -VBPtrOffset.getQuantity());
-      llvm::ConstantInt::get(CGM.IntTy, -VBPtrOffset.getQuantity()));
 
-  // These are laid out in the same order as in Itanium, which is the same as
+  MicrosoftVTableContext &Context = CGM.getMicrosoftVTableContext();
-  // the order of the vbase iterator.
   for (CXXRecordDecl::base_class_const_iterator I = ReusingBase->vbases_begin(),
        E = ReusingBase->vbases_end(); I != E; ++I) {
     const CXXRecordDecl *VBase = I->getType()->getAsCXXRecordDecl();
@@ -211,7 +209,9 @@ void VBTableInfo::EmitVBTableDefinition(
     assert(!Offset.isNegative());
     // Make it relative to the subobject vbptr.
     Offset -= VBPtrSubobject.getBaseOffset() + VBPtrOffset;
-    Offsets.push_back(llvm::ConstantInt::get(CGM.IntTy, Offset.getQuantity()));
+    unsigned VBIndex = Context.getVBTableIndex(ReusingBase, VBase);
+    assert(Offsets[VBIndex] == 0 && "The same vbindex seen twice?");
+    Offsets[VBIndex] = llvm::ConstantInt::get(CGM.IntTy, Offset.getQuantity());
   }
 
   assert(Offsets.size() ==

clang/test/CodeGenCXX/microsoft-abi-vbtables.cpp

@@ -410,3 +410,70 @@ H h;
 // CHECK-DAG: @"\01??_8B@Test21@@7B@" =
 // CHECK-DAG: @"\01??_8C@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@Test22@@7B@" = linkonce_odr unnamed_addr constant [3 x i32] [i32 0, i32 12, i32 16]
+// CHECK-DAG: @"\01??_8B@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@Test23@@7B@" = linkonce_odr unnamed_addr constant [3 x i32] [i32 0, i32 16, i32 12]
+// CHECK-DAG: @"\01??_8B@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@Test24@@7B@" = linkonce_odr unnamed_addr constant [3 x i32] [i32 0, i32 16, i32 12]
+// CHECK-DAG: @"\01??_8B@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@Test25@@7B@" = linkonce_odr unnamed_addr constant [3 x i32] [i32 -4, i32 16, i32 12]
+// CHECK-DAG: @"\01??_8B@Test25@@7B@" = linkonce_odr unnamed_addr constant [2 x i32] [i32 -4, i32 8]
+}
+
+namespace Test26 {
+struct A { int a; };
+struct B { int b; };
+struct C { int c; };
+struct D : virtual A { int d; };
+struct E : virtual B {
+  virtual void foo();  // Issues a vfptr.
+  int e;
+};
+struct F: virtual C, D, E { int f; };
+F f;
+// F reuses the D's vbptr, even though D is laid out after E.
+// CHECK-DAG: @"\01??_8F@Test26@@7BD@1@@" = linkonce_odr unnamed_addr constant [4 x i32] [i32 0, i32 16, i32 12, i32 20]
+// CHECK-DAG: @"\01??_8F@Test26@@7BE@1@@" = linkonce_odr unnamed_addr constant [2 x i32] [i32 -4, i32 28]
+}