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: <ASTRecordLayout + Size:40 FieldOffsets: [0, 32]> *** Dumping AST Record Layout Type: class B<1> Layout: <ASTRecordLayout + Size:40 FieldOffsets: [0, 32]> *** Dumping AST Record Layout Type: class C Layout: <ASTRecordLayout + Size:88 FieldOffsets: [80]> + +*** Dumping AST Record Layout +Type: class D + +Layout: <ASTRecordLayout + Size:120 + FieldOffsets: [32]> 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<int I> 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