[clang] af6c0b6 - [clang][CodeGen] Use base subobject type layout for potentially-overlapping fields
Vladislav Dzhidzhoev via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 17 06:11:57 PST 2023
Author: Vladislav Dzhidzhoev
Date: 2023-02-17T15:11:42+01:00
New Revision: af6c0b6d8c9dc52485d56e82f79a03de09aa188f
URL: https://github.com/llvm/llvm-project/commit/af6c0b6d8c9dc52485d56e82f79a03de09aa188f
DIFF: https://github.com/llvm/llvm-project/commit/af6c0b6d8c9dc52485d56e82f79a03de09aa188f.diff
LOG: [clang][CodeGen] Use base subobject type layout for potentially-overlapping fields
RecordLayoutBuilder assumes the size of a potentially-overlapping
class/struct 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
Added:
clang/test/CodeGenCXX/no-unique-address-3.cpp
Modified:
clang/include/clang/AST/Decl.h
clang/lib/AST/Decl.cpp
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index b32d021b1c695..e213c5fe29c68 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3072,6 +3072,10 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
/// [[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 cd09e2fe2b05b..9c72d7a7d928c 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4347,6 +4347,10 @@ bool FieldDecl::isZeroSize(const ASTContext &Ctx) const {
return true;
}
+bool FieldDecl::isPotentiallyOverlapping() const {
+ return hasAttr<NoUniqueAddressAttr>() && 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 2f546398338c4..641e9d62149e9 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<NoUniqueAddressAttr>() && 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 596f0bd332049..888b7ddcccd32 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -182,7 +182,7 @@ struct CGRecordLowering {
llvm::Type *StorageType);
/// Lowers an ASTRecordLayout to a llvm type.
void lower(bool NonVirtualBaseType);
- void lowerUnion();
+ void lowerUnion(bool isNoUniqueAddress);
void accumulateFields();
void accumulateBitFields(RecordDecl::field_iterator Field,
RecordDecl::field_iterator FieldEnd);
@@ -280,7 +280,7 @@ void CGRecordLowering::lower(bool NVBaseType) {
// CodeGenTypes::ComputeRecordLayout.
CharUnits Size = NVBaseType ? Layout.getNonVirtualSize() : Layout.getSize();
if (D->isUnion()) {
- lowerUnion();
+ lowerUnion(NVBaseType);
computeVolatileBitfields();
return;
}
@@ -308,8 +308,9 @@ void CGRecordLowering::lower(bool NVBaseType) {
computeVolatileBitfields();
}
-void CGRecordLowering::lowerUnion() {
- CharUnits LayoutSize = Layout.getSize();
+void CGRecordLowering::lowerUnion(bool isNoUniqueAddress) {
+ CharUnits LayoutSize =
+ isNoUniqueAddress ? Layout.getDataSize() : Layout.getSize();
llvm::Type *StorageType = nullptr;
bool SeenNamedMember = false;
// Iterate through the fields setting bitFieldInfo and the Fields array. Also
@@ -365,7 +366,12 @@ void CGRecordLowering::lowerUnion() {
FieldTypes.push_back(StorageType);
appendPaddingBytes(LayoutSize - getSize(StorageType));
// Set packed if we need it.
- if (LayoutSize % getAlignment(StorageType))
+ const auto StorageAlignment = getAlignment(StorageType);
+ assert((Layout.getSize() % StorageAlignment == 0 ||
+ Layout.getDataSize() % StorageAlignment) &&
+ "Union's standard layout and no_unique_address layout must agree on "
+ "packedness");
+ if (Layout.getDataSize() % StorageAlignment)
Packed = true;
}
@@ -379,9 +385,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;
@@ -882,7 +893,7 @@ CodeGenTypes::ComputeRecordLayout(const RecordDecl *D, llvm::StructType *Ty) {
// If we're in C++, compute the base subobject type.
llvm::StructType *BaseTy = nullptr;
- if (isa<CXXRecordDecl>(D) && !D->isUnion() && !D->hasAttr<FinalAttr>()) {
+ if (isa<CXXRecordDecl>(D)) {
BaseTy = Ty;
if (Builder.Layout.getNonVirtualSize() != Builder.Layout.getSize()) {
CGRecordLowering BaseBuilder(*this, D, /*Packed=*/Builder.Packed);
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 0000000000000..c1e84f1d31276
--- /dev/null
+++ b/clang/test/CodeGenCXX/no-unique-address-3.cpp
@@ -0,0 +1,209 @@
+// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -emit-llvm -fdump-record-layouts -std=c++17 %s -o %t | FileCheck %s
+
+// CHECK-LABEL: 0 | class Empty (empty)
+// CHECK-NEXT: | [sizeof=1, dsize=1, align=1,
+// CHECK-NEXT: | nvsize=1, nvalign=1]
+// CHECK-LABEL: 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-LABEL: 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-LABEL: 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-LABEL: 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-LABEL: 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;
+
+// CHECK-LABEL: 0 | class UnionClass
+// CHECK-NEXT: 0 | class Empty (base) (empty)
+// CHECK-NEXT: 0 | union UnionClass
+// CHECK-NEXT: 0 | int I
+// CHECK-NEXT: 0 | char C
+// CHECK-NEXT: 4 | int C
+// CHECK-NEXT: | [sizeof=8, dsize=8, align=4,
+// CHECK-NEXT: | nvsize=8, nvalign=4]
+
+// CHECK-LABEL: LLVMType:%class.UnionClass = type { %union.anon, i32 }
+// CHECK-NEXT: NonVirtualBaseLLVMType:%class.UnionClass = type { %union.anon, i32 }
+// CHECK-NEXT: IsZeroInitializable:1
+// CHECK-NEXT: BitFields:[
+// CHECK-NEXT: ]>
+class UnionClass : Empty {
+ [[no_unique_address]] union {
+ int I;
+ char C;
+ } U;
+ int C;
+};
+UnionClass L;
+
+// CHECK-LABEL: 0 | class EnumClass
+// CHECK-NEXT: 0 | class Empty (base) (empty)
+// CHECK-NEXT: 0 | enum E A
+// CHECK-NEXT: 4 | int C
+// CHECK-NEXT: | [sizeof=8, dsize=8, align=4,
+// CHECK-NEXT: | nvsize=8, nvalign=4]
+
+// CHECK-LABEL: LLVMType:%class.EnumClass = type { i32, i32 }
+// CHECK-NEXT: NonVirtualBaseLLVMType:%class.EnumClass = type { i32, i32 }
+// CHECK-NEXT: IsZeroInitializable:1
+// CHECK-NEXT: BitFields:[
+// CHECK-NEXT: ]>
+class EnumClass : Empty {
+ [[no_unique_address]] enum class E { X, Y, Z } A;
+ int C;
+};
+EnumClass M;
+
+// CHECK-LABEL: 0 | class NoBaseField
+// CHECK-NEXT: 0 | class Empty (base) (empty)
+// CHECK-NEXT: 1 | class Empty A (empty)
+// CHECK-NEXT: 0 | int B
+// CHECK-NEXT: | [sizeof=4, dsize=4, align=4,
+// CHECK-NEXT: | nvsize=4, nvalign=4]
+
+// CHECK-LABEL: LLVMType:%class.NoBaseField = type { i32 }
+// CHECK-NEXT: NonVirtualBaseLLVMType:%class.NoBaseField = type { i32 }
+// CHECK-NEXT: IsZeroInitializable:1
+// CHECK-NEXT: BitFields:[
+// CHECK-NEXT: ]>
+class NoBaseField : Empty {
+ [[no_unique_address]] Empty A;
+ int B;
+};
+NoBaseField N;
+
+// CHECK-LABEL: 0 | class FinalEmpty (empty)
+// CHECK-NEXT: | [sizeof=1, dsize=1, align=1,
+// CHECK-NEXT: | nvsize=1, nvalign=1]
+
+// CHECK-LABEL: 0 | class FinalClass
+// CHECK-NEXT: 0 | class Empty (base) (empty)
+// CHECK-NEXT: 0 | class FinalEmpty A (empty)
+// CHECK-NEXT: 0 | int B
+// CHECK-NEXT: | [sizeof=4, dsize=4, align=4,
+// CHECK-NEXT: | nvsize=4, nvalign=4]
+class FinalEmpty final {};
+class FinalClass final : Empty {
+ [[no_unique_address]] FinalEmpty A;
+ int B;
+} O;
+
+
+// CHECK-LABEL: 0 | union Union2Class::PaddedUnion
+// CHECK-NEXT: 0 | class Empty A (empty)
+// CHECK-NEXT: 0 | char B
+// CHECK-NEXT: | [sizeof=2, dsize=1, align=2,
+// CHECK-NEXT: | nvsize=1, nvalign=2]
+
+// CHECK-LABEL: 0 | class Union2Class
+// CHECK-NEXT: 0 | class Empty (base) (empty)
+// CHECK-NEXT: 2 | union Union2Class::PaddedUnion U
+// CHECK-NEXT: 2 | class Empty A (empty)
+// CHECK-NEXT: 2 | char B
+// CHECK-NEXT: 3 | char C
+// CHECK-NEXT: | [sizeof=4, dsize=4, align=2,
+// CHECK-NEXT: | nvsize=4, nvalign=2]
+class Union2Class : Empty {
+ [[no_unique_address]] union PaddedUnion {
+ private:
+ Empty A;
+ alignas(2) char B;
+ } U;
+ char C;
+} P;
+
+// CHECK-LABEL: 0 | struct NotEmptyWithBitfield
+// CHECK-NEXT: 0 | class Empty (base) (empty)
+// CHECK-NEXT: 0 | char[2] A
+// CHECK-NEXT: 2:0-0 | int B
+// CHECK-NEXT: | [sizeof=4, dsize=3, align=4,
+// CHECK-NEXT: | nvsize=3, nvalign=4]
+
+// CHECK-LABEL: 0 | union C::
+// CHECK-NEXT: 0 | short C
+// CHECK-NEXT: 0 | struct NotEmptyWithBitfield A
+// CHECK-NEXT: 0 | class Empty (base) (empty)
+// CHECK-NEXT: 0 | char[2] A
+// CHECK-NEXT: 2:0-0 | int B
+// CHECK-NEXT: | [sizeof=4, dsize=3, align=4,
+// CHECK-NEXT: | nvsize=3, nvalign=4]
+
+// CHECK-LABEL: 0 | struct C
+// CHECK-NEXT: 0 | union C::
+// CHECK-NEXT: 0 | short C
+// CHECK-NEXT: 0 | struct NotEmptyWithBitfield A
+// CHECK-NEXT: 0 | class Empty (base) (empty)
+// CHECK-NEXT: 0 | char[2] A
+// CHECK-NEXT: 2:0-0 | int B
+// CHECK-NEXT: | [sizeof=4, dsize=3, align=4,
+// CHECK-NEXT: | nvsize=3, nvalign=4]
+struct NotEmptyWithBitfield : Empty {
+ char A[2];
+ int B : 1;
+};
+struct C {
+ [[no_unique_address]] union {
+ short C;
+ [[no_unique_address]] NotEmptyWithBitfield A;
+ } U;
+} Q;
More information about the cfe-commits
mailing list