Improve support of PDB as an external layout source

[aleksandr.urakov]

Summary

This patch improves support of PDB as an external layout source in the next cases:

• Multiple non-virtual inheritance from packed base classes. When using external layout, there's no need to align NonVirtualSize of a base class. It may cause an overlapping when the next base classes will be layouted (but there is a slightly different case in the test because I can't find a way to specify a base offset);
• Support of nameless structs and unions. There is no info about nameless child structs and unions in Microsoft cl-emitted PDBs. Instead all its fields are just treated as outer structure's (union's) fields. This also causes a fields overlapping, and makes it possible for unions to have fields located at a non-zero offset.

[rnk]

lgtm

[aleksandr.urakov]

Thank you!

Can you, please, commit this for me? I have no commit access.

cfe/trunk/lib/AST/RecordLayoutBuilder.cpp

void MicrosoftRecordLayoutBuilder::cxxLayout(const CXXRecordDecl *RD) {
   layoutFields(RD);
   injectVBPtr(RD);
   injectVFPtr(RD);
   if (HasOwnVFPtr || (HasVBPtr && !SharedVBPtrBase))
     Alignment = std::max(Alignment, PointerInfo.Alignment);
   auto RoundingAlignment = Alignment;
   if (!MaxFieldAlignment.isZero())
     RoundingAlignment = std::min(RoundingAlignment, MaxFieldAlignment);
-  NonVirtualSize = Size = Size.alignTo(RoundingAlignment);
+  if (!UseExternalLayout)
+    Size = Size.alignTo(RoundingAlignment);
+  NonVirtualSize = Size;
   RequiredAlignment = std::max(
       RequiredAlignment, Context.toCharUnitsFromBits(RD->getMaxAlignment()));
   layoutVirtualBases(RD);
   finalizeLayout(RD);
 }
 
 void MicrosoftRecordLayoutBuilder::initializeLayout(const RecordDecl *RD) {
   IsUnion = RD->isUnion();

 void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) {
   if (FD->isBitField()) {
     layoutBitField(FD);
     return;
   }
   LastFieldIsNonZeroWidthBitfield = false;
   ElementInfo Info = getAdjustedElementInfo(FD);
   Alignment = std::max(Alignment, Info.Alignment);
-  if (IsUnion) {
-    placeFieldAtOffset(CharUnits::Zero());
-    Size = std::max(Size, Info.Size);
-  } else {
-    CharUnits FieldOffset;
-    if (UseExternalLayout) {
-      FieldOffset =
-          Context.toCharUnitsFromBits(External.getExternalFieldOffset(FD));
-      assert(FieldOffset >= Size && "field offset already allocated");
-    } else {
-      FieldOffset = Size.alignTo(Info.Alignment);
-    }
-    placeFieldAtOffset(FieldOffset);
-    Size = FieldOffset + Info.Size;
-  }
+  if (UseExternalLayout)
+  else if (IsUnion)
+    FieldOffset = CharUnits::Zero();
+  else
+  Size = std::max(Size, FieldOffset + Info.Size);
 }
 
 void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) {
   unsigned Width = FD->getBitWidthValue(Context);
   if (Width == 0) {
     layoutZeroWidthBitField(FD);
     return;
   }
   ElementInfo Info = getAdjustedElementInfo(FD);
   // Clamp the bitfield to a containable size for the sake of being able
   // to lay them out.  Sema will throw an error.
   if (Width > Context.toBits(Info.Size))
     Width = Context.toBits(Info.Size);
   // Check to see if this bitfield fits into an existing allocation.  Note:
   // MSVC refuses to pack bitfields of formal types with different sizes
   // into the same allocation.
   if (!UseExternalLayout && !IsUnion && LastFieldIsNonZeroWidthBitfield &&
       CurrentBitfieldSize == Info.Size && Width <= RemainingBitsInField) {
     placeFieldAtBitOffset(Context.toBits(Size) - RemainingBitsInField);
     RemainingBitsInField -= Width;
     return;
   }
   LastFieldIsNonZeroWidthBitfield = true;
   CurrentBitfieldSize = Info.Size;
-  if (IsUnion) {
-    placeFieldAtOffset(CharUnits::Zero());
-    Size = std::max(Size, Info.Size);
-    // TODO: Add a Sema warning that MS ignores bitfield alignment in unions.
-  } else if (UseExternalLayout) {
+  if (UseExternalLayout) {
     auto FieldBitOffset = External.getExternalFieldOffset(FD);
     placeFieldAtBitOffset(FieldBitOffset);
     auto NewSize = Context.toCharUnitsFromBits(
         llvm::alignTo(FieldBitOffset + Width, Context.getCharWidth()));
-    assert(NewSize >= Size && "bit field offset already allocated");
-    Size = NewSize;
+    Size = std::max(Size, NewSize);
     Alignment = std::max(Alignment, Info.Alignment);
+  } else if (IsUnion) {
+    placeFieldAtOffset(CharUnits::Zero());
+    Size = std::max(Size, Info.Size);
+    // TODO: Add a Sema warning that MS ignores bitfield alignment in unions.
   } else {
     // Allocate a new block of memory and place the bitfield in it.
     CharUnits FieldOffset = Size.alignTo(Info.Alignment);
     placeFieldAtOffset(FieldOffset);
     Size = FieldOffset + Info.Size;
     Alignment = std::max(Alignment, Info.Alignment);
     RemainingBitsInField = Context.toBits(Info.Size) - Width;
   }

cfe/trunk/test/CodeGenCXX/Inputs/override-layout-nameless-struct-union.layout

+
+*** Dumping AST Record Layout
+Type: struct S
+
+Layout: <ASTRecordLayout
+  Size:64
+  Alignment:32
+  FieldOffsets: [0, 32, 32]>
+
+*** Dumping AST Record Layout
+Type: union U
+
+Layout: <ASTRecordLayout
+  Size:96
+  Alignment:32
+  FieldOffsets: [0, 0, 32, 64, 68, 73]>

cfe/trunk/test/CodeGenCXX/Inputs/override-layout-packed-base.layout

+
+*** Dumping AST Record Layout
+Type: class B<0>
+
+Layout: <ASTRecordLayout
+  FieldOffsets: [0, 32]>
+
+*** Dumping AST Record Layout
+Type: class B<1>
+
+Layout: <ASTRecordLayout
+  FieldOffsets: [0, 32]>
+
+*** Dumping AST Record Layout
+Type: class C
+
+Layout: <ASTRecordLayout
+  FieldOffsets: [80]>

cfe/trunk/test/CodeGenCXX/override-layout-nameless-struct-union.cpp

+// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-nameless-struct-union.layout %s | FileCheck %s
+
+// CHECK: Type: struct S
+// CHECK:   Size:64
+// CHECK:   Alignment:32
+// CHECK:   FieldOffsets: [0, 32, 32]
+struct S {
+  short _s;
+//union {
+    int _su0;
+    char _su1;
+//};
+};
+
+// CHECK: Type: union U
+// CHECK:   Size:96
+// CHECK:   Alignment:32
+// CHECK:   FieldOffsets: [0, 0, 32, 64, 68, 73]
+union U {
+  short _u;
+//struct {
+    char _us0;
+    int _us1;
+    unsigned _us20 : 4;
+    unsigned _us21 : 5;
+    unsigned _us22 : 6;
+//};
+};
+
+void use_structs() {
+  S ss[sizeof(S)];
+  U us[sizeof(U)];
+}

cfe/trunk/test/CodeGenCXX/override-layout-packed-base.cpp

+// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-packed-base.layout %s | FileCheck %s
+
+// CHECK: Type: class B<0>
+// CHECK:   FieldOffsets: [0, 32]
+
+// CHECK: Type: class B<1>
+// CHECK:   FieldOffsets: [0, 32]
+
+//#pragma pack(push, 1)
+template<int I>
+class B {
+  int _b1;
+  char _b2;
+};
+//#pragma pack(pop)
+
+// CHECK: Type: class C
+// CHECK:   FieldOffsets: [80]
+
+class C : B<0>, B<1> {
+  char _c;
+};
+
+void use_structs() {
+  C cs[sizeof(C)];
+}