Override a bit fields layout from an external source

[aleksandr.urakov]

Summary

This patch adds a possibility to use an external layout for bit fields.

The problem occurred while debugging a Windows application with PDB symbols. The structure was as follows:

struct S {
  short a : 3;
  short : 8;
  short b : 5;
}

The problem is that PDB doesn't have information about the unnamed bit field, so the field b was located just behind the field a. But PDB has a valid information about fields offsets, so we can use it to solve the problem.

[smeenai]

I'm not familiar enough with this to review it.

[rsmith]

test/CodeGenCXX/override-bit-field-layout.cpp

6–13

Another relevant test:

struct T {
  virtual void f();
  short x : 3;
};

Because we don't do vfptr layout adjustments when using an external layout source, I think this would have put x at offset 0 instead of at offset 8 prior to your patch.

[aleksandr.urakov]

Thank you!

Yes, I have added a test case for that.

If it's OK, can you commit this for me, please? I have no commit access.

[aleksandr.urakov]

Thank you!

lib/AST/RecordLayoutBuilder.cpp

void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) {
   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 (!IsUnion && LastFieldIsNonZeroWidthBitfield &&
+  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) {
+    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;
+    Alignment = std::max(Alignment, Info.Alignment);
   } 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;
   }

test/CodeGenCXX/Inputs/override-bit-field-layout.layout

+
+*** Dumping AST Record Layout
+Type: struct S1
+
+Layout: <ASTRecordLayout
+  Size:16
+  Alignment:16
+  FieldOffsets: [0, 11]>
+
+*** Dumping AST Record Layout
+Type: struct S2
+
+Layout: <ASTRecordLayout
+  Size:128
+  Alignment:64
+  FieldOffsets: [64]>

test/CodeGenCXX/override-bit-field-layout.cpp

+// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s
+
+// CHECK: Type: struct S1
+// CHECK:   FieldOffsets: [0, 11]
+struct S1 {
+  short a : 3;
+  short b : 5;
+};
+
+// CHECK: Type: struct S2
+// CHECK:   FieldOffsets: [64]
+struct S2 {
+  virtual ~S2() = default;
+  short a : 3;
+};
+
+void use_structs() {
+  S1 s1s[sizeof(S1)];
+  S2 s2s[sizeof(S2)];
+}