r201199 - MS ABI: vptr injection should obey alignment requirements

David Majnemer david.majnemer at gmail.com
Tue Feb 11 16:43:02 PST 2014


Author: majnemer
Date: Tue Feb 11 18:43:02 2014
New Revision: 201199

URL: http://llvm.org/viewvc/llvm-project?rev=201199&view=rev
Log:
MS ABI: vptr injection should obey alignment requirements

vptr injection must inject padding equivalent to the alignment of the
most aligned non-virtual subobject, not the alignment of the enclosing
record.

To fascilitate this change, don't let record layout observe the
alignment of the record until we've injected our vptrs. Also, do not
allow the alignment of vbases to affect required alignment until just
before we insert the vtordisp field.

Modified:
    cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
    cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=201199&r1=201198&r2=201199&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Feb 11 18:43:02 2014
@@ -2277,6 +2277,8 @@ void MicrosoftRecordLayoutBuilder::layou
   initializeLayout(RD);
   layoutFields(RD);
   DataSize = Size = Size.RoundUpToAlignment(Alignment);
+  RequiredAlignment = std::max(
+      RequiredAlignment, Context.toCharUnitsFromBits(RD->getMaxAlignment()));
   finalizeLayout(RD);
 }
 
@@ -2287,6 +2289,8 @@ void MicrosoftRecordLayoutBuilder::cxxLa
   layoutFields(RD);
   injectVPtrs(RD);
   DataSize = Size = Size.RoundUpToAlignment(Alignment);
+  RequiredAlignment = std::max(
+      RequiredAlignment, Context.toCharUnitsFromBits(RD->getMaxAlignment()));
   layoutVirtualBases(RD);
   finalizeLayout(RD);
 }
@@ -2300,8 +2304,6 @@ void MicrosoftRecordLayoutBuilder::initi
   // 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();
-  RequiredAlignment = std::max(RequiredAlignment,
-    Context.toCharUnitsFromBits(RD->getMaxAlignment()));
   // Compute the maximum field alignment.
   MaxFieldAlignment = CharUnits::Zero();
   // Honor the default struct packing maximum alignment flag.
@@ -2352,14 +2354,14 @@ MicrosoftRecordLayoutBuilder::layoutNonV
        i != e; ++i) {
     const CXXRecordDecl *BaseDecl = i->getType()->getAsCXXRecordDecl();
     const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl);
-    // Track RequiredAlignment for all bases in this pass.
-    RequiredAlignment = std::max(RequiredAlignment,
-                                 BaseLayout.getRequiredAlignment());
     // Mark and skip virtual bases.
     if (i->isVirtual()) {
       HasVBPtr = true;
       continue;
     }
+    // Track RequiredAlignment for all bases in this pass.
+    RequiredAlignment = std::max(RequiredAlignment,
+                                 BaseLayout.getRequiredAlignment());
     // Check fo a base to share a VBPtr with.
     if (!SharedVBPtrBase && BaseLayout.hasVBPtr()) {
       SharedVBPtrBase = BaseDecl;
@@ -2525,7 +2527,8 @@ void MicrosoftRecordLayoutBuilder::injec
   CharUnits FieldStart = VBPtrOffset + PointerInfo.Size;
   // Make sure that the amount we push the fields back by is a multiple of the
   // alignment.
-  CharUnits Offset = (FieldStart - InjectionSite).RoundUpToAlignment(Alignment);
+  CharUnits Offset = (FieldStart - InjectionSite).RoundUpToAlignment(
+      std::max(RequiredAlignment, Alignment));
   // Increase the size of the object and push back all fields by the offset
   // amount.
   Size += Offset;
@@ -2547,11 +2550,12 @@ void MicrosoftRecordLayoutBuilder::injec
     return;
   // Make sure that the amount we push the struct back by is a multiple of the
   // alignment.
-  CharUnits Offset = PointerInfo.Size.RoundUpToAlignment(Alignment);
+  CharUnits Offset = PointerInfo.Size.RoundUpToAlignment(
+      std::max(RequiredAlignment, Alignment));
   // Increase the size of the object and push back all fields, the vbptr and all
   // bases by the offset amount.
   Size += Offset;
-  for (SmallVector<uint64_t, 16>::iterator i = FieldOffsets.begin(),
+  for (SmallVectorImpl<uint64_t>::iterator i = FieldOffsets.begin(),
                                            e = FieldOffsets.end();
        i != e; ++i)
     *i += Context.toBits(Offset);
@@ -2646,6 +2650,14 @@ void MicrosoftRecordLayoutBuilder::layou
   // The alignment of the vtordisp is at least the required alignment of the
   // entire record.  This requirement may be present to support vtordisp
   // injection.
+  for (CXXRecordDecl::base_class_const_iterator i = RD->vbases_begin(),
+                                                e = RD->vbases_end();
+       i != e; ++i) {
+    const CXXRecordDecl *BaseDecl = i->getType()->getAsCXXRecordDecl();
+    const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl);
+    RequiredAlignment =
+        std::max(RequiredAlignment, BaseLayout.getRequiredAlignment());
+  }
   VtorDispAlignment = std::max(VtorDispAlignment, RequiredAlignment);
   // Compute the vtordisp set.
   llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtordispSet =

Modified: 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=201199&r1=201198&r2=201199&view=diff
==============================================================================
--- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp (original)
+++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp Tue Feb 11 18:43:02 2014
@@ -371,6 +371,49 @@ struct KB : KA { __declspec(align(2)) ch
 // CHECK-X64-NEXT:      | [sizeof=4, align=2
 // CHECK-X64-NEXT:      |  nvsize=3, nvalign=2]
 
+#pragma pack(1)
+struct L {
+  virtual void fun() {}
+  __declspec(align(256)) int Field;
+};
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:    0 | struct L
+// CHECK-NEXT:    0 |   (L vftable pointer)
+// CHECK-NEXT:  256 |   int Field
+// CHECK-NEXT:      | [sizeof=512, align=256
+// CHECK-NEXT:      |  nvsize=260, nvalign=256]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT:    0 | struct L
+// CHECK-X64-NEXT:    0 |   (L vftable pointer)
+// CHECK-X64-NEXT:  256 |   int Field
+// CHECK-X64-NEXT:      | [sizeof=512, align=256
+// CHECK-X64-NEXT:      |  nvsize=260, nvalign=256]
+
+#pragma pack()
+struct MA {};
+#pragma pack(1)
+struct MB : virtual MA {
+  __declspec(align(256)) int Field;
+};
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:    0 | struct MB
+// CHECK-NEXT:    0 |   (MB vbtable pointer)
+// CHECK-NEXT:  256 |   int Field
+// CHECK-NEXT:  260 |   struct MA (virtual base) (empty)
+// CHECK-NEXT:      | [sizeof=512, align=256
+// CHECK-NEXT:      |  nvsize=260, nvalign=256]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT:    0 | struct MB
+// CHECK-X64-NEXT:    0 |   (MB vbtable pointer)
+// CHECK-X64-NEXT:  256 |   int Field
+// CHECK-X64-NEXT:  260 |   struct MA (virtual base) (empty)
+// CHECK-X64-NEXT:      | [sizeof=512, align=256
+// CHECK-X64-NEXT:      |  nvsize=260, nvalign=256]
+
 int a[
 sizeof(X)+
 sizeof(Y)+
@@ -387,4 +430,6 @@ sizeof(YF)+
 sizeof(D2)+
 sizeof(JC)+
 sizeof(KB)+
+sizeof(L)+
+sizeof(MB)+
 0];





More information about the cfe-commits mailing list