r196549 - Support MS-ABI's concept of "Required Alignment" imposed by

Warren Hunt whunt at google.com
Thu Dec 5 16:01:18 PST 2013


Author: whunt
Date: Thu Dec  5 18:01:17 2013
New Revision: 196549

URL: http://llvm.org/viewvc/llvm-project?rev=196549&view=rev
Log:
Support MS-ABI's concept of "Required Alignment" imposed by 
__declspec(align())

This patch implements required alignment in a way that makes 
__declspec(align()) and #pragma pack play correctly together. In the 
MS-ABI, __declspec(align()) is a hard rule and cannot be overridden by 
#pragma pack. This cases each record to have two interesting alignments 
"preferred alignment" (which matches Itanium's concept of alignment) and 
"required alignment" which is an alignment that must never be violated, 
even in the case of #pragma pack. This patch introduces the concept of 
Required Alignment to the record builder and tracks/uses it 
appropriately. Test cases are included.

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


Added:
    cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
Modified:
    cfe/trunk/include/clang/AST/RecordLayout.h
    cfe/trunk/lib/AST/RecordLayout.cpp
    cfe/trunk/lib/AST/RecordLayoutBuilder.cpp

Modified: cfe/trunk/include/clang/AST/RecordLayout.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/RecordLayout.h?rev=196549&r1=196548&r2=196549&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/RecordLayout.h (original)
+++ cfe/trunk/include/clang/AST/RecordLayout.h Thu Dec  5 18:01:17 2013
@@ -66,6 +66,10 @@ private:
   // Alignment - Alignment of record in characters.
   CharUnits Alignment;
 
+  /// RequiredAlignment - The required alignment of the object.  In the MS-ABI
+  /// the __declspec(align()) trumps #pramga pack and must always be obeyed.
+  CharUnits RequiredAlignment;
+
   /// FieldOffsets - Array of field offsets in bits.
   uint64_t *FieldOffsets;
 
@@ -100,10 +104,6 @@ private:
     /// a primary base class.
     bool HasExtendableVFPtr : 1;
 
-    /// AlignAfterVBases - Force appropriate alignment after virtual bases are
-    /// laid out in MS-C++-ABI.
-    bool AlignAfterVBases : 1;
-    
     /// PrimaryBase - The primary base info for this record.
     llvm::PointerIntPair<const CXXRecordDecl *, 1, bool> PrimaryBase;
 
@@ -127,6 +127,7 @@ private:
   friend class ASTContext;
 
   ASTRecordLayout(const ASTContext &Ctx, CharUnits size, CharUnits alignment,
+                  CharUnits requiredAlignment,
                   CharUnits datasize, const uint64_t *fieldoffsets,
                   unsigned fieldcount);
 
@@ -134,6 +135,7 @@ private:
   typedef CXXRecordLayoutInfo::BaseOffsetsMapTy BaseOffsetsMapTy;
   ASTRecordLayout(const ASTContext &Ctx,
                   CharUnits size, CharUnits alignment,
+                  CharUnits requiredAlignment,
                   bool hasOwnVFPtr, bool hasExtendableVFPtr,
                   CharUnits vbptroffset,
                   CharUnits datasize,
@@ -143,7 +145,6 @@ private:
                   const CXXRecordDecl *PrimaryBase,
                   bool IsPrimaryBaseVirtual,
                   const CXXRecordDecl *BaseSharingVBPtr,
-                  bool ForceAlign,
                   const BaseOffsetsMapTy& BaseOffsets,
                   const VBaseOffsetsMapTy& VBaseOffsets);
 
@@ -267,9 +268,8 @@ public:
     return !CXXInfo->VBPtrOffset.isNegative();
   }
 
-  bool getAlignAfterVBases() const {
-    assert(CXXInfo && "Record layout does not have C++ specific info!");
-    return CXXInfo->AlignAfterVBases;
+  CharUnits getRequiredAlignment() const {
+    return RequiredAlignment;
   }
 
   /// getVBPtrOffset - Get the offset for virtual base table pointer.

Modified: cfe/trunk/lib/AST/RecordLayout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayout.cpp?rev=196549&r1=196548&r2=196549&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayout.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayout.cpp Thu Dec  5 18:01:17 2013
@@ -29,10 +29,13 @@ void ASTRecordLayout::Destroy(ASTContext
 }
 
 ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, CharUnits size,
-                                 CharUnits alignment, CharUnits datasize,
+                                 CharUnits alignment, 
+                                 CharUnits requiredAlignment,
+                                 CharUnits datasize,
                                  const uint64_t *fieldoffsets,
                                  unsigned fieldcount)
-  : Size(size), DataSize(datasize), Alignment(alignment), FieldOffsets(0),
+  : Size(size), DataSize(datasize), Alignment(alignment),
+    RequiredAlignment(requiredAlignment), FieldOffsets(0),
     FieldCount(fieldcount), CXXInfo(0) {
   if (FieldCount > 0)  {
     FieldOffsets = new (Ctx) uint64_t[FieldCount];
@@ -43,6 +46,7 @@ ASTRecordLayout::ASTRecordLayout(const A
 // Constructor for C++ records.
 ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx,
                                  CharUnits size, CharUnits alignment,
+                                 CharUnits requiredAlignment,
                                  bool hasOwnVFPtr, bool hasExtendableVFPtr,
                                  CharUnits vbptroffset,
                                  CharUnits datasize,
@@ -54,10 +58,10 @@ ASTRecordLayout::ASTRecordLayout(const A
                                  const CXXRecordDecl *PrimaryBase,
                                  bool IsPrimaryBaseVirtual,
                                  const CXXRecordDecl *BaseSharingVBPtr,
-                                 bool AlignAfterVBases,
                                  const BaseOffsetsMapTy& BaseOffsets,
                                  const VBaseOffsetsMapTy& VBaseOffsets)
-  : Size(size), DataSize(datasize), Alignment(alignment), FieldOffsets(0),
+  : Size(size), DataSize(datasize), Alignment(alignment),
+    RequiredAlignment(requiredAlignment), FieldOffsets(0),
     FieldCount(fieldcount), CXXInfo(new (Ctx) CXXRecordLayoutInfo)
 {
   if (FieldCount > 0)  {
@@ -76,7 +80,6 @@ ASTRecordLayout::ASTRecordLayout(const A
   CXXInfo->VBPtrOffset = vbptroffset;
   CXXInfo->HasExtendableVFPtr = hasExtendableVFPtr;
   CXXInfo->BaseSharingVBPtr = BaseSharingVBPtr;
-  CXXInfo->AlignAfterVBases = AlignAfterVBases;
 
 
 #ifndef NDEBUG

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=196549&r1=196548&r2=196549&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Thu Dec  5 18:01:17 2013
@@ -2064,6 +2064,7 @@ public:
   void updateAlignment(CharUnits NewAlignment) {
     Alignment = std::max(Alignment, NewAlignment);
   }
+  CharUnits getBaseAlignment(const ASTRecordLayout& Layout);
   /// \brief Gets the size and alignment taking attributes into account.
   std::pair<CharUnits, CharUnits> getAdjustedFieldInfo(const FieldDecl *FD);
   /// \brief Places a field at offset 0.
@@ -2089,9 +2090,9 @@ public:
   SmallVector<uint64_t, 16> FieldOffsets;
   /// \brief The maximum allowed field alignment. This is set by #pragma pack.
   CharUnits MaxFieldAlignment;
-  /// \brief Alignment does not occur for virtual bases unless something
-  /// forces it to by explicitly using __declspec(align())
-  bool AlignAfterVBases : 1;
+  /// \brief The alignment that this record must obey.  This is imposed by
+  /// __declspec(align()) on the record itself or one of its fields or bases.
+  CharUnits RequiredAlignment;
   bool IsUnion : 1;
   /// \brief True if the last field laid out was a bitfield and was not 0
   /// width.
@@ -2148,6 +2149,16 @@ public:
 };
 } // namespace
 
+CharUnits
+MicrosoftRecordLayoutBuilder::getBaseAlignment(const ASTRecordLayout& Layout) {
+  CharUnits BaseAlignment = Layout.getAlignment();
+  if (!MaxFieldAlignment.isZero())
+    BaseAlignment = std::min(BaseAlignment,
+                             std::max(MaxFieldAlignment,
+                                      Layout.getRequiredAlignment()));
+  return BaseAlignment;
+}
+
 std::pair<CharUnits, CharUnits>
 MicrosoftRecordLayoutBuilder::getAdjustedFieldInfo(const FieldDecl *FD) {
   std::pair<CharUnits, CharUnits> FieldInfo =
@@ -2174,9 +2185,18 @@ MicrosoftRecordLayoutBuilder::getAdjuste
   // Respect alignment attributes.
   if (unsigned fieldAlign = FD->getMaxAlignment()) {
     CharUnits FieldAlign = Context.toCharUnitsFromBits(fieldAlign);
-    AlignAfterVBases = true;
+    RequiredAlignment = std::max(RequiredAlignment, FieldAlign);
     FieldInfo.second = std::max(FieldInfo.second, FieldAlign);
   }
+  // Respect attributes applied inside field base types.
+  if (const RecordType *RT =
+      FD->getType()->getBaseElementTypeUnsafe()->getAs<RecordType>()) {
+    const ASTRecordLayout &Layout = Context.getASTRecordLayout(RT->getDecl());
+    RequiredAlignment = std::max(RequiredAlignment,
+                                 Layout.getRequiredAlignment());
+    FieldInfo.second = std::max(FieldInfo.second,
+                                Layout.getRequiredAlignment());
+  }
   return FieldInfo;
 }
 
@@ -2186,7 +2206,10 @@ void MicrosoftRecordLayoutBuilder::initi
 
   Size = CharUnits::Zero();
   Alignment = CharUnits::One();
-  AlignAfterVBases = false;
+  // 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();
 
   // Compute the maximum field alignment.
   MaxFieldAlignment = CharUnits::Zero();
@@ -2236,7 +2259,6 @@ MicrosoftRecordLayoutBuilder::initialize
   SharedVBPtrBase = 0;
   PrimaryBase = 0;
   VirtualAlignment = CharUnits::One();
-  AlignAfterVBases = Is64BitMode;
 
   // 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
@@ -2247,12 +2269,12 @@ MicrosoftRecordLayoutBuilder::initialize
     const CXXRecordDecl *BaseDecl =
         cast<CXXRecordDecl>(i->getType()->getAs<RecordType>()->getDecl());
     const ASTRecordLayout &Layout = Context.getASTRecordLayout(BaseDecl);
-    // Handle forced alignment.
-    if (Layout.getAlignAfterVBases())
-      AlignAfterVBases = true;
+    // Handle required alignment.
+    RequiredAlignment = std::max(RequiredAlignment,
+                                 Layout.getRequiredAlignment());
     // Handle virtual bases.
     if (i->isVirtual()) {
-      VirtualAlignment = std::max(VirtualAlignment, Layout.getAlignment());
+      VirtualAlignment = std::max(VirtualAlignment, getBaseAlignment(Layout));
       HasVBPtr = true;
       continue;
     }
@@ -2266,7 +2288,7 @@ MicrosoftRecordLayoutBuilder::initialize
       SharedVBPtrBase = BaseDecl;
       HasVBPtr = true;
     }
-    updateAlignment(Layout.getAlignment());
+    updateAlignment(getBaseAlignment(Layout));
   }
 
   // Use LayoutFields to compute the alignment of the fields.  The layout
@@ -2335,7 +2357,7 @@ MicrosoftRecordLayoutBuilder::layoutNonV
   if (LazyEmptyBase) {
     const ASTRecordLayout &LazyLayout =
         Context.getASTRecordLayout(LazyEmptyBase);
-    Size = Size.RoundUpToAlignment(LazyLayout.getAlignment());
+    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)
@@ -2360,7 +2382,7 @@ MicrosoftRecordLayoutBuilder::layoutNonV
   }
 
   // Insert the base here.
-  CharUnits BaseOffset = Size.RoundUpToAlignment(Layout->getAlignment());
+  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
@@ -2536,7 +2558,7 @@ void MicrosoftRecordLayoutBuilder::layou
   if (LazyEmptyBase) {
     const ASTRecordLayout &LazyLayout =
         Context.getASTRecordLayout(LazyEmptyBase);
-    Size = Size.RoundUpToAlignment(LazyLayout.getAlignment());
+    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.
@@ -2560,7 +2582,7 @@ void MicrosoftRecordLayoutBuilder::layou
   }
 
   CharUnits BaseNVSize = Layout.getNonVirtualSize();
-  CharUnits BaseAlign = Layout.getAlignment();
+  CharUnits BaseAlign = getBaseAlignment(Layout);
 
   // vtordisps are always 4 bytes (even in 64-bit mode)
   if (HasVtordisp)
@@ -2580,7 +2602,7 @@ void MicrosoftRecordLayoutBuilder::final
   // Flush the lazy virtual base.
   layoutVirtualBase(0, false);
 
-  if (RD->vbases_begin() == RD->vbases_end() || AlignAfterVBases)
+  if (RD->vbases_begin() == RD->vbases_end() || !RequiredAlignment.isZero())
     Size = Size.RoundUpToAlignment(Alignment);
 
   if (Size.isZero())
@@ -2588,9 +2610,10 @@ void MicrosoftRecordLayoutBuilder::final
 }
 
 void MicrosoftRecordLayoutBuilder::honorDeclspecAlign(const RecordDecl *RD) {
-  if (unsigned MaxAlign = RD->getMaxAlignment()) {
-    AlignAfterVBases = true;
-    updateAlignment(Context.toCharUnitsFromBits(MaxAlign));
+  RequiredAlignment = std::max(RequiredAlignment,
+      Context.toCharUnitsFromBits(RD->getMaxAlignment()));
+  if (!RequiredAlignment.isZero()) {
+    updateAlignment(RequiredAlignment);
     Size = Size.RoundUpToAlignment(Alignment);
   }
 }
@@ -2684,19 +2707,19 @@ ASTContext::BuildMicrosoftASTRecordLayou
   if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D)) {
     Builder.cxxLayout(RD);
     return new (*this) ASTRecordLayout(
-        *this, Builder.Size, Builder.Alignment,
+        *this, Builder.Size, Builder.Alignment, Builder.RequiredAlignment,
         Builder.HasExtendableVFPtr && !Builder.PrimaryBase,
         Builder.HasExtendableVFPtr,
         Builder.VBPtrOffset, Builder.DataSize, Builder.FieldOffsets.data(),
         Builder.FieldOffsets.size(), Builder.DataSize,
         Builder.NonVirtualAlignment, CharUnits::Zero(), Builder.PrimaryBase,
-        false, Builder.SharedVBPtrBase, Builder.AlignAfterVBases, Builder.Bases,
+        false, Builder.SharedVBPtrBase, Builder.Bases,
         Builder.VBases);
   } else {
     Builder.layout(D);
     return new (*this) ASTRecordLayout(
-        *this, Builder.Size, Builder.Alignment, Builder.Size,
-        Builder.FieldOffsets.data(), Builder.FieldOffsets.size());
+        *this, Builder.Size, Builder.Alignment, Builder.RequiredAlignment,
+        Builder.Size, Builder.FieldOffsets.data(), Builder.FieldOffsets.size());
   }
 }
 
@@ -2747,6 +2770,8 @@ ASTContext::getASTRecordLayout(const Rec
     NewEntry =
       new (*this) ASTRecordLayout(*this, Builder.getSize(), 
                                   Builder.Alignment,
+                                  /*RequiredAlignment : used by MS-ABI)*/
+                                  Builder.Alignment,
                                   Builder.HasOwnVFPtr,
                                   RD->isDynamicClass(),
                                   CharUnits::fromQuantity(-1),
@@ -2758,7 +2783,7 @@ ASTContext::getASTRecordLayout(const Rec
                                   EmptySubobjects.SizeOfLargestEmptySubobject,
                                   Builder.PrimaryBase,
                                   Builder.PrimaryBaseIsVirtual,
-                                  0, true,
+                                  0,
                                   Builder.Bases, Builder.VBases);
   } else {
     RecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/0);
@@ -2767,6 +2792,8 @@ ASTContext::getASTRecordLayout(const Rec
     NewEntry =
       new (*this) ASTRecordLayout(*this, Builder.getSize(), 
                                   Builder.Alignment,
+                                  /*RequiredAlignment : used by MS-ABI)*/
+                                  Builder.Alignment,
                                   Builder.getSize(),
                                   Builder.FieldOffsets.data(),
                                   Builder.FieldOffsets.size());
@@ -2876,6 +2903,8 @@ ASTContext::getObjCLayout(const ObjCInte
   const ASTRecordLayout *NewEntry =
     new (*this) ASTRecordLayout(*this, Builder.getSize(), 
                                 Builder.Alignment,
+                                /*RequiredAlignment : used by MS-ABI)*/
+                                Builder.Alignment,
                                 Builder.getDataSize(),
                                 Builder.FieldOffsets.data(),
                                 Builder.FieldOffsets.size());

Added: cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp?rev=196549&view=auto
==============================================================================
--- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp (added)
+++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp Thu Dec  5 18:01:17 2013
@@ -0,0 +1,120 @@
+// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple i686-pc-win32 -fdump-record-layouts -fsyntax-only -cxx-abi microsoft %s 2>&1 \
+// RUN:            | FileCheck %s
+// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only -cxx-abi microsoft %s 2>/dev/null \
+// RUN:            | FileCheck %s -check-prefix CHECK-X64
+
+extern "C" int printf(const char *fmt, ...);
+char buffer[419430400];
+
+struct A {
+	char a;
+	A() {
+		printf("A   = %d\n", (int)((char*)this - buffer));
+		printf("A.a = %d\n", (int)((char*)&a - buffer));
+	}
+};
+
+struct B {
+	__declspec(align(4)) long long a;
+	B() {
+		printf("B   = %d\n", (int)((char*)this - buffer));
+		printf("B.a = %d\n", (int)((char*)&a - buffer));
+	}
+};
+
+#pragma pack(push, 2)
+struct X {
+	B a;
+	char b;
+	int c;
+	X() {
+		printf("X   = %d\n", (int)((char*)this - buffer));
+		printf("X.a = %d\n", (int)((char*)&a - buffer));
+		printf("X.b = %d\n", (int)((char*)&b - buffer));
+		printf("X.c = %d\n", (int)((char*)&c - buffer));
+	}
+};
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK:    0 | struct X
+// CHECK:    0 |   struct B a
+// CHECK:    0 |     long long a
+// CHECK:    8 |   char b
+// CHECK:   10 |   int c
+// CHECK:      | [sizeof=16, align=4
+// CHECK:      |  nvsize=16, nvalign=4]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64:    0 | struct X
+// CHECK-X64:    0 |   struct B a
+// CHECK-X64:    0 |     long long a
+// CHECK-X64:    8 |   char b
+// CHECK-X64:   10 |   int c
+// CHECK-X64:      | [sizeof=16, align=4
+// CHECK-X64:      |  nvsize=16, nvalign=4]
+
+struct Y : A, B {
+	char a;
+	int b;
+	Y() {
+		printf("Y   = %d\n", (int)((char*)this - buffer));
+		printf("Y.a = %d\n", (int)((char*)&a - buffer));
+		printf("Y.b = %d\n", (int)((char*)&b - buffer));
+	}
+};
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK:    0 | struct Y
+// CHECK:    0 |   struct A (base)
+// CHECK:    0 |     char a
+// CHECK:    4 |   struct B (base)
+// CHECK:    4 |     long long a
+// CHECK:   12 |   char a
+// CHECK:   14 |   int b
+// CHECK:      | [sizeof=20, align=4
+// CHECK:      |  nvsize=20, nvalign=4]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64:    0 | struct Y
+// CHECK-X64:    0 |   struct A (base)
+// CHECK-X64:    0 |     char a
+// CHECK-X64:    4 |   struct B (base)
+// CHECK-X64:    4 |     long long a
+// CHECK-X64:   12 |   char a
+// CHECK-X64:   14 |   int b
+// CHECK-X64:      | [sizeof=20, align=4
+// CHECK-X64:      |  nvsize=20, nvalign=4]
+
+struct Z : virtual B {
+	char a;
+	int b;
+	Z() {
+		printf("Z   = %d\n", (int)((char*)this - buffer));
+		printf("Z.a = %d\n", (int)((char*)&a - buffer));
+		printf("Z.b = %d\n", (int)((char*)&b - buffer));
+	}
+};
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK:    0 | struct Z
+// CHECK:    0 |   (Z vbtable pointer)
+// CHECK:    4 |   char a
+// CHECK:    6 |   int b
+// CHECK:   12 |   struct B (virtual base)
+// CHECK:   12 |     long long a
+// CHECK:      | [sizeof=20, align=4
+// CHECK:      |  nvsize=10, nvalign=2]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64:    0 | struct Z
+// CHECK-X64:    0 |   (Z vbtable pointer)
+// CHECK-X64:    8 |   char a
+// CHECK-X64:   10 |   int b
+// CHECK-X64:   16 |   struct B (virtual base)
+// CHECK-X64:   16 |     long long a
+// CHECK-X64:      | [sizeof=24, align=4
+// CHECK-X64:      |  nvsize=14, nvalign=2]
+
+#pragma pack(pop)
+
+int a[
+sizeof(X)+
+sizeof(Y)+
+sizeof(Z)];





More information about the cfe-commits mailing list