r338353 - Improve support of PDB as an external layout source
Author: aleksandr.urakov
Date: Tue Jul 31 01:27:06 2018
New Revision: 338353
URL: http://llvm.org/viewvc/llvm-project?rev=338353&view=rev
Log:
Improve support of PDB as an external layout source
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.
Reviewers: rsmith, zturner, rnk, mstorsjo, majnemer
Reviewed By: rnk
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D49871
Added:
cfe/trunk/test/CodeGenCXX/Inputs/override-layout-nameless-struct-union.layout
cfe/trunk/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
cfe/trunk/test/CodeGenCXX/override-layout-nameless-struct-union.cpp
cfe/trunk/test/CodeGenCXX/override-layout-packed-base.cpp
Modified:
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=338353&r1=338352&r2=338353&view=diff
==
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Jul 31 01:27:06 2018
@@ -2452,7 +2452,9 @@ void MicrosoftRecordLayoutBuilder::cxxLa
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);
@@ -2653,21 +2655,16 @@ void MicrosoftRecordLayoutBuilder::layou
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;
- }
+ CharUnits FieldOffset;
+ if (UseExternalLayout)
+FieldOffset =
+Context.toCharUnitsFromBits(External.getExternalFieldOffset(FD));
+ else if (IsUnion)
+FieldOffset = CharUnits::Zero();
+ else
+FieldOffset = Size.alignTo(Info.Alignment);
+ placeFieldAtOffset(FieldOffset);
+ Size = std::max(Size, FieldOffset + Info.Size);
}
void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) {
@@ -2692,18 +2689,17 @@ void MicrosoftRecordLayoutBuilder::layou
}
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);
Added:
cfe/trunk/test/CodeGenCXX/Inputs/override-layout-nameless-struct-union.layout
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/Inputs/override-layout-nameless-struct-union.layout?rev=338353&view=auto
==
---
cfe/trunk/test/CodeGenCXX/Inputs/override-layout-namel
r345012 - [AST] Do not align virtual bases in `MicrosoftRecordLayoutBuilder` when
Author: aleksandr.urakov
Date: Tue Oct 23 01:23:22 2018
New Revision: 345012
URL: http://llvm.org/viewvc/llvm-project?rev=345012&view=rev
Log:
[AST] Do not align virtual bases in `MicrosoftRecordLayoutBuilder` when
an external layout is used
Summary:
The patch removes alignment of virtual bases when an external layout is used.
We have two cases:
- the external layout source has an information about virtual bases offsets,
so we just use them;
- the external source has no information about virtual bases offsets. In this
case we can't predict where the base will be located. If we will align it but
there will be something like `#pragma pack(push, 1)` really, then likely our
layout will not fit into the real structure size, and then some asserts will
hit. The asserts look reasonable, so I don't think that we need to remove
them. May be it would be better instead don't align fields / bases etc.
(so treat it always as `#pragma pack(push, 1)`) when an external layout source
is used but no info about a field location is presented.
This one is related to D49871
Reviewers: rnk, rsmith, zturner, mstorsjo, majnemer
Reviewed By: rnk
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D53497
Modified:
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
cfe/trunk/test/CodeGenCXX/override-layout-packed-base.cpp
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=345012&r1=345011&r2=345012&view=diff
==
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Oct 23 01:23:22 2018
@@ -2829,15 +2829,14 @@ void MicrosoftRecordLayoutBuilder::layou
CharUnits BaseOffset;
// Respect the external AST source base offset, if present.
-bool FoundBase = false;
if (UseExternalLayout) {
- FoundBase = External.getExternalVBaseOffset(BaseDecl, BaseOffset);
- if (FoundBase)
-assert(BaseOffset >= Size && "base offset already allocated");
-}
-if (!FoundBase)
+ if (!External.getExternalVBaseOffset(BaseDecl, BaseOffset))
+BaseOffset = Size;
+} else
BaseOffset = Size.alignTo(Info.Alignment);
+assert(BaseOffset >= Size && "base offset already allocated");
+
VBases.insert(std::make_pair(BaseDecl,
ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp)));
Size = BaseOffset + BaseLayout.getNonVirtualSize();
Modified: cfe/trunk/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/Inputs/override-layout-packed-base.layout?rev=345012&r1=345011&r2=345012&view=diff
==
--- cfe/trunk/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
(original)
+++ cfe/trunk/test/CodeGenCXX/Inputs/override-layout-packed-base.layout Tue Oct
23 01:23:22 2018
@@ -3,16 +3,26 @@
Type: class B<0>
Layout:
*** Dumping AST Record Layout
Type: class B<1>
Layout:
*** Dumping AST Record Layout
Type: class C
Layout:
+
+*** Dumping AST Record Layout
+Type: class D
+
+Layout:
Modified: cfe/trunk/test/CodeGenCXX/override-layout-packed-base.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/override-layout-packed-base.cpp?rev=345012&r1=345011&r2=345012&view=diff
==
--- cfe/trunk/test/CodeGenCXX/override-layout-packed-base.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/override-layout-packed-base.cpp Tue Oct 23
01:23:22 2018
@@ -1,26 +1,40 @@
-// RUN: %clang_cc1 -w -fdump-record-layouts-simple
-foverride-record-layout=%S/Inputs/override-layout-packed-base.layout %s |
FileCheck %s
+// RUN: %clang_cc1 -triple i686-windows-msvc -w -fdump-record-layouts-simple
-foverride-record-layout=%S/Inputs/override-layout-packed-base.layout %s |
FileCheck %s
+
+//#pragma pack(push, 1)
// CHECK: Type: class B<0>
+// CHECK: Size:40
// CHECK: FieldOffsets: [0, 32]
// CHECK: Type: class B<1>
+// CHECK: Size:40
// CHECK: FieldOffsets: [0, 32]
-//#pragma pack(push, 1)
template
class B {
int _b1;
char _b2;
};
-//#pragma pack(pop)
// CHECK: Type: class C
+// CHECK: Size:88
// CHECK: FieldOffsets: [80]
class C : B<0>, B<1> {
char _c;
};
+// CHECK: Type: class D
+// CHECK: Size:120
+// CHECK: FieldOffsets: [32]
+
+class D : virtual B<0>, virtual B<1> {
+ char _d;
+};
+
+//#pragma pack(pop)
+
void use_structs() {
C cs[sizeof(C)];
+ D ds[sizeof(D)];
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r356047 - [AST] Improve support of external layouts in `MicrosoftRecordLayoutBuilder`
Author: aleksandr.urakov
Date: Wed Mar 13 06:38:12 2019
New Revision: 356047
URL: http://llvm.org/viewvc/llvm-project?rev=356047&view=rev
Log:
[AST] Improve support of external layouts in `MicrosoftRecordLayoutBuilder`
Summary:
This patch fixes several small problems with external layouts support in
`MicrosoftRecordLayoutBuilder`:
- aligns properly the size of a struct that ends with a bit field. It was
aligned on byte before, not on the size of the field, so the struct size was
smaller than it should be;
- adjusts the struct size when injecting a vbptr in the case when there were no
bases or fields allocated after the vbptr. Similarly, without the adjustment
the struct was smaller than it should be;
- the same fix as above for the vfptr.
All these fixes affect the non-virtual size of a struct, so they are tested
through non-virtual inheritance.
Reviewers: rnk, zturner, rsmith
Reviewed By: rnk
Subscribers: jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58544
Added:
cfe/trunk/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
cfe/trunk/test/CodeGenCXX/override-layout-virtual-base.cpp
Modified:
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/test/CodeGenCXX/Inputs/override-bit-field-layout.layout
cfe/trunk/test/CodeGenCXX/override-bit-field-layout.cpp
cfe/trunk/test/CodeGenCXX/override-layout.cpp
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=356047&r1=356046&r2=356047&view=diff
==
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Wed Mar 13 06:38:12 2019
@@ -2692,7 +2692,8 @@ void MicrosoftRecordLayoutBuilder::layou
auto FieldBitOffset = External.getExternalFieldOffset(FD);
placeFieldAtBitOffset(FieldBitOffset);
auto NewSize = Context.toCharUnitsFromBits(
-llvm::alignTo(FieldBitOffset + Width, Context.getCharWidth()));
+llvm::alignDown(FieldBitOffset, Context.toBits(Info.Alignment)) +
+Context.toBits(Info.Size));
Size = std::max(Size, NewSize);
Alignment = std::max(Alignment, Info.Alignment);
} else if (IsUnion) {
@@ -2741,12 +2742,17 @@ void MicrosoftRecordLayoutBuilder::injec
CharUnits InjectionSite = VBPtrOffset;
// But before we do, make sure it's properly aligned.
VBPtrOffset = VBPtrOffset.alignTo(PointerInfo.Alignment);
+ // Determine where the first field should be laid out after the vbptr.
+ CharUnits FieldStart = VBPtrOffset + PointerInfo.Size;
// Shift everything after the vbptr down, unless we're using an external
// layout.
- if (UseExternalLayout)
+ if (UseExternalLayout) {
+// It is possible that there were no fields or bases located after vbptr,
+// so the size was not adjusted before.
+if (Size < FieldStart)
+ Size = FieldStart;
return;
- // Determine where the first field should be laid out after the vbptr.
- 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)
@@ -2771,8 +2777,14 @@ void MicrosoftRecordLayoutBuilder::injec
if (HasVBPtr)
VBPtrOffset += Offset;
- if (UseExternalLayout)
+ if (UseExternalLayout) {
+// The class may have no bases or fields, but still have a vfptr
+// (e.g. it's an interface class). The size was not correctly set before
+// in this case.
+if (FieldOffsets.empty() && Bases.empty())
+ Size += Offset;
return;
+ }
Size += Offset;
Modified: cfe/trunk/test/CodeGenCXX/Inputs/override-bit-field-layout.layout
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/Inputs/override-bit-field-layout.layout?rev=356047&r1=356046&r2=356047&view=diff
==
--- cfe/trunk/test/CodeGenCXX/Inputs/override-bit-field-layout.layout (original)
+++ cfe/trunk/test/CodeGenCXX/Inputs/override-bit-field-layout.layout Wed Mar
13 06:38:12 2019
@@ -14,3 +14,11 @@ Layout:
+
+*** Dumping AST Record Layout
+Type: struct S3
+
+Layout:
Added: cfe/trunk/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout?rev=356047&view=auto
==
--- cfe/trunk/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout (added)
+++ cfe/trunk/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout Wed
Mar 13 06:38:12 2019
@@ -0,0 +1,8 @@
+
+*** Dumping AST Record Layout
+Type: struct S2
+
+Layout:
Modified: cfe/trunk/test/CodeGenCXX/override-bit-field-layout.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/override-b
