aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: rsmith, mstorsjo, arphaman, whunt, majnemer.
aleksandr.urakov added a project: clang.

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.


Repository:
  rC Clang

https://reviews.llvm.org/D49227

Files:
  lib/AST/RecordLayoutBuilder.cpp
  test/CodeGenCXX/Inputs/override-bit-field-layout.layout
  test/CodeGenCXX/override-bit-field-layout.cpp


Index: test/CodeGenCXX/override-bit-field-layout.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/override-bit-field-layout.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -w -fdump-record-layouts-simple 
-foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | 
FileCheck %s
+
+// CHECK: Type: struct S
+// CHECK:   FieldOffsets: [0, 11]
+struct S {
+  short a : 3;
+  short b : 5;
+};
+
+void use_structs() {
+  S ss[sizeof(S)];
+}
Index: test/CodeGenCXX/Inputs/override-bit-field-layout.layout
===================================================================
--- /dev/null
+++ test/CodeGenCXX/Inputs/override-bit-field-layout.layout
@@ -0,0 +1,8 @@
+
+*** Dumping AST Record Layout
+Type: struct S
+
+Layout: <ASTRecordLayout
+  Size:16
+  Alignment:16
+  FieldOffsets: [0, 11]>
Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -2677,7 +2677,7 @@
   // 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;
@@ -2689,6 +2689,14 @@
     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);


Index: test/CodeGenCXX/override-bit-field-layout.cpp
===================================================================
--- /dev/null
+++ test/CodeGenCXX/override-bit-field-layout.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s
+
+// CHECK: Type: struct S
+// CHECK:   FieldOffsets: [0, 11]
+struct S {
+  short a : 3;
+  short b : 5;
+};
+
+void use_structs() {
+  S ss[sizeof(S)];
+}
Index: test/CodeGenCXX/Inputs/override-bit-field-layout.layout
===================================================================
--- /dev/null
+++ test/CodeGenCXX/Inputs/override-bit-field-layout.layout
@@ -0,0 +1,8 @@
+
+*** Dumping AST Record Layout
+Type: struct S
+
+Layout: <ASTRecordLayout
+  Size:16
+  Alignment:16
+  FieldOffsets: [0, 11]>
Index: lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -2677,7 +2677,7 @@
   // 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;
@@ -2689,6 +2689,14 @@
     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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D49227: O... Aleksandr Urakov via Phabricator via cfe-commits

Reply via email to