Skip to content

Commit 893b864

Browse files
committed
Revert "GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs"
This reverts commit 2771233. There are still some issues with this change to be worked out, so revert this patch to avoid having to change ABIs from LLVM 13->14 and then again from LLVM 14->15. See discussion in https://reviews.llvm.org/D118511
1 parent a9415df commit 893b864

File tree

5 files changed

+1
-57
lines changed

5 files changed

+1
-57
lines changed

clang/docs/ReleaseNotes.rst

-6
Original file line numberDiff line numberDiff line change
@@ -338,12 +338,6 @@ ABI Changes in Clang
338338
is still in the process of being stabilized, so this type should not yet be
339339
used in interfaces that require ABI stability.
340340

341-
- GCC doesn't pack non-POD members in packed structs unless the packed
342-
attribute is also specified on the member. Clang historically did perform
343-
such packing. Clang now matches the gcc behavior (except on Darwin and PS4).
344-
You can switch back to the old ABI behavior with the flag:
345-
``-fclang-abi-compat=13.0``.
346-
347341
OpenMP Support in Clang
348342
-----------------------
349343

clang/include/clang/Basic/LangOptions.h

-4
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,6 @@ class LangOptions : public LangOptionsBase {
181181
/// global-scope inline variables incorrectly.
182182
Ver12,
183183

184-
/// Attempt to be ABI-compatible with code generated by Clang 13.0.x.
185-
/// This causes clang to not pack non-POD members of packed structs.
186-
Ver13,
187-
188184
/// Conform to the underlying platform's C and C++ ABIs as closely
189185
/// as we can.
190186
Latest

clang/lib/AST/RecordLayoutBuilder.cpp

+1-6
Original file line numberDiff line numberDiff line change
@@ -1887,12 +1887,7 @@ void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
18871887
UnfilledBitsInLastUnit = 0;
18881888
LastBitfieldStorageUnitSize = 0;
18891889

1890-
llvm::Triple Target = Context.getTargetInfo().getTriple();
1891-
bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
1892-
Context.getLangOpts().getClangABICompat() <=
1893-
LangOptions::ClangABI::Ver13 ||
1894-
Target.isPS4() || Target.isOSDarwin())) ||
1895-
D->hasAttr<PackedAttr>();
1890+
bool FieldPacked = Packed || D->hasAttr<PackedAttr>();
18961891

18971892
AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
18981893
CharUnits FieldSize;

clang/lib/Frontend/CompilerInvocation.cpp

-4
Original file line numberDiff line numberDiff line change
@@ -3560,8 +3560,6 @@ void CompilerInvocation::GenerateLangArgs(const LangOptions &Opts,
35603560
GenerateArg(Args, OPT_fclang_abi_compat_EQ, "11.0", SA);
35613561
else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver12)
35623562
GenerateArg(Args, OPT_fclang_abi_compat_EQ, "12.0", SA);
3563-
else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver13)
3564-
GenerateArg(Args, OPT_fclang_abi_compat_EQ, "13.0", SA);
35653563

35663564
if (Opts.getSignReturnAddressScope() ==
35673565
LangOptions::SignReturnAddressScopeKind::All)
@@ -4064,8 +4062,6 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
40644062
Opts.setClangABICompat(LangOptions::ClangABI::Ver11);
40654063
else if (Major <= 12)
40664064
Opts.setClangABICompat(LangOptions::ClangABI::Ver12);
4067-
else if (Major <= 13)
4068-
Opts.setClangABICompat(LangOptions::ClangABI::Ver13);
40694065
} else if (Ver != "latest") {
40704066
Diags.Report(diag::err_drv_invalid_value)
40714067
<< A->getAsString(Args) << A->getValue();

clang/test/SemaCXX/class-layout.cpp

-37
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base
22
// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
3-
// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=13
4-
// RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6
53
// 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
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
74
// expected-no-diagnostics
85

96
#define SA(n, p) int a##n[(p) ? 1 : -1]
@@ -607,37 +604,3 @@ namespace PR37275 {
607604
#endif
608605
#pragma pack(pop)
609606
}
610-
611-
namespace non_pod {
612-
struct t1 {
613-
protected:
614-
int a;
615-
};
616-
// GCC prints warning: ignoring packed attribute because of unpacked non-POD field 't1 t2::v1'`
617-
struct t2 {
618-
char c1;
619-
short s1;
620-
char c2;
621-
t1 v1;
622-
} __attribute__((packed));
623-
#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 13
624-
_Static_assert(_Alignof(t1) == 4, "");
625-
_Static_assert(_Alignof(t2) == 1, "");
626-
#else
627-
_Static_assert(_Alignof(t1) == 4, "");
628-
_Static_assert(_Alignof(t2) == 4, "");
629-
#endif
630-
_Static_assert(sizeof(t2) == 8, ""); // it's still packing the rest of the struct
631-
} // namespace non_pod
632-
633-
namespace non_pod_packed {
634-
struct t1 {
635-
protected:
636-
int a;
637-
} __attribute__((packed));
638-
struct t2 {
639-
t1 v1;
640-
} __attribute__((packed));
641-
_Static_assert(_Alignof(t1) == 1, "");
642-
_Static_assert(_Alignof(t2) == 1, "");
643-
} // namespace non_pod_packed

0 commit comments

Comments
 (0)