From 731abdfdcc33d813e6c3b4b89eff307aa5c18083 Mon Sep 17 00:00:00 2001 From: Vladislav Dzhidzhoev Date: Fri, 9 Dec 2022 21:36:16 +0300 Subject: [PATCH] [clang][CodeGen] Use base subobject type layout for potentially-overlapping fields RecordLayoutBuilder assumes the size of a potentially-overlapping field with non-zero size as the size of the base subobject type corresponding to the field type. Make CGRecordLayoutBuilder to acknowledge that in order to avoid incorrect padding insertion. Differential Revision: https://reviews.llvm.org/D139741 --- clang/include/clang/AST/Decl.h | 4 ++ clang/lib/AST/Decl.cpp | 4 ++ clang/lib/AST/RecordLayoutBuilder.cpp | 5 +- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 7 ++- clang/test/CodeGenCXX/no-unique-address-3.cpp | 80 +++++++++++++++++++++++++++ 5 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 clang/test/CodeGenCXX/no-unique-address-3.cpp diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index cd33fce..268ec7d 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -3062,6 +3062,10 @@ public: /// [[no_unique_address]] attribute. bool isZeroSize(const ASTContext &Ctx) const; + /// Determine if this field is of potentially-overlapping class type, that + /// is, subobject with the [[no_unique_address]] attribute + bool isPotentiallyOverlapping() const; + /// Get the kind of (C++11) default member initializer that this field has. InClassInitStyle getInClassInitStyle() const { InitStorageKind storageKind = InitStorage.getInt(); diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index b1fdc89..0f15d7b 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -4339,6 +4339,10 @@ bool FieldDecl::isZeroSize(const ASTContext &Ctx) const { return true; } +bool FieldDecl::isPotentiallyOverlapping() const { + return hasAttr() && getType()->getAsCXXRecordDecl(); +} + unsigned FieldDecl::getFieldIndex() const { const FieldDecl *Canonical = getCanonicalDecl(); if (Canonical != this) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 52bd3f2..56cdbb7 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -1853,9 +1853,8 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) { void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, bool InsertExtraPadding) { auto *FieldClass = D->getType()->getAsCXXRecordDecl(); - bool PotentiallyOverlapping = D->hasAttr() && FieldClass; bool IsOverlappingEmptyField = - PotentiallyOverlapping && FieldClass->isEmpty(); + D->isPotentiallyOverlapping() && FieldClass->isEmpty(); CharUnits FieldOffset = (IsUnion || IsOverlappingEmptyField) ? CharUnits::Zero() : getDataSize(); @@ -1916,7 +1915,7 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D, // A potentially-overlapping field occupies its dsize or nvsize, whichever // is larger. - if (PotentiallyOverlapping) { + if (D->isPotentiallyOverlapping()) { const ASTRecordLayout &Layout = Context.getASTRecordLayout(FieldClass); EffectiveFieldSize = std::max(Layout.getNonVirtualSize(), Layout.getDataSize()); diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp index 6f85bca..0f2d764 100644 --- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -379,9 +379,14 @@ void CGRecordLowering::accumulateFields() { for (++Field; Field != FieldEnd && Field->isBitField(); ++Field); accumulateBitFields(Start, Field); } else if (!Field->isZeroSize(Context)) { + // Use base subobject layout for the potentially-overlapping field, + // as it is done in RecordLayoutBuilder Members.push_back(MemberInfo( bitsToCharUnits(getFieldBitOffset(*Field)), MemberInfo::Field, - getStorageType(*Field), *Field)); + Field->isPotentiallyOverlapping() + ? getStorageType(Field->getType()->getAsCXXRecordDecl()) + : getStorageType(*Field), + *Field)); ++Field; } else { ++Field; diff --git a/clang/test/CodeGenCXX/no-unique-address-3.cpp b/clang/test/CodeGenCXX/no-unique-address-3.cpp new file mode 100644 index 0000000..f21e9d1 --- /dev/null +++ b/clang/test/CodeGenCXX/no-unique-address-3.cpp @@ -0,0 +1,80 @@ +// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -emit-llvm -fdump-record-layouts -std=c++17 %s -o %t | FileCheck %s + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | class Empty (empty) +// CHECK-NEXT: | [sizeof=1, dsize=1, align=1, +// CHECK-NEXT: | nvsize=1, nvalign=1] +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | class Second +// CHECK-NEXT: 0 | class Empty (base) (empty) +// CHECK-NEXT: 0:0-0 | short A +// CHECK-NEXT: | [sizeof=2, dsize=1, align=2, +// CHECK-NEXT: | nvsize=1, nvalign=2] +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | class Foo +// CHECK-NEXT: 0 | class Empty (base) (empty) +// CHECK-NEXT: 2 | class Second NZNoUnique +// CHECK-NEXT: 2 | class Empty (base) (empty) +// CHECK-NEXT: 2:0-0 | short A +// CHECK-NEXT: 3 | char B +// CHECK-NEXT: | [sizeof=4, dsize=4, align=2, +// CHECK-NEXT: | nvsize=4, nvalign=2] + +class Empty {}; + +// CHECK-LABEL: LLVMType:%class.Second = type { i8, i8 } +// CHECK-NEXT: NonVirtualBaseLLVMType:%class.Second.base = type { i8 } +class Second : Empty { + short A : 1; +}; + +// CHECK-LABEL: LLVMType:%class.Foo = type { [2 x i8], %class.Second.base, i8 } +// CHECK-NEXT: NonVirtualBaseLLVMType:%class.Foo = type { [2 x i8], %class.Second.base, i8 } +class Foo : Empty { + [[no_unique_address]] Second NZNoUnique; + char B; +}; +Foo I; + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | class SecondEmpty (empty) +// CHECK-NEXT: 0 | class Empty (base) (empty) +// CHECK-NEXT: | [sizeof=1, dsize=0, align=1, +// CHECK-NEXT: | nvsize=1, nvalign=1] +class SecondEmpty: Empty { +}; + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | class Bar +// CHECK-NEXT: 0 | class Empty (base) (empty) +// CHECK-NEXT: 1 | class SecondEmpty ZNoUnique (empty) +// CHECK-NEXT: 1 | class Empty (base) (empty) +// CHECK-NEXT: 0 | char C +// CHECK-NEXT: | [sizeof=2, dsize=1, align=1, +// CHECK-NEXT: | nvsize=2, nvalign=1] + +// CHECK-LABEL: LLVMType:%class.Bar = type { i8, i8 } +// CHECK-NEXT: NonVirtualBaseLLVMType:%class.Bar = type { i8, i8 } +class Bar : Empty { + [[no_unique_address]] SecondEmpty ZNoUnique; + char C; +}; +Bar J; + +// CHECK: *** Dumping AST Record Layout +// CHECK-NEXT: 0 | class IntFieldClass +// CHECK-NEXT: 0 | class Empty (base) (empty) +// CHECK-NEXT: 2 | class Second Field +// CHECK-NEXT: 2 | class Empty (base) (empty) +// CHECK-NEXT: 2:0-0 | short A +// CHECK-NEXT: 4 | int C +// CHECK-NEXT: | [sizeof=8, dsize=8, align=4, +// CHECK-NEXT: | nvsize=8, nvalign=4] + +// CHECK-LABEL: LLVMType:%class.IntFieldClass = type { [2 x i8], %class.Second.base, i32 } +// CHECK-NEXT: NonVirtualBaseLLVMType:%class.IntFieldClass = type { [2 x i8], %class.Second.base, i32 } +class IntFieldClass : Empty { + [[no_unique_address]] Second Field; + int C; +}; +IntFieldClass K; -- 2.7.4