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
This commit is contained in:
David Blaikie 2022-01-18 14:28:14 -08:00
parent 422084332a
commit 277123376c
5 changed files with 57 additions and 1 deletions

View File

@ -236,6 +236,12 @@ ABI Changes in Clang
is still in the process of being stabilized, so this type should not yet be is still in the process of being stabilized, so this type should not yet be
used in interfaces that require ABI stability. 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 OpenMP Support in Clang
----------------------- -----------------------

View File

@ -181,6 +181,10 @@ public:
/// global-scope inline variables incorrectly. /// global-scope inline variables incorrectly.
Ver12, 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 /// Conform to the underlying platform's C and C++ ABIs as closely
/// as we can. /// as we can.
Latest Latest

View File

@ -1887,7 +1887,12 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
UnfilledBitsInLastUnit = 0; UnfilledBitsInLastUnit = 0;
LastBitfieldStorageUnitSize = 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; AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
CharUnits FieldSize; CharUnits FieldSize;

View File

@ -3560,6 +3560,8 @@ void CompilerInvocation::GenerateLangArgs(const LangOptions &Opts,
GenerateArg(Args, OPT_fclang_abi_compat_EQ, "11.0", SA); GenerateArg(Args, OPT_fclang_abi_compat_EQ, "11.0", SA);
else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver12) else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver12)
GenerateArg(Args, OPT_fclang_abi_compat_EQ, "12.0", SA); 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() == if (Opts.getSignReturnAddressScope() ==
LangOptions::SignReturnAddressScopeKind::All) LangOptions::SignReturnAddressScopeKind::All)
@ -4062,6 +4064,8 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
Opts.setClangABICompat(LangOptions::ClangABI::Ver11); Opts.setClangABICompat(LangOptions::ClangABI::Ver11);
else if (Major <= 12) else if (Major <= 12)
Opts.setClangABICompat(LangOptions::ClangABI::Ver12); Opts.setClangABICompat(LangOptions::ClangABI::Ver12);
else if (Major <= 13)
Opts.setClangABICompat(LangOptions::ClangABI::Ver13);
} else if (Ver != "latest") { } else if (Ver != "latest") {
Diags.Report(diag::err_drv_invalid_value) Diags.Report(diag::err_drv_invalid_value)
<< A->getAsString(Args) << A->getValue(); << A->getAsString(Args) << A->getValue();

View File

@ -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++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-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=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 // expected-no-diagnostics
#define SA(n, p) int a##n[(p) ? 1 : -1] #define SA(n, p) int a##n[(p) ? 1 : -1]
@ -604,3 +607,37 @@ namespace PR37275 {
#endif #endif
#pragma pack(pop) #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