[MS-ABI] Fix alias-avoidance padding between bases

Adds padding between bases or virtual bases in an attempt to avoid 
aliasing of zero-sized sub-objects.  The approach used by the ABI adds 
two more bits of state.  Detailed comments are in the code.  Test cases 
included.

Differential Revision: http://llvm-reviews.chandlerc.com/D2258

llvm-svn: 196602

1 files changed, 70 insertions, 63 deletions

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index a1f6c41ef86..ec791564893 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2019,6 +2019,15 @@ static bool isMsLayout(const RecordDecl* D) {
 //   obvious reason.
 // * When laying out empty non-virtual bases, an extra byte of padding is added
 //   if the non-virtual base before the empty non-virtual base has a vbptr.
+// * The ABI attempts to avoid aliasing of zero sized bases by adding padding
+//   between bases or vbases with specific properties.  The criteria for
+//   additional padding between two bases is that the first base is zero sized
+//   or has a zero sized subobject and the second base is zero sized or leads
+//   with a zero sized base (sharing of vfptrs can reorder the layout of the
+//   so the leading base is not always the first one declared).  The padding
+//   added for bases is 1 byte.  The padding added for vbases depends on the
+//   alignment of the object but is at least 4 bytes (in both 32 and 64 bit
+//   modes).
 
 
 namespace {
@@ -2139,13 +2148,17 @@ public:
   CharUnits PointerAlignment;
   /// \brief Holds an empty base we haven't yet laid out.
   const CXXRecordDecl *LazyEmptyBase;
-  /// \brief Lets us know if the last base we laid out was empty.  Only used
-  /// when adjusting the placement of a last zero-sized base in 64 bit mode.
-  bool LastBaseWasEmpty;
+  /// \brief A pointer to the Layout for the most recent non-virtual base that
+  /// has been laid out.
+  const ASTRecordLayout *PreviousBaseLayout;
   /// \brief Lets us know if we're in 64-bit mode
-  bool Is64BitMode;
-  /// \brief True if the last non-virtual base has a vbptr.
-  bool LastNonVirtualBaseHasVBPtr;
+  bool Is64BitMode : 1;
+  /// \brief True if this class contains a zero sized member or base or a base
+  /// with a zero sized member or base.  Only used for MS-ABI.
+  bool HasZeroSizedSubObject : 1;
+  /// \brief True if this class is zero sized or first base is zero sized or
+  /// has this property.  Only used for MS-ABI.
+  bool LeadsWithZeroSizedBase : 1;
 };
 } // namespace
 
@@ -2164,18 +2177,6 @@ MicrosoftRecordLayoutBuilder::getAdjustedFieldInfo(const FieldDecl *FD) {
   std::pair<CharUnits, CharUnits> FieldInfo =
       Context.getTypeInfoInChars(FD->getType());
 
-  // If we're not on win32 and using ms_struct the field alignment will be wrong
-  // for 64 bit types, so we fix that here.
-  if (FD->getASTContext().getTargetInfo().getTriple().getOS() !=
-      llvm::Triple::Win32) {
-    QualType T = Context.getBaseElementType(FD->getType());
-    if (const BuiltinType *BTy = T->getAs<BuiltinType>()) {
-      CharUnits TypeSize = Context.getTypeSizeInChars(BTy);
-      if (TypeSize > FieldInfo.second)
-        FieldInfo.second = TypeSize;
-    }
-  }
-
   // Respect packed attribute.
   if (FD->hasAttr<PackedAttr>())
     FieldInfo.second = CharUnits::One();
@@ -2188,7 +2189,7 @@ MicrosoftRecordLayoutBuilder::getAdjustedFieldInfo(const FieldDecl *FD) {
     RequiredAlignment = std::max(RequiredAlignment, FieldAlign);
     FieldInfo.second = std::max(FieldInfo.second, FieldAlign);
   }
-  // Respect attributes applied inside field base types.
+  // Respect attributes applied to subobjects of the field.
   if (const RecordType *RT =
       FD->getType()->getBaseElementTypeUnsafe()->getAs<RecordType>()) {
     const ASTRecordLayout &Layout = Context.getASTRecordLayout(RT->getDecl());
@@ -2196,6 +2197,9 @@ MicrosoftRecordLayoutBuilder::getAdjustedFieldInfo(const FieldDecl *FD) {
                                  Layout.getRequiredAlignment());
     FieldInfo.second = std::max(FieldInfo.second,
                                 Layout.getRequiredAlignment());
+    // Track zero-sized subobjects here where it's already avaliable.
+    if (Layout.hasZeroSizedSubObject())
+      HasZeroSizedSubObject = true;
   }
   return FieldInfo;
 }
@@ -2206,10 +2210,12 @@ void MicrosoftRecordLayoutBuilder::initializeLayout(const RecordDecl *RD) {
 
   Size = CharUnits::Zero();
   Alignment = CharUnits::One();
+
   // In 64-bit mode we always perform an alignment step after laying out vbases.
   // In 32-bit mode we do not.  The check to see if we need to perform alignment
   // checks the RequiredAlignment field and performs alignment if it isn't 0.
   RequiredAlignment = Is64BitMode ? CharUnits::One() : CharUnits::Zero();
+  HasZeroSizedSubObject = false;
 
   // Compute the maximum field alignment.
   MaxFieldAlignment = CharUnits::Zero();
@@ -2259,6 +2265,7 @@ MicrosoftRecordLayoutBuilder::initializeCXXLayout(const CXXRecordDecl *RD) {
   SharedVBPtrBase = 0;
   PrimaryBase = 0;
   VirtualAlignment = CharUnits::One();
+  LeadsWithZeroSizedBase = false;
 
   // If the record has a dynamic base class, attempt to choose a primary base
   // class. It is the first (in direct base class order) non-virtual dynamic
@@ -2272,6 +2279,9 @@ MicrosoftRecordLayoutBuilder::initializeCXXLayout(const CXXRecordDecl *RD) {
     // Handle required alignment.
     RequiredAlignment = std::max(RequiredAlignment,
                                  Layout.getRequiredAlignment());
+    // Check for zero sized subobjects
+    if (Layout.hasZeroSizedSubObject())
+      HasZeroSizedSubObject = true;
     // Handle virtual bases.
     if (i->isVirtual()) {
       VirtualAlignment = std::max(VirtualAlignment, getBaseAlignment(Layout));
@@ -2329,13 +2339,13 @@ void MicrosoftRecordLayoutBuilder::layoutVFPtr(const CXXRecordDecl *RD) {
 void
 MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
   LazyEmptyBase = 0;
-  LastBaseWasEmpty = false;
-  LastNonVirtualBaseHasVBPtr = false;
+  PreviousBaseLayout = 0;
 
   // Lay out the primary base first.
   if (PrimaryBase)
     layoutNonVirtualBase(PrimaryBase);
 
+  const CXXRecordDecl *LeadingBase = PrimaryBase;
   // Iterate through the bases and lay out the non-virtual ones.
   for (CXXRecordDecl::base_class_const_iterator i = RD->bases_begin(),
                                                 e = RD->bases_end();
@@ -2344,6 +2354,11 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBases(const CXXRecordDecl *RD) {
       continue;
     const CXXRecordDecl *BaseDecl =
         cast<CXXRecordDecl>(i->getType()->castAs<RecordType>()->getDecl());
+    if (!LeadingBase) {
+      const ASTRecordLayout &Layout = Context.getASTRecordLayout(BaseDecl);
+      LeadsWithZeroSizedBase = Layout.leadsWithZeroSizedBase();
+      LeadingBase = BaseDecl;
+    }
     if (BaseDecl != PrimaryBase)
       layoutNonVirtualBase(BaseDecl);
   }
@@ -2360,16 +2375,14 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(const CXXRecordDecl *RD) {
     Size = Size.RoundUpToAlignment(getBaseAlignment(LazyLayout));
     // If the last non-virtual base has a vbptr we add a byte of padding for no
     // obvious reason.
-    if (LastNonVirtualBaseHasVBPtr)
+    if (PreviousBaseLayout && PreviousBaseLayout->hasVBPtr())
       Size++;
     Bases.insert(std::make_pair(LazyEmptyBase, Size));
     // Empty bases only consume space when followed by another empty base.
-    if (RD && Layout->getNonVirtualSize().isZero()) {
-      LastBaseWasEmpty = true;
+    if (RD && Layout->getNonVirtualSize().isZero())
       Size++;
-    }
     LazyEmptyBase = 0;
-    LastNonVirtualBaseHasVBPtr = false;
+    PreviousBaseLayout = &LazyLayout;
   }
 
   // RD is null when flushing the final lazy base.
@@ -2381,14 +2394,19 @@ MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(const CXXRecordDecl *RD) {
     return;
   }
 
+  // Insert padding between two bases if the left first one is zero sized or
+  // contains a zero sized subobject and the right is zero sized or one leads
+  // with a zero sized base.
+  if (PreviousBaseLayout && PreviousBaseLayout->hasZeroSizedSubObject() &&
+      Layout->leadsWithZeroSizedBase())
+    Size++;
   // Insert the base here.
   CharUnits BaseOffset = Size.RoundUpToAlignment(getBaseAlignment(*Layout));
   Bases.insert(std::make_pair(RD, BaseOffset));
   Size = BaseOffset + Layout->getDataSize();
   // Note: we don't update alignment here because it was accounted
-  // for during initialization.
-  LastBaseWasEmpty = false;
-  LastNonVirtualBaseHasVBPtr = Layout->hasVBPtr();
+  // for during initalization.
+  PreviousBaseLayout = Layout;
 }
 
 void MicrosoftRecordLayoutBuilder::layoutVBPtr(const CXXRecordDecl *RD) {
@@ -2410,10 +2428,11 @@ void MicrosoftRecordLayoutBuilder::layoutVBPtr(const CXXRecordDecl *RD) {
       // on if the second to last base was also zero sized.
       Size += OldSize % BasesAndFieldsAlignment.getQuantity();
     } else {
-      if (Is64BitMode)
-        Size += LastBaseWasEmpty ? CharUnits::One() : CharUnits::Zero();
-      else
+      if (!Is64BitMode)
         Size = OldSize + BasesAndFieldsAlignment;
+      else if (PreviousBaseLayout &&
+               PreviousBaseLayout->getNonVirtualSize().isZero())
+        Size += CharUnits::One();
     }
     updateAlignment(PointerAlignment);
   }
@@ -2529,6 +2548,7 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
     return;
 
   updateAlignment(VirtualAlignment);
+  PreviousBaseLayout = 0;
 
   // Zero-sized v-bases obey the alignment attribute so apply it here.  The
   // alignment attribute is normally accounted for in FinalizeLayout.
@@ -2555,31 +2575,15 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
 
 void MicrosoftRecordLayoutBuilder::layoutVirtualBase(const CXXRecordDecl *RD,
                                                      bool HasVtordisp) {
-  if (LazyEmptyBase) {
-    const ASTRecordLayout &LazyLayout =
-        Context.getASTRecordLayout(LazyEmptyBase);
-    Size = Size.RoundUpToAlignment(getBaseAlignment(LazyLayout));
-    VBases.insert(
-        std::make_pair(LazyEmptyBase, ASTRecordLayout::VBaseInfo(Size, false)));
-    // Empty bases only consume space when followed by another empty base.
-    // The space consumed is in an Alignment sized/aligned block and the v-base
-    // is placed at its alignment offset into the chunk, unless its alignment
-    // is less than 4 bytes, at which it is placed at 4 byte offset in the
-    // chunk.  We have no idea why.
-    if (RD && Context.getASTRecordLayout(RD).getNonVirtualSize().isZero())
-      Size = Size.RoundUpToAlignment(Alignment) + CharUnits::fromQuantity(4);
-    LazyEmptyBase = 0;
-  }
-
-  // RD is null when flushing the final lazy virtual base.
-  if (!RD)
-    return;
-
   const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
-  if (Layout.getNonVirtualSize().isZero() && !HasVtordisp) {
-    LazyEmptyBase = RD;
-    return;
-  }
+  // Insert padding between two bases if the left first one is zero sized or
+  // contains a zero sized subobject and the right is zero sized or one leads
+  // with a zero sized base.  The minimal padding between virtual bases is 4
+  // bytes (in both 32 and 64 bits modes), we don't know why.
+  if (PreviousBaseLayout && PreviousBaseLayout->hasZeroSizedSubObject() &&
+      Layout.leadsWithZeroSizedBase())
+    Size = Size.RoundUpToAlignment(Alignment) +
+        std::max(CharUnits::fromQuantity(4), Layout.getAlignment());
 
   CharUnits BaseNVSize = Layout.getNonVirtualSize();
   CharUnits BaseAlign = getBaseAlignment(Layout);
@@ -2596,17 +2600,19 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBase(const CXXRecordDecl *RD,
   Size = BaseOffset + BaseNVSize;
   // Note: we don't update alignment here because it was accounted for in
   // InitializeLayout.
+
+  PreviousBaseLayout = &Layout;
 }
 
 void MicrosoftRecordLayoutBuilder::finalizeCXXLayout(const CXXRecordDecl *RD) {
-  // Flush the lazy virtual base.
-  layoutVirtualBase(0, false);
-
   if (RD->vbases_begin() == RD->vbases_end() || !RequiredAlignment.isZero())
     Size = Size.RoundUpToAlignment(Alignment);
 
-  if (Size.isZero())
+  if (Size.isZero()) {
+    HasZeroSizedSubObject = true;
+    LeadsWithZeroSizedBase = true;
     Size = Alignment;
+  }
 }
 
 void MicrosoftRecordLayoutBuilder::honorDeclspecAlign(const RecordDecl *RD) {
@@ -2713,8 +2719,9 @@ ASTContext::BuildMicrosoftASTRecordLayout(const RecordDecl *D) const {
         Builder.VBPtrOffset, Builder.DataSize, Builder.FieldOffsets.data(),
         Builder.FieldOffsets.size(), Builder.DataSize,
         Builder.NonVirtualAlignment, CharUnits::Zero(), Builder.PrimaryBase,
-        false, Builder.SharedVBPtrBase, Builder.Bases,
-        Builder.VBases);
+        false, Builder.SharedVBPtrBase,
+        Builder.HasZeroSizedSubObject, Builder.LeadsWithZeroSizedBase,
+        Builder.Bases, Builder.VBases);
   } else {
     Builder.layout(D);
     return new (*this) ASTRecordLayout(
@@ -2783,7 +2790,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const {
                                   EmptySubobjects.SizeOfLargestEmptySubobject,
                                   Builder.PrimaryBase,
                                   Builder.PrimaryBaseIsVirtual,
-                                  0,
+                                  0, false, false,
                                   Builder.Bases, Builder.VBases);
   } else {
     RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0);