[clang] 2771233 - GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 28 11:04:28 PST 2022
Author: David Blaikie
Date: 2022-01-28T11:04:20-08:00
New Revision: 277123376ce08c98b07c154bf83e4092a5d4d3c6
URL: https://github.com/llvm/llvm-project/commit/277123376ce08c98b07c154bf83e4092a5d4d3c6
DIFF: https://github.com/llvm/llvm-project/commit/277123376ce08c98b07c154bf83e4092a5d4d3c6.diff
LOG: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs
This matches GCC: https://godbolt.org/z/sM5q95PGY
I realize this is an API break for clang+clang - so I'm totally open to
discussing how we should deal with that. If Apple wants to keep the
Clang layout indefinitely, if we want to put a flag on this so non-Apple
folks can opt out of this fix/new behavior.
Differential Revision: https://reviews.llvm.org/D117616
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/LangOptions.h
clang/lib/AST/RecordLayoutBuilder.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/SemaCXX/class-layout.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6a9b046a1427d..45e80785e462a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -236,6 +236,12 @@ ABI Changes in Clang
is still in the process of being stabilized, so this type should not yet be
used in interfaces that require ABI stability.
+- GCC doesn't pack non-POD members in packed structs unless the packed
+ attribute is also specified on the member. Clang historically did perform
+ such packing. Clang now matches the gcc behavior (except on Darwin and PS4).
+ You can switch back to the old ABI behavior with the flag:
+ ``-fclang-abi-compat=13.0``.
+
OpenMP Support in Clang
-----------------------
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 09afa641acf9b..50c7f038fc6be 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -181,6 +181,10 @@ class LangOptions : public LangOptionsBase {
/// global-scope inline variables incorrectly.
Ver12,
+ /// Attempt to be ABI-compatible with code generated by Clang 13.0.x.
+ /// This causes clang to not pack non-POD members of packed structs.
+ Ver13,
+
/// 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 61a30ead165ef..709e05716a562 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1887,7 +1887,12 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
UnfilledBitsInLastUnit = 0;
LastBitfieldStorageUnitSize = 0;
- bool FieldPacked = Packed || D->hasAttr<PackedAttr>();
+ llvm::Triple Target = Context.getTargetInfo().getTriple();
+ bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver13 ||
+ Target.isPS4() || Target.isOSDarwin())) ||
+ D->hasAttr<PackedAttr>();
AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
CharUnits FieldSize;
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 7f1ce3da7e7eb..553a0b31c0ab3 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3560,6 +3560,8 @@ void CompilerInvocation::GenerateLangArgs(const LangOptions &Opts,
GenerateArg(Args, OPT_fclang_abi_compat_EQ, "11.0", SA);
else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver12)
GenerateArg(Args, OPT_fclang_abi_compat_EQ, "12.0", SA);
+ else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver13)
+ GenerateArg(Args, OPT_fclang_abi_compat_EQ, "13.0", SA);
if (Opts.getSignReturnAddressScope() ==
LangOptions::SignReturnAddressScopeKind::All)
@@ -4062,6 +4064,8 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
Opts.setClangABICompat(LangOptions::ClangABI::Ver11);
else if (Major <= 12)
Opts.setClangABICompat(LangOptions::ClangABI::Ver12);
+ else if (Major <= 13)
+ Opts.setClangABICompat(LangOptions::ClangABI::Ver13);
} else if (Ver != "latest") {
Diags.Report(diag::err_drv_invalid_value)
<< A->getAsString(Args) << A->getValue();
diff --git a/clang/test/SemaCXX/class-layout.cpp b/clang/test/SemaCXX/class-layout.cpp
index 5403bd6e6a6f6..79fa677071109 100644
--- a/clang/test/SemaCXX/class-layout.cpp
+++ b/clang/test/SemaCXX/class-layout.cpp
@@ -1,6 +1,9 @@
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
+// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=13
+// RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=13 -DCLANG_ABI_COMPAT=13
// expected-no-diagnostics
#define SA(n, p) int a##n[(p) ? 1 : -1]
@@ -604,3 +607,37 @@ namespace PR37275 {
#endif
#pragma pack(pop)
}
+
+namespace non_pod {
+struct t1 {
+protected:
+ int a;
+};
+// GCC prints warning: ignoring packed attribute because of unpacked non-POD field 't1 t2::v1'`
+struct t2 {
+ char c1;
+ short s1;
+ char c2;
+ t1 v1;
+} __attribute__((packed));
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 13
+_Static_assert(_Alignof(t1) == 4, "");
+_Static_assert(_Alignof(t2) == 1, "");
+#else
+_Static_assert(_Alignof(t1) == 4, "");
+_Static_assert(_Alignof(t2) == 4, "");
+#endif
+_Static_assert(sizeof(t2) == 8, ""); // it's still packing the rest of the struct
+} // namespace non_pod
+
+namespace non_pod_packed {
+struct t1 {
+protected:
+ int a;
+} __attribute__((packed));
+struct t2 {
+ t1 v1;
+} __attribute__((packed));
+_Static_assert(_Alignof(t1) == 1, "");
+_Static_assert(_Alignof(t2) == 1, "");
+} // namespace non_pod_packed
More information about the cfe-commits
mailing list