From 867c2a7d369c6a067cadead7cd94ad5b391f14b4 Mon Sep 17 00:00:00 2001 From: Aleksandr Urakov Date: Wed, 13 Mar 2019 13:38:12 +0000 Subject: [PATCH] [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 llvm-svn: 356047 --- clang/lib/AST/RecordLayoutBuilder.cpp | 22 +++++++++++++++++----- .../Inputs/override-bit-field-layout.layout | 8 ++++++++ .../Inputs/override-layout-virtual-base.layout | 8 ++++++++ .../test/CodeGenCXX/override-bit-field-layout.cpp | 18 +++++++++++++++++- .../CodeGenCXX/override-layout-virtual-base.cpp | 21 +++++++++++++++++++++ clang/test/CodeGenCXX/override-layout.cpp | 21 ++++++++++++++++++++- 6 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout create mode 100644 clang/test/CodeGenCXX/override-layout-virtual-base.cpp diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 99b7cbd..2d112c4 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/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/clang/test/CodeGenCXX/Inputs/override-bit-field-layout.layout b/clang/test/CodeGenCXX/Inputs/override-bit-field-layout.layout index 8e67dce..b57c6ef 100644 --- a/clang/test/CodeGenCXX/Inputs/override-bit-field-layout.layout +++ b/clang/test/CodeGenCXX/Inputs/override-bit-field-layout.layout @@ -14,3 +14,11 @@ Layout: + +*** Dumping AST Record Layout +Type: struct S3 + +Layout: diff --git a/clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout b/clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout new file mode 100644 index 0000000..71d88c1 --- /dev/null +++ b/clang/test/CodeGenCXX/Inputs/override-layout-virtual-base.layout @@ -0,0 +1,8 @@ + +*** Dumping AST Record Layout +Type: struct S2 + +Layout: diff --git a/clang/test/CodeGenCXX/override-bit-field-layout.cpp b/clang/test/CodeGenCXX/override-bit-field-layout.cpp index e84fcb0..dee7944 100644 --- a/clang/test/CodeGenCXX/override-bit-field-layout.cpp +++ b/clang/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/clang/test/CodeGenCXX/override-layout-virtual-base.cpp b/clang/test/CodeGenCXX/override-layout-virtual-base.cpp new file mode 100644 index 0000000..d9e7346 --- /dev/null +++ b/clang/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/clang/test/CodeGenCXX/override-layout.cpp b/clang/test/CodeGenCXX/override-layout.cpp index a3c4bb4..fea2a45 100644 --- a/clang/test/CodeGenCXX/override-layout.cpp +++ b/clang/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; } - -- 2.7.4