From: Aleksandr Urakov Date: Wed, 13 Mar 2019 13:38:12 +0000 (+0000) Subject: [AST] Improve support of external layouts in `MicrosoftRecordLayoutBuilder` X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=849c257a9b781ca898bc807b97f4494b9a8de218;p=clang [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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@356047 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp index 99b7cbd022..2d112c4522 100644 --- a/lib/AST/RecordLayoutBuilder.cpp +++ b/lib/AST/RecordLayoutBuilder.cpp @@ -2692,7 +2692,8 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { 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::injectVBPtr(const CXXRecordDecl *RD) { 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::injectVFPtr(const CXXRecordDecl *RD) { 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; diff --git a/test/CodeGenCXX/Inputs/override-bit-field-layout.layout b/test/CodeGenCXX/Inputs/override-bit-field-layout.layout index 8e67dce650..b57c6efbc1 100644 --- a/test/CodeGenCXX/Inputs/override-bit-field-layout.layout +++ b/test/CodeGenCXX/Inputs/override-bit-field-layout.layout @@ -14,3 +14,11 @@ Layout: + +*** Dumping AST Record Layout +Type: struct S3 + +Layout: diff --git a/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout b/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout new file mode 100644 index 0000000000..71d88c1e60 --- /dev/null +++ b/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout @@ -0,0 +1,8 @@ + +*** Dumping AST Record Layout +Type: struct S2 + +Layout: diff --git a/test/CodeGenCXX/override-bit-field-layout.cpp b/test/CodeGenCXX/override-bit-field-layout.cpp index e84fcb0f45..dee7944f6a 100644 --- a/test/CodeGenCXX/override-bit-field-layout.cpp +++ b/test/CodeGenCXX/override-bit-field-layout.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -w -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-bit-field-layout.layout %s | FileCheck %s +// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -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] @@ -14,7 +14,23 @@ struct S2 { short a : 3; }; +// CHECK: Type: struct S3 +// CHECK: Size:32 +// CHECK: FieldOffsets: [0, 1] +struct S3 { + int a : 1; + int b : 2; +}; + +// CHECK: Type: struct S4 +// CHECK: FieldOffsets: [32] +struct S4 : S3 { + char c; +}; + void use_structs() { S1 s1s[sizeof(S1)]; S2 s2s[sizeof(S2)]; + S3 s3s[sizeof(S3)]; + S4 s4s[sizeof(S4)]; } diff --git a/test/CodeGenCXX/override-layout-virtual-base.cpp b/test/CodeGenCXX/override-layout-virtual-base.cpp new file mode 100644 index 0000000000..d9e7346737 --- /dev/null +++ b/test/CodeGenCXX/override-layout-virtual-base.cpp @@ -0,0 +1,21 @@ +// RUN: %clang_cc1 -w -triple=x86_64-pc-win32 -fms-compatibility -fdump-record-layouts-simple -foverride-record-layout=%S/Inputs/override-layout-virtual-base.layout %s | FileCheck %s + +struct S1 { + int a; +}; + +struct S2 : virtual S1 { + virtual void foo() {} +}; + +// CHECK: Type: struct S3 +// CHECK: FieldOffsets: [128] +struct S3 : S2 { + char b; +}; + +void use_structs() { + S1 s1s[sizeof(S1)]; + S2 s2s[sizeof(S2)]; + S3 s3s[sizeof(S3)]; +} diff --git a/test/CodeGenCXX/override-layout.cpp b/test/CodeGenCXX/override-layout.cpp index a3c4bb446c..fea2a45c62 100644 --- a/test/CodeGenCXX/override-layout.cpp +++ b/test/CodeGenCXX/override-layout.cpp @@ -64,6 +64,23 @@ struct PACKED X5 { short r; }; +// CHECK: Type: struct X6 +struct __attribute__((aligned(16))) X6 { + int x; + int y; + virtual ~X6(); +}; + +// CHECK: Type: struct X7 +struct X7 { + int z; +}; + +// CHECK: Type: struct X8 +struct X8 : X6, virtual X7 { + char c; +}; + void use_structs() { X0 x0s[sizeof(X0)]; X1 x1s[sizeof(X1)]; @@ -71,7 +88,9 @@ void use_structs() { X3 x3s[sizeof(X3)]; X4 x4s[sizeof(X4)]; X5 x5s[sizeof(X5)]; + X6 x6s[sizeof(X6)]; + X7 x7s[sizeof(X7)]; + X8 x8s[sizeof(X8)]; x4s[1].a = 1; x5s[1].a = 17; } -