[cfe-commits] r155905 - in /cfe/trunk: include/clang/AST/RecordLayout.h lib/AST/RecordLayout.cpp lib/AST/RecordLayoutBuilder.cpp lib/CodeGen/CGRecordLayoutBuilder.cpp test/Sema/ms_class_layout.cpp

John McCall rjmccall at apple.com
Tue May 1 01:55:32 PDT 2012


Author: rjmccall
Date: Tue May  1 03:55:32 2012
New Revision: 155905

URL: http://llvm.org/viewvc/llvm-project?rev=155905&view=rev
Log:
Add support for laying out vtordisps according to our current
working knowledge of the Microsoft ABI.  Based on a patch by
Dmitry Sokolov.

Modified:
    cfe/trunk/include/clang/AST/RecordLayout.h
    cfe/trunk/lib/AST/RecordLayout.cpp
    cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
    cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
    cfe/trunk/test/Sema/ms_class_layout.cpp

Modified: cfe/trunk/include/clang/AST/RecordLayout.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecordLayout.h?rev=155905&r1=155904&r2=155905&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/RecordLayout.h (original)
+++ cfe/trunk/include/clang/AST/RecordLayout.h Tue May  1 03:55:32 2012
@@ -33,18 +33,43 @@
 /// ObjCInterfaceDecl. FIXME - Find appropriate name.
 /// These objects are managed by ASTContext.
 class ASTRecordLayout {
+public:
+  struct VBaseInfo {
+    /// The offset to this virtual base in the complete-object layout
+    /// of this class.
+    CharUnits VBaseOffset;
+
+  private:
+    /// Whether this virtual base requires a vtordisp field in the
+    /// Microsoft ABI.  These fields are required for certain operations
+    /// in constructors and destructors.
+    bool HasVtorDisp;
+
+  public:
+    bool hasVtorDisp() const { return HasVtorDisp; }
+
+    VBaseInfo() : HasVtorDisp(false) {}
+
+    VBaseInfo(CharUnits VBaseOffset, bool hasVtorDisp) :
+     VBaseOffset(VBaseOffset), HasVtorDisp(hasVtorDisp) {}
+  };
+
+  typedef llvm::DenseMap<const CXXRecordDecl *, VBaseInfo>
+    VBaseOffsetsMapTy;
+
+private:
   /// Size - Size of record in characters.
   CharUnits Size;
 
   /// DataSize - Size of record in characters without tail padding.
   CharUnits DataSize;
 
-  /// FieldOffsets - Array of field offsets in bits.
-  uint64_t *FieldOffsets;
-
   // Alignment - Alignment of record in characters.
   CharUnits Alignment;
 
+  /// FieldOffsets - Array of field offsets in bits.
+  uint64_t *FieldOffsets;
+
   // FieldCount - Number of fields.
   unsigned FieldCount;
 
@@ -63,11 +88,13 @@
     /// any empty subobjects.
     CharUnits SizeOfLargestEmptySubobject;
 
-    /// VFPtrOffset - Virtual function table offset (Microsoft-only).
-    CharUnits VFPtrOffset;
-
     /// VBPtrOffset - Virtual base table offset (Microsoft-only).
     CharUnits VBPtrOffset;
+
+    /// HasOwnVFPtr - Does this class provide a virtual function table
+    /// (vtable in Itanium, vftbl in Microsoft) that is independent from
+    /// its base classes?
+    bool HasOwnVFPtr; // TODO: stash this somewhere more efficient
     
     /// PrimaryBase - The primary base info for this record.
     llvm::PointerIntPair<const CXXRecordDecl *, 1, bool> PrimaryBase;
@@ -79,7 +106,7 @@
     BaseOffsetsMapTy BaseOffsets;
 
     /// VBaseOffsets - Contains a map from vbase classes to their offset.
-    BaseOffsetsMapTy VBaseOffsets;
+    VBaseOffsetsMapTy VBaseOffsets;
   };
 
   /// CXXInfo - If the record layout is for a C++ record, this will have
@@ -96,7 +123,7 @@
   typedef CXXRecordLayoutInfo::BaseOffsetsMapTy BaseOffsetsMapTy;
   ASTRecordLayout(const ASTContext &Ctx,
                   CharUnits size, CharUnits alignment,
-                  CharUnits vfptroffset, CharUnits vbptroffset,
+                  bool hasOwnVFPtr, CharUnits vbptroffset,
                   CharUnits datasize,
                   const uint64_t *fieldoffsets, unsigned fieldcount,
                   CharUnits nonvirtualsize, CharUnits nonvirtualalign,
@@ -104,7 +131,7 @@
                   const CXXRecordDecl *PrimaryBase,
                   bool IsPrimaryBaseVirtual,
                   const BaseOffsetsMapTy& BaseOffsets,
-                  const BaseOffsetsMapTy& VBaseOffsets);
+                  const VBaseOffsetsMapTy& VBaseOffsets);
 
   ~ASTRecordLayout() {}
 
@@ -180,7 +207,7 @@
     assert(CXXInfo && "Record layout does not have C++ specific info!");
     assert(CXXInfo->VBaseOffsets.count(VBase) && "Did not find base!");
 
-    return CXXInfo->VBaseOffsets[VBase];
+    return CXXInfo->VBaseOffsets[VBase].VBaseOffset;
   }
 
   /// getBaseClassOffsetInBits - Get the offset, in bits, for the given
@@ -208,11 +235,16 @@
     return CXXInfo->SizeOfLargestEmptySubobject;
   }
 
-  /// getVFPtrOffset - Get the offset for virtual function table pointer.
-  /// This is only meaningful with the Microsoft ABI.
-  CharUnits getVFPtrOffset() const {
+  /// hasOwnVFPtr - Does this class provide its own virtual-function
+  /// table pointer, rather than inheriting one from a primary base
+  /// class?  If so, it is at offset zero.
+  ///
+  /// This implies that the ABI has no primary base class, meaning
+  /// that it has no base classes that are suitable under the conditions
+  /// of the ABI.
+  bool hasOwnVFPtr() const {
     assert(CXXInfo && "Record layout does not have C++ specific info!");
-    return CXXInfo->VFPtrOffset;
+    return CXXInfo->HasOwnVFPtr;
   }
 
   /// getVBPtrOffset - Get the offset for virtual base table pointer.
@@ -221,6 +253,11 @@
     assert(CXXInfo && "Record layout does not have C++ specific info!");
     return CXXInfo->VBPtrOffset;
   }
+
+  const VBaseOffsetsMapTy &getVBaseOffsetsMap() const {
+    assert(CXXInfo && "Record layout does not have C++ specific info!");
+    return CXXInfo->VBaseOffsets;
+  }
 };
 
 }  // end namespace clang

Modified: cfe/trunk/lib/AST/RecordLayout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayout.cpp?rev=155905&r1=155904&r2=155905&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayout.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayout.cpp Tue May  1 03:55:32 2012
@@ -32,7 +32,7 @@
                                  CharUnits alignment, CharUnits datasize,
                                  const uint64_t *fieldoffsets,
                                  unsigned fieldcount)
-  : Size(size), DataSize(datasize), FieldOffsets(0), Alignment(alignment),
+  : Size(size), DataSize(datasize), Alignment(alignment), FieldOffsets(0),
     FieldCount(fieldcount), CXXInfo(0) {
   if (FieldCount > 0)  {
     FieldOffsets = new (Ctx) uint64_t[FieldCount];
@@ -43,7 +43,7 @@
 // Constructor for C++ records.
 ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx,
                                  CharUnits size, CharUnits alignment,
-                                 CharUnits vfptroffset, CharUnits vbptroffset,
+                                 bool hasOwnVFPtr, CharUnits vbptroffset,
                                  CharUnits datasize,
                                  const uint64_t *fieldoffsets,
                                  unsigned fieldcount,
@@ -53,8 +53,8 @@
                                  const CXXRecordDecl *PrimaryBase,
                                  bool IsPrimaryBaseVirtual,
                                  const BaseOffsetsMapTy& BaseOffsets,
-                                 const BaseOffsetsMapTy& VBaseOffsets)
-  : Size(size), DataSize(datasize), FieldOffsets(0), Alignment(alignment),
+                                 const VBaseOffsetsMapTy& VBaseOffsets)
+  : Size(size), DataSize(datasize), Alignment(alignment), FieldOffsets(0),
     FieldCount(fieldcount), CXXInfo(new (Ctx) CXXRecordLayoutInfo)
 {
   if (FieldCount > 0)  {
@@ -69,7 +69,7 @@
   CXXInfo->SizeOfLargestEmptySubobject = SizeOfLargestEmptySubobject;
   CXXInfo->BaseOffsets = BaseOffsets;
   CXXInfo->VBaseOffsets = VBaseOffsets;
-  CXXInfo->VFPtrOffset = vfptroffset;
+  CXXInfo->HasOwnVFPtr = hasOwnVFPtr;
   CXXInfo->VBPtrOffset = vbptroffset;
 
 #ifndef NDEBUG

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=155905&r1=155904&r2=155905&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue May  1 03:55:32 2012
@@ -538,6 +538,8 @@
   }
 }
 
+typedef llvm::SmallPtrSet<const CXXRecordDecl*, 4> ClassSetTy;
+
 class RecordLayoutBuilder {
 protected:
   // FIXME: Remove this and make the appropriate fields public.
@@ -600,8 +602,9 @@
   /// out is virtual.
   bool PrimaryBaseIsVirtual;
 
-  /// VFPtrOffset - Virtual function table offset. Only for MS layout.
-  CharUnits VFPtrOffset;
+  /// HasOwnVFPtr - Whether the class provides its own vtable/vftbl
+  /// pointer, as opposed to inheriting one from a primary base class.
+  bool HasOwnVFPtr;
 
   /// VBPtrOffset - Virtual base table offset. Only for MS layout.
   CharUnits VBPtrOffset;
@@ -612,7 +615,7 @@
   BaseOffsetsMapTy Bases;
 
   // VBases - virtual base classes and their offsets in the record.
-  BaseOffsetsMapTy VBases;
+  ASTRecordLayout::VBaseOffsetsMapTy VBases;
 
   /// IndirectPrimaryBases - Virtual base classes, direct or indirect, that are
   /// primary base classes for some other direct or indirect base class.
@@ -652,7 +655,7 @@
       NonVirtualAlignment(CharUnits::One()), 
       ZeroLengthBitfield(0), PrimaryBase(0), 
       PrimaryBaseIsVirtual(false),
-      VFPtrOffset(CharUnits::fromQuantity(-1)),
+      HasOwnVFPtr(false),
       VBPtrOffset(CharUnits::fromQuantity(-1)),
       FirstNearlyEmptyVBase(0) { }
 
@@ -725,15 +728,20 @@
                                     CharUnits Offset);
 
   bool needsVFTable(const CXXRecordDecl *RD) const;
-  bool hasNewVirtualFunction(const CXXRecordDecl *RD) const;
+  bool hasNewVirtualFunction(const CXXRecordDecl *RD,
+                             bool IgnoreDestructor = false) const;
   bool isPossiblePrimaryBase(const CXXRecordDecl *Base) const;
 
+  void computeVtordisps(const CXXRecordDecl *RD, 
+                        ClassSetTy &VtordispVBases);
+
   /// LayoutVirtualBases - Lays out all the virtual bases.
   void LayoutVirtualBases(const CXXRecordDecl *RD,
                           const CXXRecordDecl *MostDerivedClass);
 
   /// LayoutVirtualBase - Lays out a single virtual base.
-  void LayoutVirtualBase(const BaseSubobjectInfo *Base);
+  void LayoutVirtualBase(const BaseSubobjectInfo *Base, 
+                         bool IsVtordispNeed = false);
 
   /// LayoutBase - Will lay out a base and return the offset where it was
   /// placed, in chars.
@@ -1044,8 +1052,7 @@
     CharUnits PtrAlign = 
       Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerAlign(0));
     EnsureVTablePointerAlignment(PtrAlign);
-    if (isMicrosoftCXXABI())
-      VFPtrOffset = getSize();
+    HasOwnVFPtr = true;
     setSize(getSize() + PtrWidth);
     setDataSize(getSize());
   }
@@ -1142,7 +1149,7 @@
       assert(!VBases.count(Info->PrimaryVirtualBaseInfo->Class) && 
              "primary vbase offset already exists!");
       VBases.insert(std::make_pair(Info->PrimaryVirtualBaseInfo->Class,
-                                   Offset));
+                                   ASTRecordLayout::VBaseInfo(Offset, false)));
 
       // Traverse the primary virtual base.
       AddPrimaryVirtualBaseOffsets(Info->PrimaryVirtualBaseInfo, Offset);
@@ -1193,19 +1200,177 @@
   return hasNewVirtualFunction(RD);
 }
 
+/// Does the given class inherit non-virtually from any of the classes
+/// in the given set?
+static bool hasNonVirtualBaseInSet(const CXXRecordDecl *RD, 
+                                   const ClassSetTy &set) {
+  for (CXXRecordDecl::base_class_const_iterator
+         I = RD->bases_begin(), E = RD->bases_end(); I != E; ++I) {
+    // Ignore virtual links.
+    if (I->isVirtual()) continue;
+
+    // Check whether the set contains the base.
+    const CXXRecordDecl *base = I->getType()->getAsCXXRecordDecl();
+    if (set.count(base))
+      return true;
+
+    // Otherwise, recurse and propagate.
+    if (hasNonVirtualBaseInSet(base, set))
+      return true;
+  }
+
+  return false;
+}
+
+/// Does the given method (B::foo()) already override a method (A::foo())
+/// such that A requires a vtordisp in B?  If so, we don't need to add a
+/// new vtordisp for B in a yet-more-derived class C providing C::foo().
+static bool overridesMethodRequiringVtorDisp(const ASTContext &Context,
+                                             const CXXMethodDecl *M) {
+  CXXMethodDecl::method_iterator
+    I = M->begin_overridden_methods(), E = M->end_overridden_methods();
+  if (I == E) return false;
+
+  const ASTRecordLayout::VBaseOffsetsMapTy &offsets =
+    Context.getASTRecordLayout(M->getParent()).getVBaseOffsetsMap();
+  do {
+    const CXXMethodDecl *overridden = *I;
+
+    // If the overridden method's class isn't recognized as a virtual
+    // base in the derived class, ignore it.
+    ASTRecordLayout::VBaseOffsetsMapTy::const_iterator
+      it = offsets.find(overridden->getParent());
+    if (it == offsets.end()) continue;
+
+    // Otherwise, check if the overridden method's class needs a vtordisp.
+    if (it->second.hasVtorDisp()) return true;
+
+  } while (++I != E);
+  return false;
+}                                             
+
+/// In the Microsoft ABI, decide which of the virtual bases require a
+/// vtordisp field.
+void RecordLayoutBuilder::computeVtordisps(const CXXRecordDecl *RD,
+                                           ClassSetTy &vtordispVBases) {
+  // Bail out if we have no virtual bases.
+  assert(RD->getNumVBases());
+
+  // Build up the set of virtual bases that we haven't decided yet.
+  ClassSetTy undecidedVBases;
+  for (CXXRecordDecl::base_class_const_iterator
+         I = RD->vbases_begin(), E = RD->vbases_end(); I != E; ++I) {
+    const CXXRecordDecl *vbase = I->getType()->getAsCXXRecordDecl();
+    undecidedVBases.insert(vbase);
+  }
+  assert(!undecidedVBases.empty());
+
+  // A virtual base requires a vtordisp field in a derived class if it
+  // requires a vtordisp field in a base class.  Walk all the direct
+  // bases and collect this information.
+  for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
+       E = RD->bases_end(); I != E; ++I) {
+    const CXXRecordDecl *base = I->getType()->getAsCXXRecordDecl();
+    const ASTRecordLayout &baseLayout = Context.getASTRecordLayout(base);
+
+    // Iterate over the set of virtual bases provided by this class.
+    for (ASTRecordLayout::VBaseOffsetsMapTy::const_iterator
+           VI = baseLayout.getVBaseOffsetsMap().begin(),
+           VE = baseLayout.getVBaseOffsetsMap().end(); VI != VE; ++VI) {
+      // If it doesn't need a vtordisp in this base, ignore it.
+      if (!VI->second.hasVtorDisp()) continue;
+
+      // If we've already seen it and decided it needs a vtordisp, ignore it.
+      if (!undecidedVBases.erase(VI->first)) 
+        continue;
+
+      // Add it.
+      vtordispVBases.insert(VI->first);
+
+      // Quit as soon as we've decided everything.
+      if (undecidedVBases.empty()) 
+        return;
+    }
+  }
+
+  // Okay, we have virtual bases that we haven't yet decided about.  A
+  // virtual base requires a vtordisp if any the non-destructor
+  // virtual methods declared in this class directly override a method
+  // provided by that virtual base.  (If so, we need to emit a thunk
+  // for that method, to be used in the construction vftable, which
+  // applies an additional 'vtordisp' this-adjustment.)
+
+  // Collect the set of bases directly overridden by any method in this class.
+  // It's possible that some of these classes won't be virtual bases, or won't be
+  // provided by virtual bases, or won't be virtual bases in the overridden
+  // instance but are virtual bases elsewhere.  Only the last matters for what
+  // we're doing, and we can ignore those:  if we don't directly override
+  // a method provided by a virtual copy of a base class, but we do directly
+  // override a method provided by a non-virtual copy of that base class,
+  // then we must indirectly override the method provided by the virtual base,
+  // and so we should already have collected it in the loop above.
+  ClassSetTy overriddenBases;
+  for (CXXRecordDecl::method_iterator
+         M = RD->method_begin(), E = RD->method_end(); M != E; ++M) {
+    // Ignore non-virtual methods and destructors.
+    if (isa<CXXDestructorDecl>(*M) || !M->isVirtual())
+      continue;
+    
+    for (CXXMethodDecl::method_iterator I = M->begin_overridden_methods(),
+          E = M->end_overridden_methods(); I != E; ++I) {
+      const CXXMethodDecl *overriddenMethod = (*I);
+
+      // Ignore methods that override methods from vbases that require
+      // require vtordisps.
+      if (overridesMethodRequiringVtorDisp(Context, overriddenMethod))
+        continue;
+
+      // As an optimization, check immediately whether we're overriding
+      // something from the undecided set.
+      const CXXRecordDecl *overriddenBase = overriddenMethod->getParent();
+      if (undecidedVBases.erase(overriddenBase)) {
+        vtordispVBases.insert(overriddenBase);
+        if (undecidedVBases.empty()) return;
+
+        // We can't 'continue;' here because one of our undecided
+        // vbases might non-virtually inherit from this base.
+        // Consider:
+        //   struct A { virtual void foo(); };
+        //   struct B : A {};
+        //   struct C : virtual A, virtual B { virtual void foo(); };
+        // We need a vtordisp for B here.
+      }
+
+      // Otherwise, just collect it.
+      overriddenBases.insert(overriddenBase);
+    }
+  }
+
+  // Walk the undecided v-bases and check whether they (non-virtually)
+  // provide any of the overridden bases.  We don't need to consider
+  // virtual links because the vtordisp inheres to the layout
+  // subobject containing the base.
+  for (ClassSetTy::const_iterator
+         I = undecidedVBases.begin(), E = undecidedVBases.end(); I != E; ++I) {
+    if (hasNonVirtualBaseInSet(*I, overriddenBases))
+      vtordispVBases.insert(*I);
+  }
+}
+
 /// hasNewVirtualFunction - Does the given polymorphic class declare a
 /// virtual function that does not override a method from any of its
 /// base classes?
 bool 
-RecordLayoutBuilder::hasNewVirtualFunction(const CXXRecordDecl *RD) const {
-  assert(RD->isPolymorphic());
+RecordLayoutBuilder::hasNewVirtualFunction(const CXXRecordDecl *RD, 
+                                           bool IgnoreDestructor) const {
   if (!RD->getNumBases()) 
     return true;
 
   for (CXXRecordDecl::method_iterator method = RD->method_begin();
        method != RD->method_end();
        ++method) {
-    if (method->isVirtual() && !method->size_overridden_methods()) {
+    if (method->isVirtual() && !method->size_overridden_methods() &&
+        !(IgnoreDestructor && method->getKind() == Decl::CXXDestructor)) {
       return true;
     }
   }
@@ -1215,11 +1380,11 @@
 /// isPossiblePrimaryBase - Is the given base class an acceptable
 /// primary base class?
 bool 
-RecordLayoutBuilder::isPossiblePrimaryBase(const CXXRecordDecl *Base) const {
+RecordLayoutBuilder::isPossiblePrimaryBase(const CXXRecordDecl *base) const {
   // In the Itanium ABI, a class can be a primary base class if it has
   // a vtable for any reason.
   if (!isMicrosoftCXXABI())
-    return Base->isDynamicClass();
+    return base->isDynamicClass();
 
   // In the MS ABI, a class can only be a primary base class if it
   // provides a vf-table at a static offset.  That means it has to be
@@ -1228,14 +1393,22 @@
   // base, which we have to guard against.
 
   // First off, it has to have virtual functions.
-  if (!Base->isPolymorphic()) return false;
+  if (!base->isPolymorphic()) return false;
 
-  // If it has no virtual bases, then everything is at a static offset.
-  if (!Base->getNumVBases()) return true;
+  // If it has no virtual bases, then the vfptr must be at a static offset.
+  if (!base->getNumVBases()) return true;
+  
+  // Otherwise, the necessary information is cached in the layout.
+  const ASTRecordLayout &layout = Context.getASTRecordLayout(base);
 
-  // Okay, just ask the base class's layout.
-  return (Context.getASTRecordLayout(Base).getVFPtrOffset()
-            != CharUnits::fromQuantity(-1));
+  // If the base has its own vfptr, it can be a primary base.
+  if (layout.hasOwnVFPtr()) return true;
+
+  // If the base has a primary base class, then it can be a primary base.
+  if (layout.getPrimaryBase()) return true;
+
+  // Otherwise it can't.
+  return false;
 }
 
 void
@@ -1288,10 +1461,12 @@
 }
 
 void RecordLayoutBuilder::MSLayoutVirtualBases(const CXXRecordDecl *RD) {
-
   if (!RD->getNumVBases())
     return;
 
+  ClassSetTy VtordispVBases;
+  computeVtordisps(RD, VtordispVBases);
+  
   // This is substantially simplified because there are no virtual
   // primary bases.
   for (CXXRecordDecl::base_class_const_iterator I = RD->vbases_begin(),
@@ -1299,12 +1474,25 @@
     const CXXRecordDecl *BaseDecl = I->getType()->getAsCXXRecordDecl();
     const BaseSubobjectInfo *BaseInfo = VirtualBaseInfo.lookup(BaseDecl);
     assert(BaseInfo && "Did not find virtual base info!");
-    
-    LayoutVirtualBase(BaseInfo);
+
+    // If this base requires a vtordisp, add enough space for an int field.
+    // This is apparently always 32-bits, even on x64.
+    bool vtordispNeeded = false;
+    if (VtordispVBases.count(BaseDecl)) {
+      CharUnits IntSize = 
+        CharUnits::fromQuantity(Context.getTargetInfo().getIntWidth() / 8);
+
+      setSize(getSize() + IntSize);
+      setDataSize(getSize());
+      vtordispNeeded = true;
+    }
+
+    LayoutVirtualBase(BaseInfo, vtordispNeeded);
   }
 }
 
-void RecordLayoutBuilder::LayoutVirtualBase(const BaseSubobjectInfo *Base) {
+void RecordLayoutBuilder::LayoutVirtualBase(const BaseSubobjectInfo *Base,
+                                            bool IsVtordispNeed) {
   assert(!Base->Derived && "Trying to lay out a primary virtual base!");
   
   // Layout the base.
@@ -1312,9 +1500,11 @@
 
   // Add its base class offset.
   assert(!VBases.count(Base->Class) && "vbase offset already exists!");
-  VBases.insert(std::make_pair(Base->Class, Offset));
-  
-  AddPrimaryVirtualBaseOffsets(Base, Offset);
+  VBases.insert(std::make_pair(Base->Class, 
+                       ASTRecordLayout::VBaseInfo(Offset, IsVtordispNeed)));
+
+  if (!isMicrosoftCXXABI())
+    AddPrimaryVirtualBaseOffsets(Base, Offset);
 }
 
 CharUnits RecordLayoutBuilder::LayoutBase(const BaseSubobjectInfo *Base) {
@@ -1461,8 +1651,8 @@
                                  Context.getTargetInfo().getCharAlign()));
   NonVirtualAlignment = Alignment;
 
-  if (isMicrosoftCXXABI() &&
-      NonVirtualSize != NonVirtualSize.RoundUpToAlignment(Alignment)) {
+  if (isMicrosoftCXXABI()) {
+    if (NonVirtualSize != NonVirtualSize.RoundUpToAlignment(Alignment)) {
     CharUnits AlignMember = 
       NonVirtualSize.RoundUpToAlignment(Alignment) - NonVirtualSize;
 
@@ -1472,9 +1662,9 @@
     NonVirtualSize = Context.toCharUnitsFromBits(
                              llvm::RoundUpToAlignment(getSizeInBits(),
                              Context.getTargetInfo().getCharAlign()));
+    }
 
     MSLayoutVirtualBases(RD);
-
   } else {
     // Lay out the virtual bases and add the primary virtual base offsets.
     LayoutVirtualBases(RD, RD);
@@ -2238,7 +2428,7 @@
     NewEntry =
       new (*this) ASTRecordLayout(*this, Builder.getSize(), 
                                   Builder.Alignment,
-                                  Builder.VFPtrOffset,
+                                  Builder.HasOwnVFPtr,
                                   Builder.VBPtrOffset,
                                   DataSize, 
                                   Builder.FieldOffsets.data(),
@@ -2375,7 +2565,7 @@
   IndentLevel++;
 
   const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase();
-  bool HasVfptr = Layout.getVFPtrOffset() != CharUnits::fromQuantity(-1);
+  bool HasVfptr = Layout.hasOwnVFPtr();
   bool HasVbptr = Layout.getVBPtrOffset() != CharUnits::fromQuantity(-1);
 
   // Vtable pointer.
@@ -2405,7 +2595,7 @@
 
   // vfptr and vbptr (for Microsoft C++ ABI)
   if (HasVfptr) {
-    PrintOffset(OS, Offset + Layout.getVFPtrOffset(), IndentLevel);
+    PrintOffset(OS, Offset, IndentLevel);
     OS << '(' << *RD << " vftable pointer)\n";
   }
   if (HasVbptr) {
@@ -2438,6 +2628,8 @@
     return;
 
   // Dump virtual bases.
+  const ASTRecordLayout::VBaseOffsetsMapTy &vtordisps = 
+    Layout.getVBaseOffsetsMap();
   for (CXXRecordDecl::base_class_const_iterator I = RD->vbases_begin(),
          E = RD->vbases_end(); I != E; ++I) {
     assert(I->isVirtual() && "Found non-virtual class!");
@@ -2445,6 +2637,12 @@
       cast<CXXRecordDecl>(I->getType()->getAs<RecordType>()->getDecl());
 
     CharUnits VBaseOffset = Offset + Layout.getVBaseClassOffset(VBase);
+
+    if (vtordisps.find(VBase)->second.hasVtorDisp()) {
+      PrintOffset(OS, VBaseOffset - CharUnits::fromQuantity(4), IndentLevel);
+      OS << "(vtordisp for vbase " << *VBase << ")\n";
+    }
+
     DumpCXXRecordLayout(OS, VBase, C, VBaseOffset, IndentLevel,
                         VBase == PrimaryBase ?
                         "(primary virtual base)" : "(virtual base)",

Modified: cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp?rev=155905&r1=155904&r2=155905&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp Tue May  1 03:55:32 2012
@@ -714,9 +714,7 @@
     }
 
   // Otherwise, add a vtable / vf-table if the layout says to do so.
-  } else if (Types.getContext().getTargetInfo().getCXXABI() == CXXABI_Microsoft
-               ? Layout.getVFPtrOffset() != CharUnits::fromQuantity(-1)
-               : RD->isDynamicClass()) {
+  } else if (Layout.hasOwnVFPtr()) {
     llvm::Type *FunctionType =
       llvm::FunctionType::get(llvm::Type::getInt32Ty(Types.getLLVMContext()),
                               /*isVarArg=*/true);

Modified: cfe/trunk/test/Sema/ms_class_layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/ms_class_layout.cpp?rev=155905&r1=155904&r2=155905&view=diff
==============================================================================
--- cfe/trunk/test/Sema/ms_class_layout.cpp (original)
+++ cfe/trunk/test/Sema/ms_class_layout.cpp Tue May  1 03:55:32 2012
@@ -97,6 +97,48 @@
 
 struct R {};
 
+class IA {
+public:
+  virtual ~IA(){}
+  virtual void ia() = 0;
+};
+
+class ICh : public virtual IA {
+public:
+  virtual ~ICh(){}
+  virtual void ia(){}
+  virtual void iCh(){}
+};
+
+struct f {
+  virtual int asd() {return -90;}
+};
+
+struct s : public virtual f {
+  virtual ~s(){}
+  int r;
+  virtual int asd() {return -9;}
+};
+
+struct sd : virtual s, virtual ICh {
+  virtual ~sd(){}
+  int q;
+  char y;
+  virtual int asd() {return -1;}
+};
+struct AV { 
+  virtual void foo(); 
+};
+struct BV : AV { 
+};
+struct CV : virtual BV { 
+  CV(); 
+  virtual void foo(); 
+};
+struct DV : BV {
+};
+struct EV : CV, DV {
+};
 #pragma pack(pop)
 
 // This needs only for building layouts. 
@@ -113,6 +155,8 @@
   O* o;
   P* p;
   R* r;
+  sd *h;
+  EV *j;
   return 0;
 }
 
@@ -333,3 +377,132 @@
 // CHECK-NEXT:  nvsize=0, nvalign=1
 
 //CHECK: %struct.R = type { i8 }
+
+// CHECK:       0 | struct f
+// CHECK-NEXT:  0 |   (f vftable pointer)
+// CHECK-NEXT: sizeof=4, dsize=4, align=4
+// CHECK-NEXT: nvsize=4, nvalign=4
+
+// CHECK:       0 | struct s
+// CHECK-NEXT:  0 |   (s vftable pointer)
+// CHECK-NEXT:  4 |   (s vbtable pointer)
+// CHECK-NEXT:  8 |   int r
+// CHECK-NEXT: 12 |   (vtordisp for vbase f)
+// CHECK-NEXT: 16 |   struct f (virtual base)
+// CHECK-NEXT: 16 |     (f vftable pointer)
+// CHECK-NEXT: sizeof=20, dsize=20, align=4
+// CHECK-NEXT: nvsize=12, nvalign=4
+
+// CHECK:       0 | class IA
+// CHECK-NEXT:  0 |   (IA vftable pointer)
+// CHECK-NEXT:  sizeof=4, dsize=4, align=4
+// CHECK-NEXT:  nvsize=4, nvalign=4
+	
+// CHECK:       0 | class ICh
+// CHECK-NEXT:  0 |   (ICh vftable pointer)
+// CHECK-NEXT:  4 |   (ICh vbtable pointer)
+// CHECK-NEXT:  8 |   (vtordisp for vbase IA)
+// CHECK-NEXT: 12 |   class IA (virtual base)
+// CHECK-NEXT: 12 |     (IA vftable pointer)
+// CHECK-NEXT: sizeof=16, dsize=16, align=4
+// CHECK-NEXT: nvsize=8, nvalign=4
+
+// CHECK:       0 | struct sd
+// CHECK-NEXT:  0 |   (sd vbtable pointer)
+// CHECK-NEXT:  4 |   int q
+// CHECK-NEXT:  8 |   char y
+// CHECK-NEXT: 12 |   (vtordisp for vbase f)
+// CHECK-NEXT: 16 |   struct f (virtual base)
+// CHECK-NEXT: 16 |     (f vftable pointer)
+// CHECK-NEXT: 20 |   struct s (virtual base)
+// CHECK-NEXT: 20 |     (s vftable pointer)
+// CHECK-NEXT: 24 |     (s vbtable pointer)
+// CHECK-NEXT: 28 |     int r
+// CHECK-NEXT: 32 |   (vtordisp for vbase IA)
+// CHECK-NEXT: 36 |   class IA (virtual base)
+// CHECK-NEXT: 36 |     (IA vftable pointer)
+// CHECK-NEXT: 40 |   class ICh (virtual base)
+// CHECK-NEXT: 40 |     (ICh vftable pointer)
+// CHECK-NEXT: 44 |     (ICh vbtable pointer)
+// CHECK-NEXT: sizeof=48, dsize=48, align=4
+// CHECK-NEXT: nvsize=12, nvalign=4
+
+// CHECK: %struct.f = type { i32 (...)** }
+// CHECK: %struct.s = type { i32 (...)**, i32*, i32, [4 x i8], %struct.f }
+// CHECK: %class.IA = type { i32 (...)** }
+// CHECK: %class.ICh = type { i32 (...)**, i32*, [4 x i8], %class.IA }
+// CHECK: %struct.sd = type { i32*, i32, i8, [7 x i8], %struct.f, %struct.s.base, [4 x i8], %class.IA, %class.ICh.base }
+
+// CHECK:       0 | struct AV
+// CHECK-NEXT:  0 |   (AV vftable pointer)
+// CHECK-NEXT: sizeof=4, dsize=4, align=4
+// CHECK-NEXT: nvsize=4, nvalign=4
+
+
+// CHECK:       0 | struct BV
+// CHECK-NEXT:  0 |   struct AV (primary base)
+// CHECK-NEXT:  0 |     (AV vftable pointer)
+// CHECK-NEXT: sizeof=4, dsize=4, align=4
+// CHECK-NEXT: nvsize=4, nvalign=4
+
+
+// CHECK:       0 | struct CV
+// CHECK-NEXT:  0 |   (CV vbtable pointer)
+// CHECK-NEXT:  4 |   (vtordisp for vbase BV)
+// CHECK-NEXT:  8 |   struct BV (virtual base)
+// CHECK-NEXT:  8 |     struct AV (primary base)
+// CHECK-NEXT:  8 |       (AV vftable pointer)
+// CHECK-NEXT: sizeof=12, dsize=12, align=4
+// CHECK-NEXT: nvsize=4, nvalign=4
+
+// CHECK: %struct.AV = type { i32 (...)** }
+// CHECK: %struct.BV = type { %struct.AV }
+// CHECK: %struct.CV = type { i32*, [4 x i8], %struct.BV }
+// CHECK: %struct.CV.base = type { i32* }
+
+// CHECK:       0 | struct DV
+// CHECK-NEXT:  0 |   struct BV (primary base)
+// CHECK-NEXT:  0 |     struct AV (primary base)
+// CHECK-NEXT:  0 |       (AV vftable pointer)
+// CHECK-NEXT: sizeof=4, dsize=4, align=4
+// CHECK-NEXT: nvsize=4, nvalign=4
+
+// CHECK: %struct.DV = type { %struct.BV }
+
+// CHECK:       0 | struct EV
+// CHECK-NEXT:  4 |   struct CV (base)
+// CHECK-NEXT:  4 |     (CV vbtable pointer)
+// CHECK-NEXT:  0 |   struct DV (primary base)
+// CHECK-NEXT:  0 |     struct BV (primary base)
+// CHECK-NEXT:  0 |       struct AV (primary base)
+// CHECK-NEXT:  0 |         (AV vftable pointer)
+// CHECK-NEXT:  8 |   (vtordisp for vbase BV)
+// CHECK-NEXT: 12 |   struct BV (virtual base)
+// CHECK-NEXT: 12 |     struct AV (primary base)
+// CHECK-NEXT: 12 |       (AV vftable pointer)
+// CHECK-NEXT: sizeof=16, dsize=16, align=4
+// CHECK-NEXT: nvsize=8, nvalign=4
+
+// CHECK: %struct.EV = type { %struct.DV, %struct.CV.base, [4 x i8], %struct.BV }
+// CHECK: %struct.EV.base = type { %struct.DV, %struct.CV.base }
+
+// Overriding a method means that all the vbases containing that
+// method need a vtordisp.
+namespace test1 {
+  struct A { virtual void foo(); };
+  struct B : A {};
+  struct C : virtual A, virtual B { virtual void foo(); };
+  void test() { C *c; }
+
+// CHECK:        0 | struct test1::C
+// CHECK-NEXT:   0 |   (C vbtable pointer)
+// CHECK-NEXT:   4 |   (vtordisp for vbase A)
+// CHECK-NEXT:   8 |   struct test1::A (virtual base)
+// CHECK-NEXT:   8 |     (A vftable pointer)
+// CHECK-NEXT:  12 |   (vtordisp for vbase B)
+// CHECK-NEXT:  16 |   struct test1::B (virtual base)
+// CHECK-NEXT:  16 |     struct test1::A (primary base)
+// CHECK-NEXT:  16 |       (A vftable pointer)
+// CHECK-NEXT:  sizeof=20, dsize=20, align=4
+// CHECK-NEXT:  nvsize=4, nvalign=4
+}





More information about the cfe-commits mailing list