https://github.com/MitalAshok updated
https://github.com/llvm/llvm-project/pull/90462
>From 96ff21d5126ebb4b9a538b8eef11f8ac9e2194c5 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mi...@mitalashok.co.uk>
Date: Mon, 29 Apr 2024 12:27:04 +0100
Subject: [PATCH 1/4] [Clang] Reuse tail-padding for more types that are not
POD for the purpose of layout
This will be done for types with over-large bitfields and
potentially-overlapping ([[no_unique_address]]) members
Compatible with old Clang 18 semantics with -fclang-abi-compat
Fixes #50766
---
clang/docs/ReleaseNotes.rst | 3 +
clang/include/clang/Basic/LangOptions.h | 4 +-
clang/include/clang/Basic/TargetCXXABI.h | 51 --------
clang/lib/AST/RecordLayoutBuilder.cpp | 134 +++++++++++++++-----
clang/test/CodeGenCXX/bitfield-layout.cpp | 20 ++-
clang/test/CodeGenCXX/no-unique-address.cpp | 9 +-
clang/test/Layout/no-unique-address.cpp | 20 ++-
7 files changed, 138 insertions(+), 103 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 604782ca43dd54..b5d76c95a24bd1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -63,6 +63,9 @@ ABI Changes in This Version
MSVC uses a different mangling for these objects, compatibility is not
affected.
(#GH85423).
+- Fixed tail padding not being reused on types with oversized bit-fields and
+ potentially-overlapping members (#GH50766).
+
AST Dumping Potentially Breaking Changes
----------------------------------------
diff --git a/clang/include/clang/Basic/LangOptions.h
b/clang/include/clang/Basic/LangOptions.h
index e2a2aa71b880b3..3fe1362a2d767e 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -233,7 +233,9 @@ class LangOptionsBase {
/// Attempt to be ABI-compatible with code generated by Clang 18.0.x.
/// This causes clang to revert some fixes to the mangling of lambdas
- /// in the initializers of members of local classes.
+ /// in the initializers of members of local classes and will not reuse
+ /// tail padding on structs with potentially-overlapping members or
+ /// oversized bit-fields under Itanium and some derived ABIs.
Ver18,
/// Conform to the underlying platform's C and C++ ABIs as closely
diff --git a/clang/include/clang/Basic/TargetCXXABI.h
b/clang/include/clang/Basic/TargetCXXABI.h
index c113a6a048ad44..45d9dcc8f1b8a8 100644
--- a/clang/include/clang/Basic/TargetCXXABI.h
+++ b/clang/include/clang/Basic/TargetCXXABI.h
@@ -255,57 +255,6 @@ class TargetCXXABI {
llvm_unreachable("bad ABI kind");
}
- /// When is record layout allowed to allocate objects in the tail
- /// padding of a base class?
- ///
- /// This decision cannot be changed without breaking platform ABI
- /// compatibility. In ISO C++98, tail padding reuse was only permitted for
- /// non-POD base classes, but that restriction was removed retroactively by
- /// DR 43, and tail padding reuse is always permitted in all de facto C++
- /// language modes. However, many platforms use a variant of the old C++98
- /// rule for compatibility.
- enum TailPaddingUseRules {
- /// The tail-padding of a base class is always theoretically
- /// available, even if it's POD.
- AlwaysUseTailPadding,
-
- /// Only allocate objects in the tail padding of a base class if
- /// the base class is not POD according to the rules of C++ TR1.
- UseTailPaddingUnlessPOD03,
-
- /// Only allocate objects in the tail padding of a base class if
- /// the base class is not POD according to the rules of C++11.
- UseTailPaddingUnlessPOD11
- };
- TailPaddingUseRules getTailPaddingUseRules() const {
- switch (getKind()) {
- // To preserve binary compatibility, the generic Itanium ABI has
- // permanently locked the definition of POD to the rules of C++ TR1,
- // and that trickles down to derived ABIs.
- case GenericItanium:
- case GenericAArch64:
- case GenericARM:
- case iOS:
- case GenericMIPS:
- case XL:
- return UseTailPaddingUnlessPOD03;
-
- // AppleARM64 and WebAssembly use the C++11 POD rules. They do not honor
- // the Itanium exception about classes with over-large bitfields.
- case AppleARM64:
- case Fuchsia:
- case WebAssembly:
- case WatchOS:
- return UseTailPaddingUnlessPOD11;
-
- // MSVC always allocates fields in the tail-padding of a base class
- // subobject, even if they're POD.
- case Microsoft:
- return AlwaysUseTailPadding;
- }
- llvm_unreachable("bad ABI kind");
- }
-
friend bool operator==(const TargetCXXABI &left, const TargetCXXABI &right) {
return left.getKind() == right.getKind();
}
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index d9bf62c2bbb04a..0b6376381a53b7 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2415,46 +2415,112 @@ DiagnosticBuilder
ItaniumRecordLayoutBuilder::Diag(SourceLocation Loc,
return Context.getDiagnostics().Report(Loc, DiagID);
}
+/// https://itanium-cxx-abi.github.io/cxx-abi/abi.html#POD
+/// POD for the purpose of layout
+/// In general, a type is considered a POD for the purposes of layout if it
is
+/// a POD type (in the sense of ISO C++ [basic.types]). However, a type is
not
+/// considered to be a POD for the purpose of layout if it is:
+/// - a POD-struct or POD-union (in the sense of ISO C++ [class]) with a
+/// bit-field whose declared width is wider than the declared type of the
+/// bit-field, or
+/// - an array type whose element type is not a POD for the purpose of
+/// layout, or
+/// - a POD-struct with one or more potentially-overlapping non-static data
+/// members.
+/// Where references to the ISO C++ are made in this paragraph, the Technical
+/// Corrigendum 1 version of the standard is intended.
+///
+/// This function does not check if the type is POD first
+static bool isItaniumPOD(const ASTContext &Context, const CXXRecordDecl *RD) {
+ const auto IsDisqualifying = [&](const FieldDecl *FD) -> bool {
+ if (FD->isBitField())
+ if (FD->getBitWidthValue(Context) > Context.getTypeSize(FD->getType()))
+ return true;
+
+ return FD->isPotentiallyOverlapping();
+ };
+
+ if (llvm::any_of(RD->fields(), IsDisqualifying))
+ return false;
+
+ return RD->forallBases([&](const CXXRecordDecl *Base) -> bool {
+ return llvm::any_of(Base->fields(), IsDisqualifying);
+ });
+}
+
/// Does the target C++ ABI require us to skip over the tail-padding
/// of the given class (considering it as a base class) when allocating
/// objects?
-static bool mustSkipTailPadding(TargetCXXABI ABI, const CXXRecordDecl *RD) {
- switch (ABI.getTailPaddingUseRules()) {
- case TargetCXXABI::AlwaysUseTailPadding:
- return false;
+static bool mustSkipTailPadding(const ASTContext &Context, TargetCXXABI ABI,
+ const CXXRecordDecl *RD) {
+ // This is equivalent to
+ // Context.getTypeDeclType(RD).isCXX11PODType(Context),
+ // but with a lot of abstraction penalty stripped off. This does
+ // assume that these properties are set correctly even in C++98
+ // mode; fortunately, that is true because we want to assign
+ // consistently semantics to the type-traits intrinsics (or at
+ // least as many of them as possible).
+ auto IsCXX11PODType = [&]() -> bool {
+ return RD->isTrivial() && RD->isCXX11StandardLayout();
+ };
+
+ if (Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver18) {
+ switch (ABI.getKind()) {
+ // ABIs derived from Itanium: In Clang 18, did not check for over-large
+ // bit-fields or potentially overlapping members
+ case TargetCXXABI::GenericItanium:
+ case TargetCXXABI::GenericAArch64:
+ case TargetCXXABI::GenericARM:
+ case TargetCXXABI::iOS:
+ case TargetCXXABI::GenericMIPS:
+ case TargetCXXABI::XL:
+ return RD->isPOD();
+
+ case TargetCXXABI::AppleARM64:
+ case TargetCXXABI::Fuchsia:
+ case TargetCXXABI::WebAssembly:
+ case TargetCXXABI::WatchOS:
+ return IsCXX11PODType();
+
+ case TargetCXXABI::Microsoft:
+ return true;
+ }
+ llvm_unreachable("bad ABI kind");
+ }
+
+ switch (ABI.getKind()) {
+ // To preserve binary compatibility, the generic Itanium ABI has permanently
+ // locked the definition of POD to the rules of C++ TR1, and that trickles
+ // down to derived ABIs.
+ case TargetCXXABI::GenericItanium:
+ case TargetCXXABI::GenericAArch64:
+ case TargetCXXABI::GenericARM:
+ case TargetCXXABI::GenericMIPS:
+ return RD->isPOD() && isItaniumPOD(Context, RD);
- case TargetCXXABI::UseTailPaddingUnlessPOD03:
- // FIXME: To the extent that this is meant to cover the Itanium ABI
- // rules, we should implement the restrictions about over-sized
- // bitfields:
- //
- // http://itanium-cxx-abi.github.io/cxx-abi/abi.html#POD :
- // In general, a type is considered a POD for the purposes of
- // layout if it is a POD type (in the sense of ISO C++
- // [basic.types]). However, a POD-struct or POD-union (in the
- // sense of ISO C++ [class]) with a bitfield member whose
- // declared width is wider than the declared type of the
- // bitfield is not a POD for the purpose of layout. Similarly,
- // an array type is not a POD for the purpose of layout if the
- // element type of the array is not a POD for the purpose of
- // layout.
- //
- // Where references to the ISO C++ are made in this paragraph,
- // the Technical Corrigendum 1 version of the standard is
- // intended.
+ case TargetCXXABI::XL:
+ case TargetCXXABI::iOS:
return RD->isPOD();
- case TargetCXXABI::UseTailPaddingUnlessPOD11:
- // This is equivalent to RD->getTypeForDecl().isCXX11PODType(),
- // but with a lot of abstraction penalty stripped off. This does
- // assume that these properties are set correctly even in C++98
- // mode; fortunately, that is true because we want to assign
- // consistently semantics to the type-traits intrinsics (or at
- // least as many of them as possible).
- return RD->isTrivial() && RD->isCXX11StandardLayout();
+ //
https://github.com/WebAssembly/tool-conventions/blob/cd83f847828336f10643d1f48aa60867c428c55c/ItaniumLikeC%2B%2BABI.md
+ // The same as Itanium except with C++11 POD instead of C++ TC1 POD
+ case TargetCXXABI::WebAssembly:
+ return IsCXX11PODType() && isItaniumPOD(Context, RD);
+
+ // Also uses C++11 POD but do not honor the Itanium exception about classes
+ // with over-large bitfields.
+ case TargetCXXABI::AppleARM64:
+ case TargetCXXABI::WatchOS:
+ case TargetCXXABI::Fuchsia:
+ return IsCXX11PODType();
+
+ // MSVC always allocates fields in the tail-padding of a base class
+ // subobject, even if they're POD.
+ case TargetCXXABI::Microsoft:
+ return true;
}
-
- llvm_unreachable("bad tail-padding use kind");
+ llvm_unreachable("bad ABI kind");
}
static bool isMsLayout(const ASTContext &Context) {
@@ -3388,7 +3454,7 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const
{
// tail-padding of base classes. This is ABI-dependent.
// FIXME: this should be stored in the record layout.
bool skipTailPadding =
- mustSkipTailPadding(getTargetInfo().getCXXABI(), RD);
+ mustSkipTailPadding(*this, getTargetInfo().getCXXABI(), RD);
// FIXME: This should be done in FinalizeLayout.
CharUnits DataSize =
diff --git a/clang/test/CodeGenCXX/bitfield-layout.cpp
b/clang/test/CodeGenCXX/bitfield-layout.cpp
index 66a140a966c640..abf43b2f552801 100644
--- a/clang/test/CodeGenCXX/bitfield-layout.cpp
+++ b/clang/test/CodeGenCXX/bitfield-layout.cpp
@@ -1,10 +1,16 @@
-// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - -O3 |
FileCheck -check-prefix=CHECK -check-prefix=CHECK-LP64 %s
-// RUN: %clang_cc1 %s -triple=i386-apple-darwin10 -emit-llvm -o - -O3 |
FileCheck %s
-// RUN: %clang_cc1 %s -triple=aarch64_be -emit-llvm -o - -O3 | FileCheck %s
-// RUN: %clang_cc1 %s -triple=thumbv7_be-none-eabi -emit-llvm -o - -O3 |
FileCheck %s
-// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -emit-llvm -o - -O3
-std=c++11 | FileCheck -check-prefix=CHECK -check-prefix=CHECK-LP64 %s
-
-// CHECK-LP64: %union.Test1 = type { i32, [4 x i8] }
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -o - -O3 |
FileCheck -check-prefix=CHECK -check-prefix=CHECK-LP64 --check-prefix=NEW
--check-prefix=NEW-LP64 %s --allow-unused-prefixes
+// RUN: %clang_cc1 %s -triple=i386-apple-darwin10 -emit-llvm -o - -O3 |
FileCheck --check-prefix=CHECK --check-prefix=NEW %s --allow-unused-prefixes
+// RUN: %clang_cc1 %s -triple=aarch64_be -emit-llvm -o - -O3 | FileCheck
--check-prefix=CHECK --check-prefix=NEW %s --allow-unused-prefixes
+// RUN: %clang_cc1 %s -triple=thumbv7_be-none-eabi -emit-llvm -o - -O3 |
FileCheck --check-prefix=CHECK --check-prefix=NEW %s --allow-unused-prefixes
+// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -emit-llvm -o - -O3
-std=c++11 | FileCheck -check-prefix=CHECK -check-prefix=CHECK-LP64
--check-prefix=NEW --check-prefix=NEW-LP64 %s --allow-unused-prefixes
+// RUN: %clang_cc1 %s -fclang-abi-compat=18.0 -triple=x86_64-apple-darwin10
-emit-llvm -o - -O3 | FileCheck -check-prefix=CHECK -check-prefix=CHECK-LP64
--check-prefix=OLD --check-prefix=OLD-LP64 %s --allow-unused-prefixes
+// RUN: %clang_cc1 %s -fclang-abi-compat=18.0 -triple=i386-apple-darwin10
-emit-llvm -o - -O3 | FileCheck --check-prefix=CHECK --check-prefix=OLD %s
--allow-unused-prefixes
+// RUN: %clang_cc1 %s -fclang-abi-compat=18.0 -triple=aarch64_be -emit-llvm -o
- -O3 | FileCheck --check-prefix=CHECK --check-prefix=OLD %s
--allow-unused-prefixes
+// RUN: %clang_cc1 %s -fclang-abi-compat=18.0 -triple=thumbv7_be-none-eabi
-emit-llvm -o - -O3 | FileCheck --check-prefix=CHECK --check-prefix=OLD %s
--allow-unused-prefixes
+// RUN: %clang_cc1 %s -fclang-abi-compat=18.0 -triple=x86_64-unknown-unknown
-emit-llvm -o - -O3 -std=c++11 | FileCheck -check-prefix=CHECK
-check-prefix=CHECK-LP64 --check-prefix=OLD --check-prefix=OLD-LP64 %s
--allow-unused-prefixes
+
+// OLD-LP64: %union.Test1 = type { i32, [4 x i8] }
+// NEW-LP64: %union.Test1 = type <{ i32, [4 x i8] }>
union Test1 {
int a;
int b: 39;
diff --git a/clang/test/CodeGenCXX/no-unique-address.cpp
b/clang/test/CodeGenCXX/no-unique-address.cpp
index 7b4bbbf2a05d51..123069b79f2046 100644
--- a/clang/test/CodeGenCXX/no-unique-address.cpp
+++ b/clang/test/CodeGenCXX/no-unique-address.cpp
@@ -1,5 +1,7 @@
-// RUN: %clang_cc1 -std=c++2a %s -emit-llvm -o - -triple x86_64-linux-gnu |
FileCheck %s
-// RUN: %clang_cc1 -std=c++2a %s -emit-llvm -o - -triple x86_64-linux-gnu -O2
-disable-llvm-passes | FileCheck %s --check-prefix=CHECK-OPT
+// RUN: %clang_cc1 -std=c++2a %s -emit-llvm -o - -triple x86_64-linux-gnu |
FileCheck %s --check-prefix=CHECK --check-prefix=NEW --allow-unused-prefixes
+// RUN: %clang_cc1 -std=c++2a %s -emit-llvm -o - -triple x86_64-linux-gnu -O2
-disable-llvm-passes | FileCheck %s --check-prefix=CHECK-OPT
--check-prefix=NEW-OPT --allow-unused-prefixes
+// RUN: %clang_cc1 -fclang-abi-compat=18.0 -std=c++2a %s -emit-llvm -o -
-triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK --check-prefix=OLD
--allow-unused-prefixes
+// RUN: %clang_cc1 -fclang-abi-compat=18.0 -std=c++2a %s -emit-llvm -o -
-triple x86_64-linux-gnu -O2 -disable-llvm-passes | FileCheck %s
--check-prefix=CHECK-OPT --check-prefix=OLD-OPT --allow-unused-prefixes
struct A { ~A(); int n; char c[3]; };
struct B { [[no_unique_address]] A a; char k; };
@@ -39,7 +41,8 @@ Empty1 HasEmptyDuplicates::*off2 = &HasEmptyDuplicates::e2;
// CHECK-DAG: @off3 ={{.*}} global i64 8
Empty1 HasEmptyDuplicates::*off3 = &HasEmptyDuplicates::e3;
-// CHECK-DAG: @hed ={{.*}} global %{{[^ ]*}} { i32 1, i32 2, [4 x i8] undef }
+// OLD-DAG: @hed ={{.*}} global %struct.HasEmptyDuplicates { i32 1, i32 2, [4
x i8] undef }, align 4
+// NEW-DAG: @hed ={{.*}} global { i32, i32, [4 x i8] } { i32 1, i32 2, [4 x
i8] undef }, align 4
HasEmptyDuplicates hed = {{}, 1, {}, 2, {}};
struct __attribute__((packed, aligned(2))) PackedAndPadded {
diff --git a/clang/test/Layout/no-unique-address.cpp
b/clang/test/Layout/no-unique-address.cpp
index d5bb46647b88d0..60abd01ce5648a 100644
--- a/clang/test/Layout/no-unique-address.cpp
+++ b/clang/test/Layout/no-unique-address.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-linux-gnu
-fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-linux-gnu
-fdump-record-layouts %s | FileCheck %s --check-prefix=NEW --check-prefix=CHECK
+// RUN: %clang_cc1 -fclang-abi-compat=18.0 -DOLD_ABI=1 -std=c++2a
-fsyntax-only -triple x86_64-linux-gnu -fdump-record-layouts %s | FileCheck %s
--check-prefix=OLD --check-prefix=CHECK
namespace Empty {
struct A {};
@@ -40,7 +41,8 @@ namespace Empty {
// CHECK-NEXT: 0 | struct Empty::A a1 (empty)
// CHECK-NEXT: 1 | struct Empty::A a2 (empty)
// CHECK-NEXT: 0 | char e
- // CHECK-NEXT: | [sizeof=2, dsize=2, align=1,
+ // OLD-NEXT: | [sizeof=2, dsize=2, align=1,
+ // NEW-NEXT: | [sizeof=2, dsize=1, align=1,
// CHECK-NEXT: | nvsize=2, nvalign=1]
struct F {
@@ -120,8 +122,10 @@ namespace Empty {
// CHECK-NEXT: 4 | struct Empty::A b (empty)
// CHECK-NEXT: 4 | int y
// CHECK-NEXT: 8 | struct Empty::A c (empty)
- // CHECK-NEXT: | [sizeof=12, dsize=12, align=4,
- // CHECK-NEXT: | nvsize=12, nvalign=4]
+ // OLD-NEXT: | [sizeof=12, dsize=12, align=4,
+ // OLD-NEXT: | nvsize=12, nvalign=4]
+ // NEW-NEXT: | [sizeof=12, dsize=8, align=4,
+ // NEW-NEXT: | nvsize=9, nvalign=4]
struct EmptyWithNonzeroDSizeNonPOD {
~EmptyWithNonzeroDSizeNonPOD();
@@ -145,7 +149,7 @@ namespace Empty {
}
namespace POD {
- // Cannot reuse tail padding of a PDO type.
+ // Cannot reuse tail padding of a POD type.
struct A { int n; char c[3]; };
struct B { [[no_unique_address]] A a; char d; };
static_assert(sizeof(B) == 12);
@@ -156,8 +160,10 @@ namespace POD {
// CHECK-NEXT: 0 | int n
// CHECK-NEXT: 4 | char[3] c
// CHECK-NEXT: 8 | char d
- // CHECK-NEXT: | [sizeof=12, dsize=12, align=4,
- // CHECK-NEXT: | nvsize=12, nvalign=4]
+ // OLD-NEXT: | [sizeof=12, dsize=12, align=4,
+ // OLD-NEXT: | nvsize=12, nvalign=4]
+ // NEW-NEXT: | [sizeof=12, dsize=9, align=4,
+ // NEW-NEXT: | nvsize=9, nvalign=4]
}
namespace NonPOD {
>From 6c040b4fca91f07b9c67219728ce7aed80510ac0 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mi...@mitalashok.co.uk>
Date: Wed, 22 May 2024 18:02:28 +0100
Subject: [PATCH 2/4] Reorganise into a single switch + comments
---
clang/lib/AST/RecordLayoutBuilder.cpp | 68 +++++++++++++--------------
1 file changed, 34 insertions(+), 34 deletions(-)
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 0b6376381a53b7..5169b03e3e8e89 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2451,6 +2451,13 @@ static bool isItaniumPOD(const ASTContext &Context,
const CXXRecordDecl *RD) {
/// Does the target C++ ABI require us to skip over the tail-padding
/// of the given class (considering it as a base class) when allocating
/// objects?
+///
+/// This decision cannot be changed without breaking platform ABI
+/// compatibility. In ISO C++98, tail padding reuse was only permitted for
+/// non-POD base classes, but that restriction was removed retroactively by
+/// DR 43, and tail padding reuse is always permitted in all de facto C++
+/// language modes. However, many platforms use a variant of the old C++98
+/// rule for compatibility.
static bool mustSkipTailPadding(const ASTContext &Context, TargetCXXABI ABI,
const CXXRecordDecl *RD) {
// This is equivalent to
@@ -2458,37 +2465,12 @@ static bool mustSkipTailPadding(const ASTContext
&Context, TargetCXXABI ABI,
// but with a lot of abstraction penalty stripped off. This does
// assume that these properties are set correctly even in C++98
// mode; fortunately, that is true because we want to assign
- // consistently semantics to the type-traits intrinsics (or at
+ // consistent semantics to the type-traits intrinsics (or at
// least as many of them as possible).
auto IsCXX11PODType = [&]() -> bool {
return RD->isTrivial() && RD->isCXX11StandardLayout();
};
- if (Context.getLangOpts().getClangABICompat() <=
- LangOptions::ClangABI::Ver18) {
- switch (ABI.getKind()) {
- // ABIs derived from Itanium: In Clang 18, did not check for over-large
- // bit-fields or potentially overlapping members
- case TargetCXXABI::GenericItanium:
- case TargetCXXABI::GenericAArch64:
- case TargetCXXABI::GenericARM:
- case TargetCXXABI::iOS:
- case TargetCXXABI::GenericMIPS:
- case TargetCXXABI::XL:
- return RD->isPOD();
-
- case TargetCXXABI::AppleARM64:
- case TargetCXXABI::Fuchsia:
- case TargetCXXABI::WebAssembly:
- case TargetCXXABI::WatchOS:
- return IsCXX11PODType();
-
- case TargetCXXABI::Microsoft:
- return true;
- }
- llvm_unreachable("bad ABI kind");
- }
-
switch (ABI.getKind()) {
// To preserve binary compatibility, the generic Itanium ABI has permanently
// locked the definition of POD to the rules of C++ TR1, and that trickles
@@ -2497,26 +2479,44 @@ static bool mustSkipTailPadding(const ASTContext
&Context, TargetCXXABI ABI,
case TargetCXXABI::GenericAArch64:
case TargetCXXABI::GenericARM:
case TargetCXXABI::GenericMIPS:
- return RD->isPOD() && isItaniumPOD(Context, RD);
-
- case TargetCXXABI::XL:
+ if (!RD->isPOD())
+ return false;
+ // Prior to Clang 19, over-large bitfields and potentially overlapping
+ // members were not checked
+ return (Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver18) ||
+ isItaniumPOD(Context, RD);
+
+ // The same as generic Itanium but does not honor the exception about classes
+ // with over-large bit-fields.
+ // FIXME: do the iOS/AppleARM64/WatchOS ABI care about
potentially-overlapping
+ // members?
case TargetCXXABI::iOS:
return RD->isPOD();
//
https://github.com/WebAssembly/tool-conventions/blob/cd83f847828336f10643d1f48aa60867c428c55c/ItaniumLikeC%2B%2BABI.md
// The same as Itanium except with C++11 POD instead of C++ TC1 POD
case TargetCXXABI::WebAssembly:
- return IsCXX11PODType() && isItaniumPOD(Context, RD);
+ if (!IsCXX11PODType())
+ return false;
+ return (Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver18) ||
+ isItaniumPOD(Context, RD);
- // Also uses C++11 POD but do not honor the Itanium exception about classes
- // with over-large bitfields.
+ // Also use C++11 POD but without honoring the exception about classes with
+ // over-large bit-fields.
case TargetCXXABI::AppleARM64:
case TargetCXXABI::WatchOS:
+ return IsCXX11PODType();
+
+ // FIXME: do these two ABIs need to check isItaniumPOD?
+ case TargetCXXABI::XL:
+ return RD->isPOD();
case TargetCXXABI::Fuchsia:
return IsCXX11PODType();
- // MSVC always allocates fields in the tail-padding of a base class
- // subobject, even if they're POD.
+ // MSVC always allocates fields in the tail-padding of a base class
subobject,
+ // even if they're POD.
case TargetCXXABI::Microsoft:
return true;
}
>From b98b2e6613e4dbbe86157c2cf4e0ebd9f545e9ab Mon Sep 17 00:00:00 2001
From: Mital Ashok <mi...@mitalashok.co.uk>
Date: Thu, 8 Aug 2024 09:45:12 +0100
Subject: [PATCH 3/4] Clang 19 -> Clang 20
---
clang/include/clang/Basic/LangOptions.h | 10 +++++++---
clang/lib/AST/RecordLayoutBuilder.cpp | 6 +++---
clang/lib/Frontend/CompilerInvocation.cpp | 5 +++++
clang/test/CodeGenCXX/bitfield-layout.cpp | 10 +++++-----
clang/test/CodeGenCXX/no-unique-address.cpp | 8 ++++----
clang/test/Layout/no-unique-address.cpp | 2 +-
6 files changed, 25 insertions(+), 16 deletions(-)
diff --git a/clang/include/clang/Basic/LangOptions.h
b/clang/include/clang/Basic/LangOptions.h
index f60216561860e5..dbd80d42a45a41 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -233,11 +233,15 @@ class LangOptionsBase {
/// Attempt to be ABI-compatible with code generated by Clang 18.0.x.
/// This causes clang to revert some fixes to the mangling of lambdas
- /// in the initializers of members of local classes and will not reuse
- /// tail padding on structs with potentially-overlapping members or
- /// oversized bit-fields under Itanium and some derived ABIs.
+ /// in the initializers of members of local classes.
Ver18,
+ /// Attempt to be ABI-compatible with code generated by Clang 19.0.x.
+ /// This causes clang to not reuse tail padding on structs with
+ /// potentially-overlapping members or oversized bit-fields under Itanium
+ /// and some derived ABIs.
+ Ver19,
+
/// Conform to the underlying platform's C and C++ ABIs as closely
/// as we can.
Latest
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 5169b03e3e8e89..638a2c7e2b0a73 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2481,10 +2481,10 @@ static bool mustSkipTailPadding(const ASTContext
&Context, TargetCXXABI ABI,
case TargetCXXABI::GenericMIPS:
if (!RD->isPOD())
return false;
- // Prior to Clang 19, over-large bitfields and potentially overlapping
+ // Prior to Clang 20, over-large bitfields and potentially overlapping
// members were not checked
return (Context.getLangOpts().getClangABICompat() <=
- LangOptions::ClangABI::Ver18) ||
+ LangOptions::ClangABI::Ver19) ||
isItaniumPOD(Context, RD);
// The same as generic Itanium but does not honor the exception about classes
@@ -2500,7 +2500,7 @@ static bool mustSkipTailPadding(const ASTContext
&Context, TargetCXXABI ABI,
if (!IsCXX11PODType())
return false;
return (Context.getLangOpts().getClangABICompat() <=
- LangOptions::ClangABI::Ver18) ||
+ LangOptions::ClangABI::Ver19) ||
isItaniumPOD(Context, RD);
// Also use C++11 POD but without honoring the exception about classes with
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp
b/clang/lib/Frontend/CompilerInvocation.cpp
index 225bd6416ce5fc..378498bf983c7f 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3796,6 +3796,9 @@ void CompilerInvocationBase::GenerateLangArgs(const
LangOptions &Opts,
case LangOptions::ClangABI::Ver18:
GenerateArg(Consumer, OPT_fclang_abi_compat_EQ, "18.0");
break;
+ case LangOptions::ClangABI::Ver19:
+ GenerateArg(Consumer, OPT_fclang_abi_compat_EQ, "19.0");
+ break;
case LangOptions::ClangABI::Latest:
break;
}
@@ -4305,6 +4308,8 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts,
ArgList &Args,
Opts.setClangABICompat(LangOptions::ClangABI::Ver17);
else if (Major <= 18)
Opts.setClangABICompat(LangOptions::ClangABI::Ver18);
+ else if (Major <= 19)
+ Opts.setClangABICompat(LangOptions::ClangABI::Ver19);
} else if (Ver != "latest") {
Diags.Report(diag::err_drv_invalid_value)
<< A->getAsString(Args) << A->getValue();
diff --git a/clang/test/CodeGenCXX/bitfield-layout.cpp
b/clang/test/CodeGenCXX/bitfield-layout.cpp
index abf43b2f552801..9a6b5bf24c84ec 100644
--- a/clang/test/CodeGenCXX/bitfield-layout.cpp
+++ b/clang/test/CodeGenCXX/bitfield-layout.cpp
@@ -3,11 +3,11 @@
// RUN: %clang_cc1 %s -triple=aarch64_be -emit-llvm -o - -O3 | FileCheck
--check-prefix=CHECK --check-prefix=NEW %s --allow-unused-prefixes
// RUN: %clang_cc1 %s -triple=thumbv7_be-none-eabi -emit-llvm -o - -O3 |
FileCheck --check-prefix=CHECK --check-prefix=NEW %s --allow-unused-prefixes
// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -emit-llvm -o - -O3
-std=c++11 | FileCheck -check-prefix=CHECK -check-prefix=CHECK-LP64
--check-prefix=NEW --check-prefix=NEW-LP64 %s --allow-unused-prefixes
-// RUN: %clang_cc1 %s -fclang-abi-compat=18.0 -triple=x86_64-apple-darwin10
-emit-llvm -o - -O3 | FileCheck -check-prefix=CHECK -check-prefix=CHECK-LP64
--check-prefix=OLD --check-prefix=OLD-LP64 %s --allow-unused-prefixes
-// RUN: %clang_cc1 %s -fclang-abi-compat=18.0 -triple=i386-apple-darwin10
-emit-llvm -o - -O3 | FileCheck --check-prefix=CHECK --check-prefix=OLD %s
--allow-unused-prefixes
-// RUN: %clang_cc1 %s -fclang-abi-compat=18.0 -triple=aarch64_be -emit-llvm -o
- -O3 | FileCheck --check-prefix=CHECK --check-prefix=OLD %s
--allow-unused-prefixes
-// RUN: %clang_cc1 %s -fclang-abi-compat=18.0 -triple=thumbv7_be-none-eabi
-emit-llvm -o - -O3 | FileCheck --check-prefix=CHECK --check-prefix=OLD %s
--allow-unused-prefixes
-// RUN: %clang_cc1 %s -fclang-abi-compat=18.0 -triple=x86_64-unknown-unknown
-emit-llvm -o - -O3 -std=c++11 | FileCheck -check-prefix=CHECK
-check-prefix=CHECK-LP64 --check-prefix=OLD --check-prefix=OLD-LP64 %s
--allow-unused-prefixes
+// RUN: %clang_cc1 %s -fclang-abi-compat=19.0 -triple=x86_64-apple-darwin10
-emit-llvm -o - -O3 | FileCheck -check-prefix=CHECK -check-prefix=CHECK-LP64
--check-prefix=OLD --check-prefix=OLD-LP64 %s --allow-unused-prefixes
+// RUN: %clang_cc1 %s -fclang-abi-compat=19.0 -triple=i386-apple-darwin10
-emit-llvm -o - -O3 | FileCheck --check-prefix=CHECK --check-prefix=OLD %s
--allow-unused-prefixes
+// RUN: %clang_cc1 %s -fclang-abi-compat=19.0 -triple=aarch64_be -emit-llvm -o
- -O3 | FileCheck --check-prefix=CHECK --check-prefix=OLD %s
--allow-unused-prefixes
+// RUN: %clang_cc1 %s -fclang-abi-compat=19.0 -triple=thumbv7_be-none-eabi
-emit-llvm -o - -O3 | FileCheck --check-prefix=CHECK --check-prefix=OLD %s
--allow-unused-prefixes
+// RUN: %clang_cc1 %s -fclang-abi-compat=19.0 -triple=x86_64-unknown-unknown
-emit-llvm -o - -O3 -std=c++11 | FileCheck -check-prefix=CHECK
-check-prefix=CHECK-LP64 --check-prefix=OLD --check-prefix=OLD-LP64 %s
--allow-unused-prefixes
// OLD-LP64: %union.Test1 = type { i32, [4 x i8] }
// NEW-LP64: %union.Test1 = type <{ i32, [4 x i8] }>
diff --git a/clang/test/CodeGenCXX/no-unique-address.cpp
b/clang/test/CodeGenCXX/no-unique-address.cpp
index bc6bf63009f459..85eb8991fb7293 100644
--- a/clang/test/CodeGenCXX/no-unique-address.cpp
+++ b/clang/test/CodeGenCXX/no-unique-address.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -std=c++2a %s -emit-llvm -o - -triple x86_64-linux-gnu |
FileCheck %s --check-prefix=CHECK --check-prefix=NEW --allow-unused-prefixes
-// RUN: %clang_cc1 -std=c++2a %s -emit-llvm -o - -triple x86_64-linux-gnu -O2
-disable-llvm-passes | FileCheck %s --check-prefix=CHECK-OPT
--check-prefix=NEW-OPT --allow-unused-prefixes
-// RUN: %clang_cc1 -fclang-abi-compat=18.0 -std=c++2a %s -emit-llvm -o -
-triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK --check-prefix=OLD
--allow-unused-prefixes
-// RUN: %clang_cc1 -fclang-abi-compat=18.0 -std=c++2a %s -emit-llvm -o -
-triple x86_64-linux-gnu -O2 -disable-llvm-passes | FileCheck %s
--check-prefix=CHECK-OPT --check-prefix=OLD-OPT --allow-unused-prefixes
+// RUN: %clang_cc1 -std=c++20 %s -emit-llvm -o - -triple x86_64-linux-gnu |
FileCheck %s --check-prefixes=CHECK,NEW --allow-unused-prefixes
+// RUN: %clang_cc1 -std=c++20 %s -emit-llvm -o - -triple x86_64-linux-gnu -O2
-disable-llvm-passes | FileCheck %s --check-prefixes=CHECK-OPT,NEW-OPT
--allow-unused-prefixes
+// RUN: %clang_cc1 -fclang-abi-compat=19.0 -std=c++20 %s -emit-llvm -o -
-triple x86_64-linux-gnu | FileCheck %s --check-prefixes=CHECK,OLD
--allow-unused-prefixes
+// RUN: %clang_cc1 -fclang-abi-compat=19.0 -std=c++20 %s -emit-llvm -o -
-triple x86_64-linux-gnu -O2 -disable-llvm-passes | FileCheck %s
--check-prefixes=CHECK-OPT,OLD-OPT --allow-unused-prefixes
struct A { ~A(); int n; char c[3]; };
struct B { [[no_unique_address]] A a; char k; };
diff --git a/clang/test/Layout/no-unique-address.cpp
b/clang/test/Layout/no-unique-address.cpp
index 60abd01ce5648a..6f718b843daedb 100644
--- a/clang/test/Layout/no-unique-address.cpp
+++ b/clang/test/Layout/no-unique-address.cpp
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-linux-gnu
-fdump-record-layouts %s | FileCheck %s --check-prefix=NEW --check-prefix=CHECK
-// RUN: %clang_cc1 -fclang-abi-compat=18.0 -DOLD_ABI=1 -std=c++2a
-fsyntax-only -triple x86_64-linux-gnu -fdump-record-layouts %s | FileCheck %s
--check-prefix=OLD --check-prefix=CHECK
+// RUN: %clang_cc1 -fclang-abi-compat=19.0 -DOLD_ABI=1 -std=c++2a
-fsyntax-only -triple x86_64-linux-gnu -fdump-record-layouts %s | FileCheck %s
--check-prefix=OLD --check-prefix=CHECK
namespace Empty {
struct A {};
>From cf8be1bac0eb37caaaecd47cb463ca58ee0fbe59 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mi...@mitalashok.co.uk>
Date: Thu, 8 Aug 2024 11:40:46 +0100
Subject: [PATCH 4/4] Consider "_BitInt(5) : 6" to be oversized; add tests for
bit-fields
---
clang/lib/AST/RecordLayoutBuilder.cpp | 12 +-
.../test/Layout/itanium-padded-bit-field.cpp | 106 ++++++++++++++++++
clang/test/Layout/no-unique-address.cpp | 4 +-
3 files changed, 118 insertions(+), 4 deletions(-)
create mode 100644 clang/test/Layout/itanium-padded-bit-field.cpp
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 638a2c7e2b0a73..999aec72b9db0a 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2433,9 +2433,17 @@ DiagnosticBuilder
ItaniumRecordLayoutBuilder::Diag(SourceLocation Loc,
/// This function does not check if the type is POD first
static bool isItaniumPOD(const ASTContext &Context, const CXXRecordDecl *RD) {
const auto IsDisqualifying = [&](const FieldDecl *FD) -> bool {
- if (FD->isBitField())
- if (FD->getBitWidthValue(Context) > Context.getTypeSize(FD->getType()))
+ if (FD->isBitField()) {
+ QualType DeclaredType = FD->getType();
+ unsigned DeclaredWidth;
+ if (const BitIntType *BIT = DeclaredType->getAs<BitIntType>())
+ DeclaredWidth = BIT->getNumBits();
+ else
+ DeclaredWidth = Context.getTypeSize(FD->getType());
+
+ if (FD->getBitWidthValue(Context) > DeclaredWidth)
return true;
+ }
return FD->isPotentiallyOverlapping();
};
diff --git a/clang/test/Layout/itanium-padded-bit-field.cpp
b/clang/test/Layout/itanium-padded-bit-field.cpp
new file mode 100644
index 00000000000000..f2a8c98af0d038
--- /dev/null
+++ b/clang/test/Layout/itanium-padded-bit-field.cpp
@@ -0,0 +1,106 @@
+// RUN: %clang_cc1 -DOLD_ABI=false -std=c++17 -fsyntax-only -triple
%itanium_abi_triple -fdump-record-layouts -Wno-bitfield-width %s | FileCheck %s
--check-prefixes=CHECK,NEW
+// RUN: %clang_cc1 -fclang-abi-compat=19.0 -DOLD_ABI=true -std=c++17
-fsyntax-only -triple %itanium_abi_triple -fdump-record-layouts
-Wno-bitfield-width %s | FileCheck %s --check-prefixes=CHECK,OLD
+
+using i4 = __INT32_TYPE__;
+using i1 = __INT8_TYPE__;
+
+struct OversizedBase {
+ i4 i : 33;
+};
+
+static_assert(sizeof(OversizedBase) == 8);
+// CHECK:*** Dumping AST Record Layout
+// CHECK: 0 | struct OversizedBase
+// CHECK-NEXT: 0:0-32 | i4 i
+
+// NEW-NEXT: | [sizeof=8, dsize=5, align=4,
+// NEW-NEXT: | nvsize=5, nvalign=4]
+
+// OLD-NEXT: | [sizeof=8, dsize=8, align=4,
+// OLD-NEXT: | nvsize=8, nvalign=4]
+
+struct X : OversizedBase {
+ i1 in_padding;
+};
+static_assert(sizeof(X) == (OLD_ABI ? 12 : 8));
+// CHECK:*** Dumping AST Record Layout
+// CHECK: 0 | struct X
+// CHECK-NEXT: 0 | struct OversizedBase (base)
+// CHECK-NEXT: 0:0-32 | i4 i
+
+// NEW-NEXT: 5 | i1 in_padding
+// NEW-NEXT: | [sizeof=8, dsize=6, align=4,
+// NEW-NEXT: | nvsize=6, nvalign=4]
+
+// OLD-NEXT: 8 | i1 in_padding
+// OLD-NEXT: | [sizeof=12, dsize=9, align=4,
+// OLD-NEXT: | nvsize=9, nvalign=4]
+
+struct Y : OversizedBase {
+ i1 in_padding[3];
+};
+
+static_assert(sizeof(Y) == (OLD_ABI ? 12 : 8));
+// CHECK:*** Dumping AST Record Layout
+// CHECK: 0 | struct Y
+// CHECK-NEXT: 0 | struct OversizedBase (base)
+// CHECK-NEXT: 0:0-32 | i4 i
+
+// NEW-NEXT: 5 | i1[3] in_padding
+// NEW-NEXT: | [sizeof=8, dsize=8, align=4,
+// NEW-NEXT: | nvsize=8, nvalign=4]
+
+// OLD-NEXT: 8 | i1[3] in_padding
+// OLD-NEXT: | [sizeof=12, dsize=11, align=4,
+// OLD-NEXT: | nvsize=11, nvalign=4]
+
+struct Z : OversizedBase {
+ i1 in_padding[4];
+};
+
+static_assert(sizeof(Z) == 12);
+// CHECK:*** Dumping AST Record Layout
+// CHECK: 0 | struct Z
+// CHECK-NEXT: 0 | struct OversizedBase (base)
+// CHECK-NEXT: 0:0-32 | i4 i
+
+// NEW-NEXT: 5 | i1[4] in_padding
+// NEW-NEXT: | [sizeof=12, dsize=9, align=4,
+// NEW-NEXT: | nvsize=9, nvalign=4]
+
+// OLD-NEXT: 8 | i1[4] in_padding
+// OLD-NEXT: | [sizeof=12, dsize=12, align=4,
+// OLD-NEXT: | nvsize=12, nvalign=4]
+
+namespace BitInt {
+struct alignas(4) OversizedBase {
+ _BitInt(9) i : 10;
+};
+static_assert(sizeof(OversizedBase) == 4);
+// CHECK:*** Dumping AST Record Layout
+// CHECK: 0 | struct BitInt::OversizedBase
+// CHECK-NEXT: 0:0-9 | _BitInt(9) i
+
+// NEW-NEXT: | [sizeof=4, dsize=2, align=4,
+// NEW-NEXT: | nvsize=2, nvalign=4]
+
+// OLD-NEXT: | [sizeof=4, dsize=4, align=4,
+// OLD-NEXT: | nvsize=4, nvalign=4]
+
+struct X : OversizedBase {
+ i1 in_padding;
+};
+static_assert(sizeof(X) == (OLD_ABI ? 8 : 4));
+// CHECK:*** Dumping AST Record Layout
+// CHECK: 0 | struct BitInt::X
+// CHECK-NEXT: 0 | struct BitInt::OversizedBase (base)
+// CHECK-NEXT: 0:0-9 | _BitInt(9) i
+
+// NEW-NEXT: 2 | i1 in_padding
+// NEW-NEXT: | [sizeof=4, dsize=3, align=4,
+// NEW-NEXT: | nvsize=3, nvalign=4]
+
+// OLD-NEXT: 4 | i1 in_padding
+// OLD-NEXT: | [sizeof=8, dsize=5, align=4,
+// OLD-NEXT: | nvsize=5, nvalign=4]
+}
diff --git a/clang/test/Layout/no-unique-address.cpp
b/clang/test/Layout/no-unique-address.cpp
index 6f718b843daedb..897b47f37ce87f 100644
--- a/clang/test/Layout/no-unique-address.cpp
+++ b/clang/test/Layout/no-unique-address.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-linux-gnu
-fdump-record-layouts %s | FileCheck %s --check-prefix=NEW --check-prefix=CHECK
-// RUN: %clang_cc1 -fclang-abi-compat=19.0 -DOLD_ABI=1 -std=c++2a
-fsyntax-only -triple x86_64-linux-gnu -fdump-record-layouts %s | FileCheck %s
--check-prefix=OLD --check-prefix=CHECK
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -triple x86_64-linux-gnu
-fdump-record-layouts %s | FileCheck %s --check-prefixes=CHECK,NEW
+// RUN: %clang_cc1 -fclang-abi-compat=19.0 -std=c++20 -fsyntax-only -triple
x86_64-linux-gnu -fdump-record-layouts %s | FileCheck %s
--check-prefixes=CHECK,OLD
namespace Empty {
struct A {};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
