dzhidzhoev created this revision.
dzhidzhoev added reviewers: kjdyck, rjmccall, efriedma, majnemer,
aaron.ballman, rsmith.
Herald added a project: All.
dzhidzhoev requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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 RecordLayoutBuilder to acknowledge that in order to avoid incorrect
padding insertion.
Without this patch, test fails on assertion
Assertion failed: (Offset >= Size), function insertPadding, file
CGRecordLayoutBuilder.cpp, line 802.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D139741
Files:
clang/include/clang/AST/Decl.h
clang/lib/AST/Decl.cpp
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
clang/test/CodeGenCXX/no-unique-address-3.cpp
Index: clang/test/CodeGenCXX/no-unique-address-3.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/no-unique-address-3.cpp
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -triple aarch64-unknown-linux-gnu -emit-obj -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]
+
+// 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]
+
+// 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: *** 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]
+
+class Empty {};
+
+class Second : Empty {
+ short A : 1;
+};
+
+class Foo : Empty {
+ [[no_unique_address]] Second NZNoUnique;
+ char B;
+};
+Foo I;
+
+class SecondEmpty: Empty {
+};
+class Bar : Empty {
+ [[no_unique_address]] SecondEmpty ZNoUnique;
+ char C;
+};
+Bar J;
+
+class IntFieldClass : Empty {
+ [[no_unique_address]] Second Field;
+ int C;
+};
+IntFieldClass K;
Index: clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
===================================================================
--- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -379,9 +379,14 @@
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;
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1853,9 +1853,8 @@
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 @@
// 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());
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -4339,6 +4339,10 @@
return true;
}
+bool FieldDecl::isPotentiallyOverlapping() const {
+ return hasAttr<NoUniqueAddressAttr>() && getType()->getAsCXXRecordDecl();
+}
+
unsigned FieldDecl::getFieldIndex() const {
const FieldDecl *Canonical = getCanonicalDecl();
if (Canonical != this)
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -3059,6 +3059,10 @@
/// [[no_unique_address]] attribute.
bool isZeroSize(const ASTContext &Ctx) const;
+ /// Determine if this field is a potentially-overlapping, 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();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
