[Lldb-commits] [clang] [lldb] [clang][RecordLayoutBuilder] Be stricter about inferring packed-ness in ExternalLayouts (PR #97443)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Wed Oct 2 06:01:08 PDT 2024
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/97443
>From 3a718c75d0458b7aece72f2ba8e5aa5a68815237 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Tue, 2 Jul 2024 18:43:34 +0200
Subject: [PATCH 1/6] [clang][RecordLayoutBuilder] Be stricter about inferring
packed-ness in ExternalLayouts
This patch is motivated by the LLDB support required for:
https://github.com/llvm/llvm-project/issues/93069
In the presence of `[[no_unique_address]]`, LLDB may ask Clang
to lay out types with overlapping field offsets. Because we don't
have attributes such as `packed` or `no_unique_address` in the LLDB
AST, the `RecordLayoutBuilder` supports an `InferAlignment` mode, which,
in the past, attempted to detect layouts which came from packed
structures in order to provide a conservative estimate of alignment for
it (since `DW_AT_alignment` isn't emitted unless explicitly changed with
`alignas`, etc.).
However, in the presence of overlapping fields due to `no_unique_address`,
`InferAlignment` would set the alignment of structures to `1` for which
that's incorrect. This poses issues in some LLDB formatters that
synthesize new Clang types and rely on the layout builder to get the
`FieldOffset` of structures right that we did have DWARF offsets for.
The result of this is that if we get the alignment wrong, LLDB reads
out garbage data from the wrong field offsets.
There are a couple of solutions to this that we considered:
1. Make LLDB formatters not do the above, and make them more robust
to inaccurate alignment.
2. Remove `InferAlignment` entirely and rely on Clang emitting
`DW_AT_alignment` for packed structures.
3. Remove `InferAlignment` and detect packedness from within LLDB.
4. Make the `InferAlignment` logic account for overlapping fields.
Option (1) turned out quite hairy and it's not clear we can achieve
this with the tools available for certain STL formatters (particularly
`std::map`). But I would still very much like to simplify this if we
can.
Option (2) wouldn't help with GCC-compiled binaries, and if we can
get away with LLDB not needing the alignment, then we wouldn't need
to increase debug-info size.
Option (3), AFAICT, would require us to reimplement some of the layout
logic in the layout builder. Would be great if we can avoid this added
complexity.
Option (4) seemed like the best option in the interim. As part of this
change I also removed one of the `InferAlignment` blocks. The test-cases
associated with this code-path pass regardless, and from the description
of the change that introduced it it's not clear why specifically the
base offsets would influence the `Alignment` field, and how it would
imply packedness. But happy to be proven wrong. Ultimately it would be
great if we can get rid of the `InferAlignment` infrastructure and
support our use-cases in LLDB or DWARF instead.
---
clang/lib/AST/RecordLayoutBuilder.cpp | 34 +++++++++----------
.../DWARF/no_unique_address-alignment.cpp | 2 --
.../no_unique_address-base-alignment.cpp | 2 --
3 files changed, 17 insertions(+), 21 deletions(-)
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index d9bf62c2bbb04a..8dbf69c310cbb4 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -802,7 +802,8 @@ class ItaniumRecordLayoutBuilder {
/// \param Field The field whose offset is being queried.
/// \param ComputedOffset The offset that we've computed for this field.
uint64_t updateExternalFieldOffset(const FieldDecl *Field,
- uint64_t ComputedOffset);
+ uint64_t ComputedOffset,
+ uint64_t PreviousOffset);
void CheckFieldPadding(uint64_t Offset, uint64_t UnpaddedOffset,
uint64_t UnpackedOffset, unsigned UnpackedAlign,
@@ -1296,13 +1297,6 @@ ItaniumRecordLayoutBuilder::LayoutBase(const BaseSubobjectInfo *Base) {
bool Allowed = EmptySubobjects->CanPlaceBaseAtOffset(Base, Offset);
(void)Allowed;
assert(Allowed && "Base subobject externally placed at overlapping offset");
-
- if (InferAlignment && Offset < getDataSize().alignTo(AlignTo)) {
- // The externally-supplied base offset is before the base offset we
- // computed. Assume that the structure is packed.
- Alignment = CharUnits::One();
- InferAlignment = false;
- }
}
if (!Base->Class->isEmpty()) {
@@ -1770,7 +1764,8 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
// If we're using external layout, give the external layout a chance
// to override this information.
if (UseExternalLayout)
- FieldOffset = updateExternalFieldOffset(D, FieldOffset);
+ FieldOffset = updateExternalFieldOffset(
+ D, FieldOffset, FieldOffsets.empty() ? 0 : FieldOffsets.back());
// Okay, place the bitfield at the calculated offset.
FieldOffsets.push_back(FieldOffset);
@@ -2063,8 +2058,9 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
UnpackedFieldOffset = UnpackedFieldOffset.alignTo(UnpackedFieldAlign);
if (UseExternalLayout) {
- FieldOffset = Context.toCharUnitsFromBits(
- updateExternalFieldOffset(D, Context.toBits(FieldOffset)));
+ FieldOffset = Context.toCharUnitsFromBits(updateExternalFieldOffset(
+ D, Context.toBits(FieldOffset),
+ FieldOffsets.empty() ? 0 : FieldOffsets.back()));
if (!IsUnion && EmptySubobjects) {
// Record the fact that we're placing a field at this offset.
@@ -2250,14 +2246,18 @@ void ItaniumRecordLayoutBuilder::UpdateAlignment(
}
}
-uint64_t
-ItaniumRecordLayoutBuilder::updateExternalFieldOffset(const FieldDecl *Field,
- uint64_t ComputedOffset) {
+uint64_t ItaniumRecordLayoutBuilder::updateExternalFieldOffset(
+ const FieldDecl *Field, uint64_t ComputedOffset, uint64_t PreviousOffset) {
uint64_t ExternalFieldOffset = External.getExternalFieldOffset(Field);
- if (InferAlignment && ExternalFieldOffset < ComputedOffset) {
- // The externally-supplied field offset is before the field offset we
- // computed. Assume that the structure is packed.
+ // If the externally-supplied field offset is before the field offset we
+ // computed. Check against the previous field offset to make sure we don't
+ // misinterpret overlapping fields as packedness of the structure.
+ const bool assume_packed = ExternalFieldOffset > 0 &&
+ ExternalFieldOffset < ComputedOffset &&
+ ExternalFieldOffset > PreviousOffset;
+
+ if (InferAlignment && assume_packed) {
Alignment = CharUnits::One();
PreferredAlignment = CharUnits::One();
InferAlignment = false;
diff --git a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
index 1488199a3ad2d3..ecefa664747727 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-alignment.cpp
@@ -1,5 +1,3 @@
-// XFAIL: *
-
// RUN: %clangxx_host -gdwarf -o %t %s
// RUN: %lldb %t \
// RUN: -o "expr alignof(OverlappingFields)" \
diff --git a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
index 15d8de0e3ee988..9f574f9846e358 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-base-alignment.cpp
@@ -1,5 +1,3 @@
-// XFAIL: *
-
// RUN: %clangxx_host -gdwarf -o %t %s
// RUN: %lldb %t \
// RUN: -o "expr alignof(OverlappingDerived)" \
>From 4f82043a46cf9f06041ad5aa1c909624c5d90128 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 2 Oct 2024 13:42:18 +0100
Subject: [PATCH 2/6] fixup! move isEmptyFieldForLayout; ignore empty fields
instead of checking overlap
---
clang/include/clang/AST/RecordLayout.h | 10 +++++
clang/lib/AST/RecordLayoutBuilder.cpp | 56 ++++++++++++++++++++++++--
clang/lib/CodeGen/ABIInfoImpl.cpp | 35 ----------------
clang/lib/CodeGen/ABIInfoImpl.h | 10 -----
4 files changed, 62 insertions(+), 49 deletions(-)
diff --git a/clang/include/clang/AST/RecordLayout.h b/clang/include/clang/AST/RecordLayout.h
index dd18f9c49f84fb..a5c1ccfeb34ad4 100644
--- a/clang/include/clang/AST/RecordLayout.h
+++ b/clang/include/clang/AST/RecordLayout.h
@@ -337,6 +337,16 @@ class ASTRecordLayout {
}
};
+/// isEmptyFieldForLayout - Return true iff the field is "empty", that is,
+/// either a zero-width bit-field or an \ref isEmptyRecordForLayout.
+bool isEmptyFieldForLayout(const ASTContext &Context, const FieldDecl *FD);
+
+/// isEmptyRecordForLayout - Return true iff a structure contains only empty
+/// base classes (per \ref isEmptyRecordForLayout) and fields (per
+/// \ref isEmptyFieldForLayout). Note, C++ record fields are considered empty
+/// if the [[no_unique_address]] attribute would have made them empty.
+bool isEmptyRecordForLayout(const ASTContext &Context, QualType T);
+
} // namespace clang
#endif // LLVM_CLANG_AST_RECORDLAYOUT_H
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 8dbf69c310cbb4..31cc14562543fe 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2250,12 +2250,24 @@ uint64_t ItaniumRecordLayoutBuilder::updateExternalFieldOffset(
const FieldDecl *Field, uint64_t ComputedOffset, uint64_t PreviousOffset) {
uint64_t ExternalFieldOffset = External.getExternalFieldOffset(Field);
- // If the externally-supplied field offset is before the field offset we
- // computed. Check against the previous field offset to make sure we don't
- // misinterpret overlapping fields as packedness of the structure.
+ // DWARF doesn't tell us whether a structure was declared as packed.
+ // So we try to figure out if the supplied Field is at a packed offset
+ // (i.e., the externally-supplied offset is less than the layout builder
+ // expected).
+ //
+ // There are cases where fields are placed at overlapping offsets (e.g.,
+ // as a result of [[no_unique_address]]). In those cases we don't want
+ // to incorrectly deduce that they are placed at packed offsets. Hence,
+ // ignore empty fields (which are the only fields that can overlap).
+ //
+ // Note, we use \ref isEmptyFieldForLayout because the externally provided
+ // AST may be lacking enough fidelity to derive isEmpty for the Field's type.
+ //
+ // FIXME: emit enough information in DWARF to get rid of InferAlignment.
+ //
const bool assume_packed = ExternalFieldOffset > 0 &&
ExternalFieldOffset < ComputedOffset &&
- ExternalFieldOffset > PreviousOffset;
+ !clang::isEmptyFieldForLayout(Context, Field);
if (InferAlignment && assume_packed) {
Alignment = CharUnits::One();
@@ -3807,3 +3819,39 @@ void ASTContext::DumpRecordLayout(const RecordDecl *RD, raw_ostream &OS,
}
OS << "]>\n";
}
+
+bool clang::isEmptyFieldForLayout(const ASTContext &Context,
+ const FieldDecl *FD) {
+ if (FD->isZeroLengthBitField(Context))
+ return true;
+
+ if (FD->isUnnamedBitField())
+ return false;
+
+ return isEmptyRecordForLayout(Context, FD->getType());
+}
+
+bool clang::isEmptyRecordForLayout(const ASTContext &Context, QualType T) {
+ const RecordType *RT = T->getAs<RecordType>();
+ if (!RT)
+ return false;
+
+ const RecordDecl *RD = RT->getDecl();
+
+ // If this is a C++ record, check the bases first.
+ if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+ if (CXXRD->isDynamicClass())
+ return false;
+
+ for (const auto &I : CXXRD->bases())
+ if (!isEmptyRecordForLayout(Context, I.getType()))
+ return false;
+ }
+
+ for (const auto *I : RD->fields())
+ if (!isEmptyFieldForLayout(Context, I))
+ return false;
+
+ return true;
+}
+
diff --git a/clang/lib/CodeGen/ABIInfoImpl.cpp b/clang/lib/CodeGen/ABIInfoImpl.cpp
index be91b85e3a816f..b1d1b6ee5f526d 100644
--- a/clang/lib/CodeGen/ABIInfoImpl.cpp
+++ b/clang/lib/CodeGen/ABIInfoImpl.cpp
@@ -311,41 +311,6 @@ bool CodeGen::isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays,
return true;
}
-bool CodeGen::isEmptyFieldForLayout(const ASTContext &Context,
- const FieldDecl *FD) {
- if (FD->isZeroLengthBitField(Context))
- return true;
-
- if (FD->isUnnamedBitField())
- return false;
-
- return isEmptyRecordForLayout(Context, FD->getType());
-}
-
-bool CodeGen::isEmptyRecordForLayout(const ASTContext &Context, QualType T) {
- const RecordType *RT = T->getAs<RecordType>();
- if (!RT)
- return false;
-
- const RecordDecl *RD = RT->getDecl();
-
- // If this is a C++ record, check the bases first.
- if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
- if (CXXRD->isDynamicClass())
- return false;
-
- for (const auto &I : CXXRD->bases())
- if (!isEmptyRecordForLayout(Context, I.getType()))
- return false;
- }
-
- for (const auto *I : RD->fields())
- if (!isEmptyFieldForLayout(Context, I))
- return false;
-
- return true;
-}
-
const Type *CodeGen::isSingleElementStruct(QualType T, ASTContext &Context) {
const RecordType *RT = T->getAs<RecordType>();
if (!RT)
diff --git a/clang/lib/CodeGen/ABIInfoImpl.h b/clang/lib/CodeGen/ABIInfoImpl.h
index 2a3ef6b8a6c961..92986fb4316465 100644
--- a/clang/lib/CodeGen/ABIInfoImpl.h
+++ b/clang/lib/CodeGen/ABIInfoImpl.h
@@ -137,16 +137,6 @@ bool isEmptyField(ASTContext &Context, const FieldDecl *FD, bool AllowArrays,
bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays,
bool AsIfNoUniqueAddr = false);
-/// isEmptyFieldForLayout - Return true iff the field is "empty", that is,
-/// either a zero-width bit-field or an \ref isEmptyRecordForLayout.
-bool isEmptyFieldForLayout(const ASTContext &Context, const FieldDecl *FD);
-
-/// isEmptyRecordForLayout - Return true iff a structure contains only empty
-/// base classes (per \ref isEmptyRecordForLayout) and fields (per
-/// \ref isEmptyFieldForLayout). Note, C++ record fields are considered empty
-/// if the [[no_unique_address]] attribute would have made them empty.
-bool isEmptyRecordForLayout(const ASTContext &Context, QualType T);
-
/// isSingleElementStruct - Determine if a structure is a "single
/// element struct", i.e. it has exactly one non-empty field or
/// exactly one field which is itself a single element
>From 04100396b5ba18bab76fc66acc661da47cb8659d Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 2 Oct 2024 13:42:30 +0100
Subject: [PATCH 3/6] fixup! clang-format
---
clang/lib/AST/RecordLayoutBuilder.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 31cc14562543fe..983cadf9dacadc 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -3821,7 +3821,7 @@ void ASTContext::DumpRecordLayout(const RecordDecl *RD, raw_ostream &OS,
}
bool clang::isEmptyFieldForLayout(const ASTContext &Context,
- const FieldDecl *FD) {
+ const FieldDecl *FD) {
if (FD->isZeroLengthBitField(Context))
return true;
@@ -3854,4 +3854,3 @@ bool clang::isEmptyRecordForLayout(const ASTContext &Context, QualType T) {
return true;
}
-
>From 43230b5803d28d8f1a4b906e43d14cf4ddc2c363 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 2 Oct 2024 13:43:32 +0100
Subject: [PATCH 4/6] fixup! add test with packed attribute and NUA
---
.../no_unique_address-packed-alignment.cpp | 23 +++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 lldb/test/Shell/SymbolFile/DWARF/no_unique_address-packed-alignment.cpp
diff --git a/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-packed-alignment.cpp b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-packed-alignment.cpp
new file mode 100644
index 00000000000000..80fb699ddafa3f
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/no_unique_address-packed-alignment.cpp
@@ -0,0 +1,23 @@
+// RUN: %clangxx_host -gdwarf -o %t %s
+// RUN: %lldb %t \
+// RUN: -o "expr alignof(PackedNUA)" \
+// RUN: -o "expr sizeof(PackedNUA)" \
+// RUN: -o exit | FileCheck %s
+
+struct Empty {};
+struct __attribute((packed)) PackedNUA {
+ [[no_unique_address]] Empty a,b,c,d;
+ char x;
+ int y;
+};
+static_assert(alignof(PackedNUA) == 1);
+static_assert(sizeof(PackedNUA) == 5);
+
+PackedNUA packed;
+
+// CHECK: (lldb) expr alignof(PackedNUA)
+// CHECK-NEXT: ${{.*}} = 1
+// CHECK: (lldb) expr sizeof(PackedNUA)
+// CHECK-NEXT: ${{.*}} = 5
+
+int main() { PackedNUA d; }
>From fc121efb72c60ed2ca5602d4e42a003ae26a5384 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 2 Oct 2024 13:59:18 +0100
Subject: [PATCH 5/6] fixup! use isEmpty API instead of isEmptyXXForLayout
---
clang/include/clang/AST/RecordLayout.h | 11 -------
clang/lib/AST/RecordLayoutBuilder.cpp | 44 +++-----------------------
clang/lib/CodeGen/ABIInfoImpl.cpp | 35 ++++++++++++++++++++
clang/lib/CodeGen/ABIInfoImpl.h | 11 +++++++
4 files changed, 51 insertions(+), 50 deletions(-)
diff --git a/clang/include/clang/AST/RecordLayout.h b/clang/include/clang/AST/RecordLayout.h
index a5c1ccfeb34ad4..13af256982b335 100644
--- a/clang/include/clang/AST/RecordLayout.h
+++ b/clang/include/clang/AST/RecordLayout.h
@@ -336,17 +336,6 @@ class ASTRecordLayout {
return CXXInfo->VBaseOffsets;
}
};
-
-/// isEmptyFieldForLayout - Return true iff the field is "empty", that is,
-/// either a zero-width bit-field or an \ref isEmptyRecordForLayout.
-bool isEmptyFieldForLayout(const ASTContext &Context, const FieldDecl *FD);
-
-/// isEmptyRecordForLayout - Return true iff a structure contains only empty
-/// base classes (per \ref isEmptyRecordForLayout) and fields (per
-/// \ref isEmptyFieldForLayout). Note, C++ record fields are considered empty
-/// if the [[no_unique_address]] attribute would have made them empty.
-bool isEmptyRecordForLayout(const ASTContext &Context, QualType T);
-
} // namespace clang
#endif // LLVM_CLANG_AST_RECORDLAYOUT_H
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 983cadf9dacadc..c2117abcbf1e3d 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2260,14 +2260,15 @@ uint64_t ItaniumRecordLayoutBuilder::updateExternalFieldOffset(
// to incorrectly deduce that they are placed at packed offsets. Hence,
// ignore empty fields (which are the only fields that can overlap).
//
- // Note, we use \ref isEmptyFieldForLayout because the externally provided
- // AST may be lacking enough fidelity to derive isEmpty for the Field's type.
- //
// FIXME: emit enough information in DWARF to get rid of InferAlignment.
//
+ CXXRecordDecl * CXX = nullptr;
+ if (auto * RT = dyn_cast<RecordType>(Field->getType()))
+ CXX = RT->getAsCXXRecordDecl();
+
const bool assume_packed = ExternalFieldOffset > 0 &&
ExternalFieldOffset < ComputedOffset &&
- !clang::isEmptyFieldForLayout(Context, Field);
+ !(CXX && CXX->isEmpty());
if (InferAlignment && assume_packed) {
Alignment = CharUnits::One();
@@ -3819,38 +3820,3 @@ void ASTContext::DumpRecordLayout(const RecordDecl *RD, raw_ostream &OS,
}
OS << "]>\n";
}
-
-bool clang::isEmptyFieldForLayout(const ASTContext &Context,
- const FieldDecl *FD) {
- if (FD->isZeroLengthBitField(Context))
- return true;
-
- if (FD->isUnnamedBitField())
- return false;
-
- return isEmptyRecordForLayout(Context, FD->getType());
-}
-
-bool clang::isEmptyRecordForLayout(const ASTContext &Context, QualType T) {
- const RecordType *RT = T->getAs<RecordType>();
- if (!RT)
- return false;
-
- const RecordDecl *RD = RT->getDecl();
-
- // If this is a C++ record, check the bases first.
- if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
- if (CXXRD->isDynamicClass())
- return false;
-
- for (const auto &I : CXXRD->bases())
- if (!isEmptyRecordForLayout(Context, I.getType()))
- return false;
- }
-
- for (const auto *I : RD->fields())
- if (!isEmptyFieldForLayout(Context, I))
- return false;
-
- return true;
-}
diff --git a/clang/lib/CodeGen/ABIInfoImpl.cpp b/clang/lib/CodeGen/ABIInfoImpl.cpp
index b1d1b6ee5f526d..1ae7a70839ce0d 100644
--- a/clang/lib/CodeGen/ABIInfoImpl.cpp
+++ b/clang/lib/CodeGen/ABIInfoImpl.cpp
@@ -311,6 +311,41 @@ bool CodeGen::isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays,
return true;
}
+bool CodeGen::isEmptyFieldForLayout(const ASTContext &Context,
+ const FieldDecl *FD) {
+ if (FD->isZeroLengthBitField(Context))
+ return true;
+
+ if (FD->isUnnamedBitField())
+ return false;
+
+ return isEmptyRecordForLayout(Context, FD->getType());
+}
+
+bool CodeGen::isEmptyRecordForLayout(const ASTContext &Context, QualType T) {
+ const RecordType *RT = T->getAs<RecordType>();
+ if (!RT)
+ return false;
+
+ const RecordDecl *RD = RT->getDecl();
+
+ // If this is a C++ record, check the bases first.
+ if (const CXXRecordDecl *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+ if (CXXRD->isDynamicClass())
+ return false;
+
+ for (const auto &I : CXXRD->bases())
+ if (!isEmptyRecordForLayout(Context, I.getType()))
+ return false;
+ }
+
+ for (const auto *I : RD->fields())
+ if (!isEmptyFieldForLayout(Context, I))
+ return false;
+
+ return true;
+}
+
const Type *CodeGen::isSingleElementStruct(QualType T, ASTContext &Context) {
const RecordType *RT = T->getAs<RecordType>();
if (!RT)
diff --git a/clang/lib/CodeGen/ABIInfoImpl.h b/clang/lib/CodeGen/ABIInfoImpl.h
index 92986fb4316465..f6a8884dafec25 100644
--- a/clang/lib/CodeGen/ABIInfoImpl.h
+++ b/clang/lib/CodeGen/ABIInfoImpl.h
@@ -137,6 +137,17 @@ bool isEmptyField(ASTContext &Context, const FieldDecl *FD, bool AllowArrays,
bool isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays,
bool AsIfNoUniqueAddr = false);
+/// isEmptyFieldForLayout - Return true iff the field is "empty", that is,
+/// either a zero-width bit-field or an \ref isEmptyRecordForLayout.
+bool isEmptyFieldForLayout(const ASTContext &Context, const FieldDecl *FD);
+
+/// isEmptyRecordForLayout - Return true iff a structure contains only empty
+/// base classes (per \ref isEmptyRecordForLayout) and fields (per
+/// \ref isEmptyFieldForLayout). Note, C++ record fields are considered empty
+/// if the [[no_unique_address]] attribute would have made them empty.
+bool isEmptyRecordForLayout(const ASTContext &Context, QualType T);
+
+
/// isSingleElementStruct - Determine if a structure is a "single
/// element struct", i.e. it has exactly one non-empty field or
/// exactly one field which is itself a single element
>From 6d55789da9ebb4a37cc4ba98cfc7cdf77f0d264a Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Wed, 2 Oct 2024 13:59:36 +0100
Subject: [PATCH 6/6] fixup! clang-format
---
clang/lib/AST/RecordLayoutBuilder.cpp | 4 ++--
clang/lib/CodeGen/ABIInfoImpl.cpp | 2 +-
clang/lib/CodeGen/ABIInfoImpl.h | 1 -
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index c2117abcbf1e3d..5d998f0f014a4b 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2262,8 +2262,8 @@ uint64_t ItaniumRecordLayoutBuilder::updateExternalFieldOffset(
//
// FIXME: emit enough information in DWARF to get rid of InferAlignment.
//
- CXXRecordDecl * CXX = nullptr;
- if (auto * RT = dyn_cast<RecordType>(Field->getType()))
+ CXXRecordDecl *CXX = nullptr;
+ if (auto *RT = dyn_cast<RecordType>(Field->getType()))
CXX = RT->getAsCXXRecordDecl();
const bool assume_packed = ExternalFieldOffset > 0 &&
diff --git a/clang/lib/CodeGen/ABIInfoImpl.cpp b/clang/lib/CodeGen/ABIInfoImpl.cpp
index 1ae7a70839ce0d..be91b85e3a816f 100644
--- a/clang/lib/CodeGen/ABIInfoImpl.cpp
+++ b/clang/lib/CodeGen/ABIInfoImpl.cpp
@@ -312,7 +312,7 @@ bool CodeGen::isEmptyRecord(ASTContext &Context, QualType T, bool AllowArrays,
}
bool CodeGen::isEmptyFieldForLayout(const ASTContext &Context,
- const FieldDecl *FD) {
+ const FieldDecl *FD) {
if (FD->isZeroLengthBitField(Context))
return true;
diff --git a/clang/lib/CodeGen/ABIInfoImpl.h b/clang/lib/CodeGen/ABIInfoImpl.h
index f6a8884dafec25..2a3ef6b8a6c961 100644
--- a/clang/lib/CodeGen/ABIInfoImpl.h
+++ b/clang/lib/CodeGen/ABIInfoImpl.h
@@ -147,7 +147,6 @@ bool isEmptyFieldForLayout(const ASTContext &Context, const FieldDecl *FD);
/// if the [[no_unique_address]] attribute would have made them empty.
bool isEmptyRecordForLayout(const ASTContext &Context, QualType T);
-
/// isSingleElementStruct - Determine if a structure is a "single
/// element struct", i.e. it has exactly one non-empty field or
/// exactly one field which is itself a single element
More information about the lldb-commits
mailing list