Author: Dmitri Gribenko Date: 2023-08-07T20:30:48+02:00 New Revision: bddaa35177861545fc329123e55b6a3b34549507
URL: https://github.com/llvm/llvm-project/commit/bddaa35177861545fc329123e55b6a3b34549507 DIFF: https://github.com/llvm/llvm-project/commit/bddaa35177861545fc329123e55b6a3b34549507.diff LOG: Anonymous unions should be transparent wrt `[[clang::trivial_abi]]`. Anonymous unions should be transparent wrt `[[clang::trivial_abi]]`. Consider the test input below: ``` struct [[clang::trivial_abi]] Trivial { Trivial() {} Trivial(Trivial&& other) {} Trivial& operator=(Trivial&& other) { return *this; } ~Trivial() {} }; static_assert(__is_trivially_relocatable(Trivial), ""); struct [[clang::trivial_abi]] S2 { S2(S2&& other) {} S2& operator=(S2&& other) { return *this; } ~S2() {} union { Trivial field; }; }; static_assert(__is_trivially_relocatable(S2), ""); ``` Before the fix Clang would warn that 'trivial_abi' is disallowed on 'S2' because it has a field of a non-trivial class type (the type of the anonymous union is non-trivial, because it doesn't have the `[[clang::trivial_abi]]` attribute applied to it). Consequently, before the fix the `static_assert` about `__is_trivially_relocatable` would fail. Note that `[[clang::trivial_abi]]` cannot be applied to the anonymous union, because Clang warns that 'trivial_abi' is disallowed on '(unnamed union at ...)' because its copy constructors and move constructors are all deleted. Also note that it is impossible to provide copy nor move constructors for anonymous unions and structs. Reviewed By: gribozavr2 Differential Revision: https://reviews.llvm.org/D155895 Added: Modified: clang/include/clang/Basic/LangOptions.h clang/lib/Frontend/CompilerInvocation.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/test/SemaCXX/attr-trivial-abi.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 3ef68ca8af6685..d926cfcd4d8665 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -230,6 +230,12 @@ class LangOptions : public LangOptionsBase { /// - consider classes with defaulted special member functions non-pod. Ver15, + /// Attempt to be ABI-compatible with code generated by Clang 17.0.x. + /// This causes clang to: + /// - Treat `[[clang::trivial_abi]]` as ill-formed and ignore it if any + /// field is an anonymous union and/or struct. + Ver17, + /// Conform to the underlying platform's C and C++ ABIs as closely /// as we can. Latest diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index be53ce3e472659..699b315269deb3 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -3474,6 +3474,8 @@ void CompilerInvocation::GenerateLangArgs(const LangOptions &Opts, GenerateArg(Consumer, OPT_fclang_abi_compat_EQ, "14.0"); else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver15) GenerateArg(Consumer, OPT_fclang_abi_compat_EQ, "15.0"); + else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver17) + GenerateArg(Consumer, OPT_fclang_abi_compat_EQ, "17.0"); if (Opts.getSignReturnAddressScope() == LangOptions::SignReturnAddressScopeKind::All) @@ -3959,6 +3961,8 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args, Opts.setClangABICompat(LangOptions::ClangABI::Ver14); else if (Major <= 15) Opts.setClangABICompat(LangOptions::ClangABI::Ver15); + else if (Major <= 17) + Opts.setClangABICompat(LangOptions::ClangABI::Ver17); } else if (Ver != "latest") { Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << A->getValue(); diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 2c2c9dc8147437..6c29efc03a4b78 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -45,6 +45,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/Support/SaveAndRestore.h" #include <map> @@ -10413,21 +10414,35 @@ void Sema::checkIllFormedTrivialABIStruct(CXXRecordDecl &RD) { } } - for (const auto *FD : RD.fields()) { - // Ill-formed if the field is an ObjectiveC pointer or of a type that is - // non-trivial for the purpose of calls. + llvm::SmallVector<const FieldDecl *> FieldsToCheck{RD.fields()}; + while (!FieldsToCheck.empty()) { + const FieldDecl *FD = FieldsToCheck.pop_back_val(); + + // Ill-formed if the field is an ObjectiveC pointer. QualType FT = FD->getType(); if (FT.getObjCLifetime() == Qualifiers::OCL_Weak) { PrintDiagAndRemoveAttr(4); return; } - if (const auto *RT = FT->getBaseElementTypeUnsafe()->getAs<RecordType>()) + if (const auto *RT = FT->getBaseElementTypeUnsafe()->getAs<RecordType>()) { + // Check the fields of anonymous structs (and/or or unions) instead of + // checking the type of the anonynous struct itself. + if (getLangOpts().getClangABICompat() > LangOptions::ClangABI::Ver17 && + RT->getDecl()->isAnonymousStructOrUnion()) { + FieldsToCheck.append(RT->getDecl()->field_begin(), + RT->getDecl()->field_end()); + continue; + } + + // Ill-formed if the field is of a type that is non-trivial for the + // purpose of calls. if (!RT->isDependentType() && !cast<CXXRecordDecl>(RT->getDecl())->canPassInRegisters()) { PrintDiagAndRemoveAttr(5); return; } + } } } diff --git a/clang/test/SemaCXX/attr-trivial-abi.cpp b/clang/test/SemaCXX/attr-trivial-abi.cpp index c215f90eb124ce..accac42c25cf5f 100644 --- a/clang/test/SemaCXX/attr-trivial-abi.cpp +++ b/clang/test/SemaCXX/attr-trivial-abi.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 +// RUN: %clang_cc1 -fsyntax-only -fclang-abi-compat=17 -verify %s -std=c++11 -DCLANG_ABI_COMPAT=17 void __attribute__((trivial_abi)) foo(); // expected-warning {{'trivial_abi' attribute only applies to classes}} @@ -169,3 +170,115 @@ static_assert(!__is_trivially_relocatable(S20), ""); static_assert(__is_trivially_relocatable(S20), ""); #endif } // namespace deletedCopyMoveConstructor + +namespace anonymousUnionsAndStructs { + // Test helper: + struct [[clang::trivial_abi]] Trivial { + Trivial() {} + Trivial(Trivial&& other) {} + Trivial& operator=(Trivial&& other) { return *this; } + ~Trivial() {} + }; + static_assert(__is_trivially_relocatable(Trivial), ""); + + // Test helper: + struct Nontrivial { + Nontrivial() {} + Nontrivial(Nontrivial&& other) {} + Nontrivial& operator=(Nontrivial&& other) { return *this; } + ~Nontrivial() {} + }; + static_assert(!__is_trivially_relocatable(Nontrivial), ""); + + // Basic smoke test, not yet related to anonymous unions or structs: + struct [[clang::trivial_abi]] BasicStruct { + BasicStruct(BasicStruct&& other) {} + BasicStruct& operator=(BasicStruct&& other) { return *this; } + ~BasicStruct() {} + Trivial field; + }; + static_assert(__is_trivially_relocatable(BasicStruct), ""); + + // `StructWithAnonymousUnion` is like `BasicStruct`, but `field` is wrapped in + // an anonymous union, and thus trivial relocatability of `BasicStruct` and + // `StructWithAnonymousUnion` should be the same). + // + // It's impossible to declare a constructor for an anonymous unions so to + // support applying `[[clang::trivial_abi]]` to structs containing anonymous + // unions, and therefore when processing fields of the struct containing the + // anonymous union, the trivial relocatability of the *union* is ignored and + // instead the union's fields are recursively inspected in + // `checkIllFormedTrivialABIStruct`. + struct [[clang::trivial_abi]] StructWithAnonymousUnion { +#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17 + // expected-warning@-2 {{'trivial_abi' cannot be applied to 'StructWithAnonymousUnion'}} + // expected-note@-3 {{trivial_abi' is disallowed on 'StructWithAnonymousUnion' because it has a field of a non-trivial class type}} +#endif + StructWithAnonymousUnion(StructWithAnonymousUnion&& other) {} + StructWithAnonymousUnion& operator=(StructWithAnonymousUnion&& other) { return *this; } + ~StructWithAnonymousUnion() {} + union { Trivial field; }; + }; +#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17 + static_assert(!__is_trivially_relocatable(StructWithAnonymousUnion), ""); +#else + static_assert(__is_trivially_relocatable(StructWithAnonymousUnion), ""); +#endif + + // `StructWithAnonymousStruct` is like `StructWithAnonymousUnion` but uses an + // anonymous `struct` rather than an anonymous `union. The same expectations + // can be applied to CLANG_ABI_COMPAT <= 17 and 18+, because the anonymous + // `struct` does have move constructors in the test below (unlike the + // anonymous `union` in the previous `StructWithAnonymousUnion` test). + struct [[clang::trivial_abi]] StructWithAnonymousStruct { + StructWithAnonymousStruct(StructWithAnonymousStruct&& other) {} + StructWithAnonymousStruct& operator=(StructWithAnonymousStruct&& other) { return *this; } + ~StructWithAnonymousStruct() {} + struct { Trivial field; }; + }; + static_assert(__is_trivially_relocatable(StructWithAnonymousStruct), ""); + + // `TrivialAbiAttributeAppliedToAnonymousUnion` is like + // `StructWithAnonymousUnion` but with `[[clang::trivial_abi]]` also applied + // to the anonymous union. + // + // The example below shows that it is still *not* okay to explicitly apply + // `[[clang::trivial_abi]]` to anonymous unions. Handling this would require + // relaxing the `HasNonDeletedCopyOrMoveConstructor` check when + // `isAnonymousStructOrUnion` in `checkIllFormedTrivialABIStruct` but when + // that check runs `setAnonymousStructOrUnion` hasn't been called yet (i.e. at + // this point it's not possible to rely on `RD->isAnonymousStructOrUnion()`). + struct [[clang::trivial_abi]] TrivialAbiAttributeAppliedToAnonymousUnion { +#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17 + // expected-warning@-2 {{'trivial_abi' cannot be applied to 'TrivialAbiAttributeAppliedToAnonymousUnion'}} + // expected-note@-3 {{trivial_abi' is disallowed on 'TrivialAbiAttributeAppliedToAnonymousUnion' because it has a field of a non-trivial class type}} +#endif + TrivialAbiAttributeAppliedToAnonymousUnion(TrivialAbiAttributeAppliedToAnonymousUnion&& other) {} + TrivialAbiAttributeAppliedToAnonymousUnion& operator=(TrivialAbiAttributeAppliedToAnonymousUnion&& other) { return *this; } + ~TrivialAbiAttributeAppliedToAnonymousUnion() {} + union [[clang::trivial_abi]] { // expected-warning {{'trivial_abi' cannot be applied to '(unnamed union}} expected-note {{copy constructors and move constructors are all deleted}} + Trivial field; + }; + }; +#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 17 + static_assert(!__is_trivially_relocatable(TrivialAbiAttributeAppliedToAnonymousUnion), ""); +#else + static_assert(__is_trivially_relocatable(TrivialAbiAttributeAppliedToAnonymousUnion), ""); +#endif + + // Like `StructWithAnonymousUnion`, but the field of the anonymous union is + // *not* trivial. + struct [[clang::trivial_abi]] StructWithAnonymousUnionWithNonTrivialField { + // expected-warning@-1 {{'trivial_abi' cannot be applied to 'StructWithAnonymousUnionWithNonTrivialField'}} + // expected-note@-2 {{trivial_abi' is disallowed on 'StructWithAnonymousUnionWithNonTrivialField' because it has a field of a non-trivial class type}} + StructWithAnonymousUnionWithNonTrivialField(StructWithAnonymousUnionWithNonTrivialField&& other) {} + StructWithAnonymousUnionWithNonTrivialField& operator=(StructWithAnonymousUnionWithNonTrivialField&& other) { return *this; } + ~StructWithAnonymousUnionWithNonTrivialField() {} + union { + Nontrivial field; + }; + }; + static_assert(!__is_trivially_relocatable(StructWithAnonymousUnionWithNonTrivialField), ""); + +} // namespace anonymousStructsAndUnions + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
