[clang] Override the Fuchsia platform ABI using the triple environment

[leonardchan]

Summary

This is an alternative to D85802 which added a flag for selecting between C++ ABIs at compile-time. This instead allows for overriding the platform ABI on a Fuchsia system, rather than overriding the C++ ABI.

This will be used as a way to integrate code built with Clang with code built by other compilers that don't contain Fuchsia-specific ABI changes. The immediate use case is for generating multilibs that use the generic Itanium ABI to be used by code built by GCC.

[rnk]

Generally makes sense, but I had a concern.

clang/include/clang/Basic/LangOptions.h

333

Why isn't this part of the LangOptions.def xmacro list?

[phosek]

I'd prefer to use the target triple rather than introducing a custom flag.

With dedicated flags, you might eventually end up in a similar situation as D85802, that is in the extreme case you might end up with -f[no-]fuchsia-c++-abi, -f[no-]webassembly-c++-abi, etc. which is not any better than -fc++-abi=.

With target triple, I can imagine using either <arch>-unknown-fuchsia-itanium or <arch>-unknown-fuchsia-gnu, where the former would mean targeting Fuchsia with Itanium C++ ABI while the latter would mean using GCC compatible ABI (which would imply Itanium C++ ABI). Both of these are already used by MinGW for the same purpose so there's a precedent and we don't need to invent anything new.

[rnk]

I guess a triple of -fuchsia-itanium would be a reasonable way of expressing this.

Why would we want a feature flag for the wasm C++ ABI? Is there a use case for using the webassembly C++ ABI on non-wasm ISAs?

[phosek]

In D93668#2482995, @rnk wrote:

I guess a triple of -fuchsia-itanium would be a reasonable way of expressing this.

Why would we want a feature flag for the wasm C++ ABI? Is there a use case for using the webassembly C++ ABI on non-wasm ISAs?

We've been considering the use of WASM as a binary format in Fuchsia at which point we'd need to decide which C++ ABI to use in those cases. There are no concrete plans yet, but I want to make sure that whatever solution we come up with doesn't result in multiple new flags down the line.

[leonardchan]

In D93668#2482986, @phosek wrote:

I'd prefer to use the target triple rather than introducing a custom flag.

With dedicated flags, you might eventually end up in a similar situation as D85802, that is in the extreme case you might end up with -f[no-]fuchsia-c++-abi, -f[no-]webassembly-c++-abi, etc. which is not any better than -fc++-abi=.

With target triple, I can imagine using either <arch>-unknown-fuchsia-itanium or <arch>-unknown-fuchsia-gnu, where the former would mean targeting Fuchsia with Itanium C++ ABI while the latter would mean using GCC compatible ABI (which would imply Itanium C++ ABI). Both of these are already used by MinGW for the same purpose so there's a precedent and we don't need to invent anything new.

@mcgrathr Would you be fine with using the triple instead of a new flag in this case?

[mcgrathr]

It's fine to have a target tuple translate to a default C++ ABI.
But the C++ ABI selection is fundamentally not a target flavor thing. It's just a C++ ABI thing.
So using the target tuple as the sole mechanism to determine C++ ABI is fundamentally wrong.

[rnk]

In D93668#2485719, @mcgrathr wrote:

It's fine to have a target tuple translate to a default C++ ABI.
But the C++ ABI selection is fundamentally not a target flavor thing. It's just a C++ ABI thing.
So using the target tuple as the sole mechanism to determine C++ ABI is fundamentally wrong.

Generally speaking, the C++ ABI corresponds to the target tuple. I don't want to give the user separate control of the C++ ABI without a compelling use case. If a command line flag is available, users will set it, they will request the AArch64 or Microsoft C++ ABI on x64, they will create a novel, never-before-previously-tested configuration, and when it doesn't work, they will file bugs about it. I aim to head that off at the pass. The two compelling use cases that I've heard of so far are:

1. vanilla Itanium C++ ABI code on Fuchsia
2. targeting fuchsia using wasm, some Fuchsia C++ ABI features may be needed

I think both of those cases can be handled with target tuples: *-*-fuchsia-itanium or webassembly-*-fuchsia
Do you think we need another way of expressing this?

[mcgrathr]

You've said you want to avoid expressing the C++ ABI as a generic switch orthogonal to target because people can use it when it's not what they actually want or isn't actually useful to attempt. This is true of all switches affecting ABI and they haven't all been rejected on this basis. But fine, consistency is the hobgoblin of small minds. If the concern there is either user confusion or support burden for configurations nobody wants to test, then both of those are easily addressed by having -fc++abi= constrain the settings it allows based on target. For Fuchsia targets, we want to maintain and test both "fuchsia" and "compat" (or "itanium") modes. If for other targets the only setting allowed is the one that's the default, or if using non-default settings generates a scare warning or suchlike, that's fine. That seems to be the natural way to constrain the explicit control of the C++ ABI to the supported cases, while not conflating control of the C++ ABI with the language-independent platform ABI being targeted.

You've said you want to avoid a proliferation of explicit switches for specific per-target C++ ABI selections. This is in fact the original rationale for -fc++abi=. Instead you've proposed a proliferation of specific per-target pseudo-tuples. This is exactly equivalent in material effect except that it's more hidden because possible tuples don't appear in --help output like separate switches do. It's worse in the abstract sense of correctness and in the self-documenting sense of being misleading and confusing by conflating the language-specific ABI with the platform ABI. You've cited the precedent of the mingw tuples. I'm not sufficiently familiar with the mingw cases to know whether they in fact are about nothing but the language ABI or are in fact actually about properly different platform ABIs in some other sense. If the latter, then your proposal exemplifies and worsens the kind of confusion between platform ABI (which tuples are explicitly about and always have been) with language ABI (which is outside the scope of what tuples were specified to describe) that I'm objecting to. If the former, then this bad precedent is a sign this problem has already caused the confusion I warned about and IMHO a precedent of doing the wrong thing is not a reason to compound the error.

[phosek]

I agree that if we want to allow selecting C++ ABI only, a flag like -fc++abi= with some additional checks to disallow invalid combinations is better. The question is whether we want to allow that in the first place? The motivation behind this change is to support generating code that's ABI compatible with the code generated by GCC. That could be expressed using the target triple (one idea is to use *-*-fuchsia-gnu), which would cover both C++ ABI as well as other aspects of the ABI (for example it could also disable SafeStack/ShadowCallStack) so developers don't need to concern themselves with all the details of the ABI, which could evolve over time expanding the set of flags you'd need to pass to the compiler to generate code that's ABI compatible with GCC.

[leonardchan]

In D93668#2492081, @phosek wrote:

I agree that if we want to allow selecting C++ ABI only, a flag like -fc++abi= with some additional checks to disallow invalid combinations is better. The question is whether we want to allow that in the first place? The motivation behind this change is to support generating code that's ABI compatible with the code generated by GCC. That could be expressed using the target triple (one idea is to use *-*-fuchsia-gnu), which would cover both C++ ABI as well as other aspects of the ABI (for example it could also disable SafeStack/ShadowCallStack) so developers don't need to concern themselves with all the details of the ABI, which could evolve over time expanding the set of flags you'd need to pass to the compiler to generate code that's ABI compatible with GCC.

In the context of representing the fourth component of the triple to represent the *platform* ABI rather than exclusively the C++ ABI, I think this would make sense. Using this other component would essentially just mean adding another layer of default flag configurations whose default values differ from those set by the first three components. I think we'd only want to have this extra layer if we have a sizable number of flags that we'd want separate from the default triple.

For our case, I think this mainly includes

• the C++ ABI (which encompasses "return this" on ctors/dtors and relative vtables),
• SS/SCS
• and maybe the PIC-friendly lookup tables if that ends up getting hidden behind a switch

but there could be new ABI changes down the line that could fall under this (or others that exist now but don't come to mind right now). Unless I'm missing some other features, this list seems fairly small and might not be worth tucking this under something that's meant to control a set of flags, but if we care more about being preventative from adding new flags, then it seems fine to just stick with using the triple.

[leonardchan]

Also, this probably isn't very important, but just as a heads up: if we were to depend on the environment component in the triple as the flag for selecting which multilib to use, then the multilib would be stored in a directory containing that environment.

So assuming we would check for the flag -target x86_64-fuchsia-gnu the multilib path would be something like:

${BUILD_DIR}/bin/../lib/x86_64-fuchsia-gnu/c++/compat+noexcept/

rather than

${BUILD_DIR}/bin/../lib/x86_64-fuchsia/c++/compat+noexcept/

[phosek]

In D93668#2510463, @leonardchan wrote:

Also, this probably isn't very important, but just as a heads up: if we were to depend on the environment component in the triple as the flag for selecting which multilib to use, then the multilib would be stored in a directory containing that environment.

So assuming we would check for the flag -target x86_64-fuchsia-gnu the multilib path would be something like:

${BUILD_DIR}/bin/../lib/x86_64-fuchsia-gnu/c++/compat+noexcept/

rather than

${BUILD_DIR}/bin/../lib/x86_64-fuchsia/c++/compat+noexcept/

I don't think we would use compat multilib in that case, instead we would simply use multiarch since these are now distinct targets. That is we would use just ${BUILD_DIR}/bin/../lib/x86_64-fuchsia-gnu/c++.

[mcgrathr]

The Fuchsia platform ABI includes the SS & SCS ABI. There is no compatibility issue with those. PIC-friendly optimizations are purely an optimization and correctness issue for any PIC/PIE code, and is not an ABI issue.
The only thing where we have an incompatible difference in ABI between our compiler environments is the C++ ABI.

[rnk]

I guess the spelling -fc++-abi= is intuitive, discoverable, and easy to use. I'd be OK going back to that, as long as the compiler knows which ABIs work on which targets and rejects the unsupported configurations.

Regarding the idea of separating the platform ABI from the C++ ABI, I can't say I really believe in the idea of language neutral platform ABIs. The platform ABI always incorporates aspects of the most widely used source languages, typically C. This is where LLVM gets into trouble when non-C frontends want to pass _Complex values or structs as arguments, for example. Whether you think of C++ features as being part of the platform or not probably depends on your view on whether C++ support is a critical part of the platform. I tend to think of C++ as pretty critical, but I'm a C++ programmer working on C++ ABI support, so that's my view.

Regarding reducing global ABI switches, yes, I would like to minimize them. We have many, and they have costs.

[phosek]

LGTM

[mcgrathr]

I am also a C++ programmer working on C++ ABI support. I'm glad to be working on a platform that made many intentional design decisions to enable keeping the C++ ABI out of the platform ABI. It makes it vastly easier to do interesting work on C++ ABIs such as what Leo is doing. It also makes it significantly easier to do interesting work on platform ABIs, which I also do.

[leonardchan]

Abandoned in favor of D85802.

clang/include/clang/AST/ASTContext.h

 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Linkage.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SanitizerBlacklist.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/TargetCXXABI.h"
 #include "clang/Basic/XRayLists.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/MapVector.h"
public:
   }
 
   DiagnosticsEngine &getDiagnostics() const;
 
   FullSourceLoc getFullLoc(SourceLocation Loc) const {
     return FullSourceLoc(Loc,SourceMgr);
   }
 
+  /// Return the C++ ABI kind that should be used. This is located here so the
+  /// ABI can be overriden by Clang-specific flags at compile-time. If there are
+  /// no overrides, we instead use the default ABI specified by the target.
+  TargetCXXABI::Kind getCXXABIKind(const TargetInfo *T = nullptr) const;
+
   /// All comments in this translation unit.
   RawCommentList Comments;
 
   /// True if comments are already loaded from ExternalASTSource.
   mutable bool CommentsLoaded = false;
 
   /// Mapping from declaration to directly attached comment.
   ///

clang/include/clang/Basic/LangOptions.h

public:
   /// Name of the IR file that contains the result of the OpenMP target
   /// host code generation.
   std::string OMPHostIRFile;
 
   /// Indicates whether the front-end is explicitly told that the
   /// input is a header file (i.e. -x c-header).
   bool IsHeaderFile = false;
 
+  /// Indicates that the Fuchsia C++ ABI is being used, either by default
+  /// through the target triple (ARCH-VENDOR-fuchsia) or explicitly through
+  /// the -ffuchsia-c++-abi flag.
+  bool UseFuchsiaCXXABI;
+
   LangOptions();
 
   // Define accessors/mutators for language options of enumeration type.
 #define LANGOPT(Name, Bits, Default, Description)
 #define ENUM_LANGOPT(Name, Type, Bits, Default, Description) \
   Type get##Name() const { return static_cast<Type>(Name); } \
   void set##Name(Type Value) { Name = static_cast<unsigned>(Value); }
 #include "clang/Basic/LangOptions.def"

clang/include/clang/Driver/Options.td

 defm fixed_point : OptInFFlag<"fixed-point", "Enable", "Disable", " fixed point types">;
 defm cxx_static_destructors : BoolFOption<"c++-static-destructors",
   "LangOpts->RegisterStaticDestructors", DefaultsToTrue,
   ChangedBy<NegFlag, [], "Disable C++ static destructor registration">,
   ResetBy<PosFlag>>;
 def fsymbol_partition_EQ : Joined<["-"], "fsymbol-partition=">, Group<f_Group>,
   Flags<[CC1Option]>, MarshallingInfoString<"CodeGenOpts.SymbolPartition">;
 
+def ffuchsia_cxx_abi : Flag<["-"], "ffuchsia-c++-abi">, Flags<[CC1Option]>,
+             Group<f_Group>, HelpText<"Explicitly use the Fuchsia C++ ABI">;
+def fno_fuchsia_cxx_abi : Flag<["-"], "fno-fuchsia-c++-abi">, Flags<[CC1Option]>,
+             Group<f_Group>, HelpText<"Do not explicitly use the Fuchsia C++ ABI. Instead use the default C++ ABI indicated by the target">;
+
 defm memory_profile : OptInFFlag<"memory-profile", "Enable", "Disable", " heap memory profiling">;
 def fmemory_profile_EQ : Joined<["-"], "fmemory-profile=">,
     Group<f_Group>, Flags<[CC1Option]>, MetaVarName<"<directory>">,
     HelpText<"Enable heap memory profiling and dump results into <directory>">;
 
 // Begin sanitizer flags. These should all be core options exposed in all driver
 // modes.
 let Flags = [CC1Option, CoreOption] in {

clang/lib/AST/ASTContext.cpp

ASTContext::getCanonicalTemplateTemplateParmDecl(
   (void)Canonical;
 
   // Create the canonical template template parameter entry.
   Canonical = new (*this) CanonicalTemplateTemplateParm(CanonTTP);
   CanonTemplateTemplateParms.InsertNode(Canonical, InsertPos);
   return CanonTTP;
 }
 
+TargetCXXABI::Kind ASTContext::getCXXABIKind(const TargetInfo *T) const {
+  if (getLangOpts().UseFuchsiaCXXABI)
+    return TargetCXXABI::Fuchsia;
+  if (T)
+    return T->getCXXABI().getKind();
+  return getTargetInfo().getCXXABI().getKind();
+}
+
 CXXABI *ASTContext::createCXXABI(const TargetInfo &T) {
   if (!LangOpts.CPlusPlus) return nullptr;
 
-  switch (T.getCXXABI().getKind()) {
+  switch (getCXXABIKind(&T)) {
   case TargetCXXABI::AppleARM64:
   case TargetCXXABI::Fuchsia:
   case TargetCXXABI::GenericARM: // Same as Itanium at this level
   case TargetCXXABI::iOS:
   case TargetCXXABI::WatchOS:
   case TargetCXXABI::GenericAArch64:
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:
if (!VTContext.get()) {
     }
   }
   return VTContext.get();
 }
 
 MangleContext *ASTContext::createMangleContext(const TargetInfo *T) {
   if (!T)
     T = Target;
-  switch (T->getCXXABI().getKind()) {
+  switch (getCXXABIKind(T)) {
   case TargetCXXABI::AppleARM64:
   case TargetCXXABI::Fuchsia:
   case TargetCXXABI::GenericAArch64:
   case TargetCXXABI::GenericItanium:
   case TargetCXXABI::GenericARM:
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::iOS:
   case TargetCXXABI::WebAssembly:

clang/lib/CodeGen/CodeGenModule.cpp

 static llvm::cl::opt<bool> LimitedCoverage(
     "limited-coverage-experimental", llvm::cl::ZeroOrMore, llvm::cl::Hidden,
     llvm::cl::desc("Emit limited coverage mapping information (experimental)"),
     llvm::cl::init(false));
 
 static const char AnnotationSection[] = "llvm.metadata";
 
 static CGCXXABI *createCXXABI(CodeGenModule &CGM) {
-  switch (CGM.getTarget().getCXXABI().getKind()) {
+  switch (CGM.getContext().getCXXABIKind()) {
   case TargetCXXABI::AppleARM64:
   case TargetCXXABI::Fuchsia:
   case TargetCXXABI::GenericAArch64:
   case TargetCXXABI::GenericARM:
   case TargetCXXABI::iOS:
   case TargetCXXABI::WatchOS:
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:

clang/lib/CodeGen/ItaniumCXXABI.cpp

 
 private:
   void emitCXXStermFinalizer(const VarDecl &D, llvm::Function *dtorStub,
                              llvm::Constant *addr);
 };
 }
 
 CodeGen::CGCXXABI *CodeGen::CreateItaniumCXXABI(CodeGenModule &CGM) {
-  switch (CGM.getTarget().getCXXABI().getKind()) {
+  switch (CGM.getContext().getCXXABIKind()) {
   // For IR-generation purposes, there's no significant difference
   // between the ARM and iOS ABIs.
   case TargetCXXABI::GenericARM:
   case TargetCXXABI::iOS:
   case TargetCXXABI::WatchOS:
     return new ARMCXXABI(CGM);
 
   case TargetCXXABI::AppleARM64:

clang/lib/Driver/ToolChains/Clang.cpp

#endif
   Args.AddLastArg(CmdArgs, options::OPT_pedantic_errors);
   Args.AddLastArg(CmdArgs, options::OPT_w);
 
   // Fixed point flags
   if (Args.hasFlag(options::OPT_ffixed_point, options::OPT_fno_fixed_point,
                    /*Default=*/false))
     Args.AddLastArg(CmdArgs, options::OPT_ffixed_point);
 
+  if (Args.hasFlag(options::OPT_ffuchsia_cxx_abi,
+                   options::OPT_fno_fuchsia_cxx_abi,
+                   /*Default=*/false))
+    Args.AddLastArg(CmdArgs, options::OPT_ffuchsia_cxx_abi);
+
   // Handle -{std, ansi, trigraphs} -- take the last of -{std, ansi}
   // (-ansi is equivalent to -std=c89 or -std=c++98).
   //
   // If a std is supplied, only add -trigraphs if it follows the
   // option.
   bool ImplyVCPPCXXVer = false;
   const Arg *Std = Args.getLastArg(options::OPT_std_EQ, options::OPT_ansi);
   if (Std) {

clang/lib/Frontend/CompilerInvocation.cpp

if (Arg *A = Args.getLastArg(OPT_msign_return_address_key_EQ)) {
               LangOptions::SignReturnAddressKeyKind::BKey);
         else
           Diags.Report(diag::err_drv_invalid_value)
               << A->getAsString(Args) << SignKey;
       }
     }
   }
 
+  Opts.UseFuchsiaCXXABI =
+      Args.hasFlag(OPT_ffuchsia_cxx_abi, OPT_fno_fuchsia_cxx_abi,
+                   /*default=*/T.isOSFuchsia());
+
   std::string ThreadModel =
       std::string(Args.getLastArgValue(OPT_mthread_model, "posix"));
   if (ThreadModel != "posix" && ThreadModel != "single")
     Diags.Report(diag::err_drv_invalid_value)
         << Args.getLastArg(OPT_mthread_model)->getAsString(Args) << ThreadModel;
   Opts.setThreadModel(
       llvm::StringSwitch<LangOptions::ThreadModelKind>(ThreadModel)
           .Case("posix", LangOptions::ThreadModelKind::POSIX)

clang/test/CodeGenCXX/constructor-destructor-return-this.cpp

 //RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-unknown-linux | FileCheck --check-prefix=CHECKGEN %s
 //RUN: %clang_cc1 %s -emit-llvm -o - -triple=thumbv7-apple-ios6.0 -target-abi apcs-gnu | FileCheck --check-prefix=CHECKARM %s
 //RUN: %clang_cc1 %s -emit-llvm -o - -triple=thumbv7-apple-ios5.0 -target-abi apcs-gnu | FileCheck --check-prefix=CHECKIOS5 %s
 //RUN: %clang_cc1 %s -emit-llvm -o - -triple=wasm32-unknown-unknown \
 //RUN:   | FileCheck --check-prefix=CHECKARM %s
 //RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-unknown-fuchsia | FileCheck --check-prefix=CHECKFUCHSIA %s
 //RUN: %clang_cc1 %s -emit-llvm -o - -triple=aarch64-unknown-fuchsia | FileCheck --check-prefix=CHECKFUCHSIA %s
+//RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-unknown-linux-gnu -ffuchsia-c++-abi | FileCheck --check-prefix=CHECKFUCHSIA %s
+//RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-unknown-fuchsia -fno-fuchsia-c++-abi| FileCheck --check-prefix=CHECKFUCHSIA %s
 //RUN: %clang_cc1 %s -emit-llvm -o - -triple=i386-pc-win32 -fno-rtti | FileCheck --check-prefix=CHECKMS %s
 // FIXME: these tests crash on the bots when run with -triple=x86_64-pc-win32
 
 // Make sure we attach the 'returned' attribute to the 'this' parameter of
 // constructors and destructors which return this (and only these cases)
 
 class A {
 public: