https://github.com/DanShaders updated https://github.com/llvm/llvm-project/pull/71148
>From 19124226ee4d08a8025f1e6b61d750096e13ee75 Mon Sep 17 00:00:00 2001 From: Dan Klishch <danilklis...@gmail.com> Date: Fri, 3 Nov 2023 21:18:06 -0400 Subject: [PATCH] [clang] Stub out gcc_struct attribute This commit implements gcc_struct attribute with the behavior similar to one in GCC. Current behavior is as follows: When C++ ABI is not "Microsoft" (i. e. when ItaniumRecordLayoutBuilder is used), [[gcc_struct]] will locally cancel the effect of -mms-bitfields on a record. If -mms-bitfields is not supplied and is not a default behavior on a target, [[gcc_struct]] will be a no-op. This should provide enough compatibility with GCC. If C++ ABI is "Microsoft", [[gcc_struct]] will currently always be a no-op, since support for it is not yet implemented in MicrosoftRecordLayoutBuilder. Note, however, that all the infrastructure is ready for the future implementation. In particular, check for default value of -mms-bitfield is moved from a driver to TargetInfo, since it now non-trivially depends on other supplied flags. Also, unfortunately, this makes it impossible to use usual argument parsing for `-m{no-,}ms-bitfield`. The patch doesn't introduce any not backwards-compatible changes, except for situations when cc1 is called directly on a target with -mms-bitfields being a default option. --- clang/include/clang/Basic/Attr.td | 7 +++++ clang/include/clang/Basic/LangOptions.def | 1 - clang/include/clang/Basic/LangOptions.h | 2 ++ clang/include/clang/Basic/TargetInfo.h | 4 +++ clang/include/clang/Driver/Options.td | 4 +-- clang/lib/AST/Decl.cpp | 7 ++++- clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 5 +--- clang/lib/Driver/ToolChains/Clang.cpp | 10 ++++--- clang/lib/Frontend/CompilerInvocation.cpp | 12 +++++++++ clang/lib/Sema/SemaDecl.cpp | 4 +-- clang/lib/Sema/SemaDeclCXX.cpp | 27 ++++++++++++------- clang/test/Driver/ms-bitfields.c | 11 +++++--- clang/test/Layout/itanium-union-bitfield.cpp | 2 +- ...a-attribute-supported-attributes-list.test | 1 + 14 files changed, 69 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 60b549999c155e5..34ec6c22481837e 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3635,6 +3635,13 @@ def MSStruct : InheritableAttr { let SimpleHandler = 1; } +def GCCStruct : InheritableAttr { + let Spellings = [GCC<"gcc_struct">]; + let Subjects = SubjectList<[Record]>; + let Documentation = [Undocumented]; + let SimpleHandler = 1; +} + def DLLExport : InheritableAttr, TargetSpecificAttr<TargetHasDLLImportExport> { let Spellings = [Declspec<"dllexport">, GCC<"dllexport">]; let Subjects = SubjectList<[Function, Var, CXXRecord, ObjCInterface]>; diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index c0ea4ecb9806a5b..afbb3c724a8d193 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -148,7 +148,6 @@ LANGOPT(ExternCNoUnwind , 1, 0, "Assume extern C functions don't unwind") LANGOPT(TraditionalCPP , 1, 0, "traditional CPP emulation") LANGOPT(RTTI , 1, 1, "run-time type information") LANGOPT(RTTIData , 1, 1, "emit run-time type information data") -LANGOPT(MSBitfields , 1, 0, "Microsoft-compatible structure layout") LANGOPT(MSVolatile , 1, 0, "Microsoft-compatible volatile loads and stores") LANGOPT(Freestanding, 1, 0, "freestanding implementation") LANGOPT(NoBuiltin , 1, 0, "disable builtin functions") diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 20a8ada60e0fe51..07eae8dad61813c 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -502,6 +502,8 @@ class LangOptions : public LangOptionsBase { // received as a result of a standard operator new (-fcheck-new) bool CheckNew = false; + std::optional<bool> MSBitfields; + LangOptions(); /// Set language defaults for the given input language and diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index b3c5cbfb319f01f..6865b6e23bc9628 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/include/clang/Basic/TargetInfo.h @@ -1724,6 +1724,10 @@ class TargetInfo : public TransferrableTargetInfo, /// Whether to support HIP image/texture API's. virtual bool hasHIPImageSupport() const { return true; } + virtual bool defaultsToMsStruct() const { + return getCXXABI().isMicrosoft() || getTriple().isWindowsGNUEnvironment(); + } + protected: /// Copy type and layout related info. void copyAuxTarget(const TargetInfo *Aux); diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 9c0dcaa8b3f6276..d29214ed3cb13db 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -4381,8 +4381,7 @@ def : Joined<["-"], "mmacosx-version-min=">, Group<m_Group>, Alias<mmacos_version_min_EQ>; def mms_bitfields : Flag<["-"], "mms-bitfields">, Group<m_Group>, Visibility<[ClangOption, CC1Option]>, - HelpText<"Set the default structure layout to be compatible with the Microsoft compiler standard">, - MarshallingInfoFlag<LangOpts<"MSBitfields">>; + HelpText<"Set the default structure layout to be compatible with the Microsoft compiler standard">; def moutline : Flag<["-"], "moutline">, Group<f_clang_Group>, Visibility<[ClangOption, CC1Option]>, HelpText<"Enable function outlining (AArch64 only)">; @@ -4390,6 +4389,7 @@ def mno_outline : Flag<["-"], "mno-outline">, Group<f_clang_Group>, Visibility<[ClangOption, CC1Option]>, HelpText<"Disable function outlining (AArch64 only)">; def mno_ms_bitfields : Flag<["-"], "mno-ms-bitfields">, Group<m_Group>, + Visibility<[ClangOption, CC1Option]>, HelpText<"Do not set the default structure layout to be compatible with the Microsoft compiler standard">; def mskip_rax_setup : Flag<["-"], "mskip-rax-setup">, Group<m_Group>, Visibility<[ClangOption, CC1Option]>, diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index ed848e574233468..a0817fab9f7dcf8 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -5031,7 +5031,12 @@ void RecordDecl::completeDefinition() { /// This which can be turned on with an attribute, pragma, or the /// -mms-bitfields command-line option. bool RecordDecl::isMsStruct(const ASTContext &C) const { - return hasAttr<MSStructAttr>() || C.getLangOpts().MSBitfields == 1; + if (hasAttr<MSStructAttr>()) + return true; + if (hasAttr<GCCStructAttr>()) + return false; + return C.getLangOpts().MSBitfields.value_or( + C.getTargetInfo().defaultsToMsStruct()); } void RecordDecl::reorderDecls(const SmallVectorImpl<Decl *> &Decls) { diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp index cbfa79e10bfefcc..296151c32c91c12 100644 --- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp +++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp @@ -104,10 +104,7 @@ struct CGRecordLowering { /// fields of the same formal type. We want to emit a layout with /// these discrete storage units instead of combining them into a /// continuous run. - bool isDiscreteBitFieldABI() { - return Context.getTargetInfo().getCXXABI().isMicrosoft() || - D->isMsStruct(Context); - } + bool isDiscreteBitFieldABI() { return D->isMsStruct(Context); } /// Helper function to check if we are targeting AAPCS. bool isAAPCS() const { diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 79f7fba22570746..2602d1f6bedee9f 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -5645,9 +5645,13 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, if (KernelOrKext && RawTriple.isOSDarwin()) CmdArgs.push_back("-fforbid-guard-variables"); - if (Args.hasFlag(options::OPT_mms_bitfields, options::OPT_mno_ms_bitfields, - Triple.isWindowsGNUEnvironment())) { - CmdArgs.push_back("-mms-bitfields"); + if (Args.hasArg(options::OPT_mms_bitfields) || + Args.hasArg(options::OPT_mno_ms_bitfields)) { + if (Args.hasFlag(options::OPT_mms_bitfields, options::OPT_mno_ms_bitfields, + false)) + CmdArgs.push_back("-mms-bitfields"); + else + CmdArgs.push_back("-mno-ms-bitfields"); } if (Triple.isWindowsGNUEnvironment()) { diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 637c6a35af6532b..a0576441c5d24a7 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -3619,6 +3619,13 @@ void CompilerInvocationBase::GenerateLangArgs(const LangOptions &Opts, GenerateArg(Consumer, OPT_fcxx_abi_EQ, TargetCXXABI::getSpelling(*Opts.CXXABI)); + if (Opts.MSBitfields.has_value()) { + if (Opts.MSBitfields.value()) + GenerateArg(Consumer, OPT_mms_bitfields); + else + GenerateArg(Consumer, OPT_mno_ms_bitfields); + } + if (Opts.RelativeCXXABIVTables) GenerateArg(Consumer, OPT_fexperimental_relative_cxx_abi_vtables); else @@ -4153,6 +4160,11 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args, options::OPT_fno_experimental_relative_cxx_abi_vtables, TargetCXXABI::usesRelativeVTables(T)); + if (Args.hasArg(options::OPT_mms_bitfields) || + Args.hasArg(options::OPT_mno_ms_bitfields)) + Opts.MSBitfields = Args.hasFlag(options::OPT_mms_bitfields, + options::OPT_mno_ms_bitfields, false); + // RTTI is on by default. bool HasRTTI = Args.hasFlag(options::OPT_frtti, options::OPT_fno_rtti, true); Opts.OmitVTableRTTI = diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 396566a8f10a9b7..27fb20eb8181b62 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -18199,9 +18199,7 @@ ExprResult Sema::VerifyBitField(SourceLocation FieldLoc, // ABI. bool CStdConstraintViolation = BitfieldIsOverwide && !getLangOpts().CPlusPlus; - bool MSBitfieldViolation = - Value.ugt(TypeStorageSize) && - (IsMsStruct || Context.getTargetInfo().getCXXABI().isMicrosoft()); + bool MSBitfieldViolation = Value.ugt(TypeStorageSize) && IsMsStruct; if (CStdConstraintViolation || MSBitfieldViolation) { unsigned DiagWidth = CStdConstraintViolation ? TypeWidth : TypeStorageSize; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 397b7a00e453126..09e5768deefdc57 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -7255,20 +7255,27 @@ void Sema::CheckCompletedCXXClass(Scope *S, CXXRecordDecl *Record) { CheckCompletedMemberFunction(MD); } - // ms_struct is a request to use the same ABI rules as MSVC. Check - // whether this class uses any C++ features that are implemented - // completely differently in MSVC, and if so, emit a diagnostic. - // That diagnostic defaults to an error, but we allow projects to - // map it down to a warning (or ignore it). It's a fairly common - // practice among users of the ms_struct pragma to mass-annotate - // headers, sweeping up a bunch of types that the project doesn't - // really rely on MSVC-compatible layout for. We must therefore - // support "ms_struct except for C++ stuff" as a secondary ABI. + // {ms,gcc}_struct is a request to change ABI rules to either follow + // Microsoft or Itanium C++ ABI. However, even if these attributes are + // present, we do not layout classes following foreign ABI rules, but + // instead enter a special "compatibility mode", which only changes + // alignements of fundamental types and layout of bit fields. + // Check whether this class uses any C++ features that are implemented + // completely differently in the requested ABI, and if so, emit a + // diagnostic. That diagnostic defaults to an error, but we allow + // projects to map it down to a warning (or ignore it). It's a fairly + // common practice among users of the ms_struct pragma to + // mass-annotate headers, sweeping up a bunch of types that the + // project doesn't really rely on MSVC-compatible layout for. We must + // therefore support "ms_struct except for C++ stuff" as a secondary + // ABI. // Don't emit this diagnostic if the feature was enabled as a // language option (as opposed to via a pragma or attribute), as // the option -mms-bitfields otherwise essentially makes it impossible // to build C++ code, unless this diagnostic is turned off. - if (Record->isMsStruct(Context) && !Context.getLangOpts().MSBitfields && + if (!Context.getLangOpts().MSBitfields.has_value() && + Record->isMsStruct(Context) != + Context.getTargetInfo().defaultsToMsStruct() && (Record->isPolymorphic() || Record->getNumBases())) { Diag(Record->getLocation(), diag::warn_cxx_ms_struct); } diff --git a/clang/test/Driver/ms-bitfields.c b/clang/test/Driver/ms-bitfields.c index 031ed41e2aad678..4cd3f1380e41acf 100644 --- a/clang/test/Driver/ms-bitfields.c +++ b/clang/test/Driver/ms-bitfields.c @@ -1,8 +1,13 @@ -// RUN: %clang -### -target x86_64-linux-gnu %s 2>&1 | FileCheck %s -check-prefix=NO-MSBITFIELDS -// RUN: %clang -### -target x86_64-windows-gnu %s 2>&1 | FileCheck %s -check-prefix=MSBITFIELDS +// RUN: %clang -### -target x86_64-linux-gnu %s 2>&1 | FileCheck %s -check-prefix=DEFAULT-MSBITFIELDS +// RUN: %clang -### -target x86_64-windows-gnu %s 2>&1 | FileCheck %s -check-prefix=DEFAULT-MSBITFIELDS +// RUN: %clang -### -target x86_64-windows-msvc %s 2>&1 | FileCheck %s -check-prefix=DEFAULT-MSBITFIELDS +// RUN: %clang -### -mms-bitfields %s 2>&1 | FileCheck %s -check-prefix=MSBITFIELDS +// RUN: %clang -### -mno-ms-bitfields %s 2>&1 | FileCheck %s -check-prefix=NO-MSBITFIELDS // RUN: %clang -### -mno-ms-bitfields -mms-bitfields %s 2>&1 | FileCheck %s -check-prefix=MSBITFIELDS // RUN: %clang -### -mms-bitfields -mno-ms-bitfields %s 2>&1 | FileCheck %s -check-prefix=NO-MSBITFIELDS // MSBITFIELDS: -mms-bitfields -// NO-MSBITFIELDS-NOT: -mms-bitfields +// NO-MSBITFIELDS: -mno-ms-bitfields +// DEFAULT-MSBITFIELDS-NOT: -mms-bitfields +// DEFAULT-MSBITFIELDS-NOT: -mno-ms-bitfields diff --git a/clang/test/Layout/itanium-union-bitfield.cpp b/clang/test/Layout/itanium-union-bitfield.cpp index febfc46dfee54c1..3e22fe9a3faabd3 100644 --- a/clang/test/Layout/itanium-union-bitfield.cpp +++ b/clang/test/Layout/itanium-union-bitfield.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -emit-llvm-only -triple %itanium_abi_triple -fdump-record-layouts %s 2>/dev/null \ +// RUN: %clang_cc1 -emit-llvm-only -triple %itanium_abi_triple -mno-ms-bitfields -fdump-record-layouts %s 2>/dev/null \ // RUN: | FileCheck %s // On z/OS, a bit-field has single byte alignment. Add aligned(4) on z/OS so the union has diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index f48126775c8685c..eb237f7ebb1b2e8 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -73,6 +73,7 @@ // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum) // CHECK-NEXT: Flatten (SubjectMatchRule_function) // CHECK-NEXT: FunctionReturnThunks (SubjectMatchRule_function) +// CHECK-NEXT: GCCStruct (SubjectMatchRule_record) // CHECK-NEXT: GNUInline (SubjectMatchRule_function) // CHECK-NEXT: HIPManaged (SubjectMatchRule_variable) // CHECK-NEXT: Hot (SubjectMatchRule_function) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits