Implement target(branch-protection) attribute for AArch64

[chill]

Summary

This patch implements __attribute__((target("branch-protection=..."))) in a manner, compatible with the analogous GCC feature:

https://gcc.gnu.org/onlinedocs/gcc-9.2.0/gcc/AArch64-Function-Attributes.html#AArch64-Function-Attributes

[chill]

Removed some accidental change.

[aaron.ballman]

You should also update AttrDocs.td for the new feature.

clang/include/clang/Basic/DiagnosticSemaKinds.td

2595

Invalid -> invalid

should perhaps have '%0' to show what part is invalid?

clang/lib/CodeGen/TargetInfo.cpp

5062–5063

Don't do hasAttr<> followed by getAttr<>, you should do:

if (const auto *TA = FD->getAttr<TargetAttr>())

5071

auto *

[chill]

Ping?

[ostannard]

I think this also needs a check to make sure the selected architecture supports PAC/BTI. Gcc gives an unknown target attribute or pragma error if this is used for a non-AArch64 target.

[chill]

Moved parsing out of Attr.td to TargetInfo, following the example (good or bad) of validateOutputConstraint or validateGlobalregisterVariable ;)
Now issue a warning if attribute is not supported by target, like for other unsupported target options.

[ostannard]

clang/lib/Basic/Targets/AArch64.cpp

106

This completely duplicates the ParseAArch64BranchProtection function in the driver, could we share one implementation between the command line option and attribute?

[chill]

Moved parsing of branch protection spec to TargetParser.

[chill]

Ping?

[ostannard]

LGTM, thanks

clang/include/clang/Basic/Attr.td

def Target : InheritableAttr {
   let Spellings = [GCC<"target">];
   let Args = [StringArgument<"featuresStr">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [TargetDocs];
   let AdditionalMembers = [{
     struct ParsedTargetAttr {
       std::vector<std::string> Features;
       StringRef Architecture;
+      StringRef BranchProtection;
       bool DuplicateArchitecture = false;
       bool operator ==(const ParsedTargetAttr &Other) const {
         return DuplicateArchitecture == Other.DuplicateArchitecture &&
                Architecture == Other.Architecture && Features == Other.Features;
       }
     };
     ParsedTargetAttr parse() const {
       return parse(getFeaturesStr());
static ParsedTargetAttr parse(StringRef Features) {
 
         // We don't support cpu tuning this way currently.
         // TODO: Support the fpmath option. It will require checking
         // overall feature validity for the function with the rest of the
         // attributes on the function.
         if (Feature.startswith("fpmath=") || Feature.startswith("tune="))
           continue;
 
+        if (Feature.startswith("branch-protection=")) {
+          Ret.BranchProtection = Feature.split('=').second.trim();
+          continue;
+        }
+
         // While we're here iterating check for a different target cpu.
         if (Feature.startswith("arch=")) {
           if (!Ret.Architecture.empty())
             Ret.DuplicateArchitecture = true;
           else
             Ret.Architecture = Feature.split("=").second.trim();
         } else if (Feature.startswith("no-"))
           Ret.Features.push_back("-" + Feature.split("-").second.str());

clang/include/clang/Basic/AttrDocs.td

 the backend to use different code generation options than were passed on the
 command line.
 
 The current set of options correspond to the existing "subtarget features" for
 the target with or without a "-mno-" in front corresponding to the absence
 of the feature, as well as ``arch="CPU"`` which will change the default "CPU"
 for the function.
 
+For AArch64, the attribute also allows the "branch-protection=<args>" option,
+where the permissible arguments and their effect on code generation are the same
+as for the command-line option ``-mbranch-protection``.
+
 Example "subtarget features" from the x86 backend include: "mmx", "sse", "sse4.2",
 "avx", "xop" and largely correspond to the machine specific options handled by
 the front end.
 
 Additionally, this attribute supports function multiversioning for ELF based
 x86/x86-64 targets, which can be used to create multiple implementations of the
 same function that will be resolved at runtime based on the priority of their
 ``target`` attribute strings. A function is considered a multiversioned function

clang/include/clang/Basic/CodeGenOptions.h

public:
 
   enum EmbedBitcodeKind {
     Embed_Off,      // No embedded bitcode.
     Embed_All,      // Embed both bitcode and commandline in the output.
     Embed_Bitcode,  // Embed just the bitcode in the output.
     Embed_Marker    // Embed a marker as a placeholder for bitcode.
   };
 
-  enum SignReturnAddressScope {
+  enum class SignReturnAddressScope {
     None,    // No signing for any function
     NonLeaf, // Sign the return address of functions that spill LR
     All      // Sign the return address of all functions
   };
 
-  enum SignReturnAddressKeyValue { AKey, BKey };
+  enum class SignReturnAddressKeyValue { AKey, BKey };
 
   enum class FramePointerKind {
     None,        // Omit all frame pointers.
     NonLeaf,     // Keep non-leaf frame pointers.
     All,         // Keep all frame pointers.
   };
 
   /// The code model to use (-mcmodel).

clang/include/clang/Basic/CodeGenOptions.def

 CODEGENOPT(EmbedSource, 1, 0)
 
 /// Whether to emit all vtables
 CODEGENOPT(ForceEmitVTables, 1, 0)
 
 /// Whether to emit an address-significance table into the object file.
 CODEGENOPT(Addrsig, 1, 0)
 
-ENUM_CODEGENOPT(SignReturnAddress, SignReturnAddressScope, 2, None)
-ENUM_CODEGENOPT(SignReturnAddressKey, SignReturnAddressKeyValue, 1, AKey)
+ENUM_CODEGENOPT(SignReturnAddress, SignReturnAddressScope, 2, SignReturnAddressScope::None)
+ENUM_CODEGENOPT(SignReturnAddressKey, SignReturnAddressKeyValue, 1, SignReturnAddressKeyValue::AKey)
 CODEGENOPT(BranchTargetEnforcement, 1, 0)
 
 /// Whether to emit unused static constants.
 CODEGENOPT(KeepStaticConsts, 1, 0)
 
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
 #undef VALUE_CODEGENOPT
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def err_attribute_invalid_vector_type : Error<"invalid vector element type %0">;
 def err_attribute_bad_neon_vector_size : Error<
   "Neon vector size must be 64 or 128 bits">;
 def err_attribute_requires_positive_integer : Error<
   "%0 attribute requires a %select{positive|non-negative}1 "
   "integral compile time constant expression">;
 def err_attribute_requires_opencl_version : Error<
   "%0 attribute requires OpenCL version %1%select{| or above}2">;
+def err_invalid_branch_protection_spec : Error<
+  "invalid or misplaced branch protection specification '%0'">;
 def warn_unsupported_target_attribute
     : Warning<"%select{unsupported|duplicate}0%select{| architecture}1 '%2' in"
               " the 'target' attribute string; 'target' attribute ignored">,
       InGroup<IgnoredAttributes>;
 def err_attribute_unsupported
     : Error<"%0 attribute is not supported for this target">;
 // The err_*_attribute_argument_not_int are separate because they're used by
 // VerifyIntegerConstantExpression.

clang/include/clang/Basic/TargetInfo.h

 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_TARGETINFO_H
 #define LLVM_CLANG_BASIC_TARGETINFO_H
 
 #include "clang/Basic/AddressSpaces.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetCXXABI.h"
 #include "clang/Basic/TargetOptions.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringMap.h"
virtual void setFeatureEnabled(llvm::StringMap<bool> &Features,
     Features[Name] = Enabled;
   }
 
   /// Determine whether this TargetInfo supports the given feature.
   virtual bool isValidFeatureName(StringRef Feature) const {
     return true;
   }
 
+  struct BranchProtectionInfo {
+    CodeGenOptions::SignReturnAddressScope SignReturnAddr =
+        CodeGenOptions::SignReturnAddressScope::None;
+    CodeGenOptions::SignReturnAddressKeyValue SignKey =
+        CodeGenOptions::SignReturnAddressKeyValue::AKey;
+    bool BranchTargetEnforcement = false;
+  };
+
+  /// Determine if this TargetInfo supports the given branch protection
+  /// specification
+  virtual bool validateBranchProtection(StringRef Spec,
+                                        BranchProtectionInfo &BPI,
+                                        StringRef &Err) const {
+    Err = "";
+    return false;
+  }
+
   /// Perform initialization based on the user configured
   /// set of features (e.g., +sse4).
   ///
   /// The list is guaranteed to have at most one entry per feature.
   ///
   /// The target may modify the features list, to change which options are
   /// passed onwards to the backend.
   /// FIXME: This part should be fixed so that we can change handleTargetFeatures

clang/lib/Basic/Targets/AArch64.h

class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
   std::string ABI;
 
 public:
   AArch64TargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts);
 
   StringRef getABI() const override;
   bool setABI(const std::string &Name) override;
 
+  bool validateBranchProtection(StringRef, BranchProtectionInfo &,
+                                StringRef &) const override;
+
   bool isValidCPUName(StringRef Name) const override;
   void fillValidCPUList(SmallVectorImpl<StringRef> &Values) const override;
   bool setCPU(const std::string &Name) override;
 
   bool useFP16ConversionIntrinsics() const override {
     return false;
   }
 

clang/lib/Basic/Targets/AArch64.cpp

 bool AArch64TargetInfo::setABI(const std::string &Name) {
   if (Name != "aapcs" && Name != "darwinpcs")
     return false;
 
   ABI = Name;
   return true;
 }
 
+bool AArch64TargetInfo::validateBranchProtection(StringRef Spec,
+                                                 BranchProtectionInfo &BPI,
+                                                 StringRef &Err) const {
+  BPI = {};
+  if (Spec == "none") {
+    // defaults are fine
+    return true;
+  }
+
+  if (Spec == "standard") {
+    BPI.SignReturnAddr = CodeGenOptions::SignReturnAddressScope::NonLeaf;
+    BPI.BranchTargetEnforcement = true;
+    return true;
+  }
+
+  SmallVector<StringRef, 4> Opts;
+  Spec.split(Opts, "+");
+  for (int I = 0, E = Opts.size(); I != E; ++I) {
+    StringRef Opt = Opts[I].trim();
+    if (Opt == "bti") {
+      BPI.BranchTargetEnforcement = true;
+      continue;
+    }
+    if (Opt == "pac-ret") {
+      BPI.SignReturnAddr = CodeGenOptions::SignReturnAddressScope::NonLeaf;
+      for (; I + 1 != E; ++I) {
+        StringRef PACOpt = Opts[I + 1].trim();
+        if (PACOpt == "leaf")
+          BPI.SignReturnAddr = CodeGenOptions::SignReturnAddressScope::All;
+        else if (PACOpt == "b-key")
+          BPI.SignKey = CodeGenOptions::SignReturnAddressKeyValue::BKey;
+        else
+          break;
+      }
+      continue;
+    }
+    if (Opt == "")
+      Err = "<empty>";
+    else
+      Err = Opt;
+    return false;
+  }
+
+  return true;
+}
+
 bool AArch64TargetInfo::isValidCPUName(StringRef Name) const {
   return Name == "generic" ||
          llvm::AArch64::parseCPUArch(Name) != llvm::AArch64::ArchKind::INVALID;
 }
 
 bool AArch64TargetInfo::setCPU(const std::string &Name) {
   return isValidCPUName(Name);
 }

clang/lib/CodeGen/TargetInfo.cpp

public:
 
   bool doesReturnSlotInterfereWithArgs() const override { return false; }
 
   void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
                            CodeGen::CodeGenModule &CGM) const override {
     const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D);
     if (!FD)
       return;
-    llvm::Function *Fn = cast<llvm::Function>(GV);
 
-    auto Kind = CGM.getCodeGenOpts().getSignReturnAddress();
-    if (Kind != CodeGenOptions::SignReturnAddressScope::None) {
+    CodeGenOptions::SignReturnAddressScope Scope = CGM.getCodeGenOpts().getSignReturnAddress();
+    CodeGenOptions::SignReturnAddressKeyValue Key = CGM.getCodeGenOpts().getSignReturnAddressKey();
+    bool BranchTargetEnforcement = CGM.getCodeGenOpts().BranchTargetEnforcement;
+    if (const auto *TA = FD->getAttr<TargetAttr>()) {
+      TargetAttr::ParsedTargetAttr Attr = TA->parse();
+      if (!Attr.BranchProtection.empty()) {
+        TargetInfo::BranchProtectionInfo BPI;
+        StringRef Error;
+        (void)CGM.getTarget().validateBranchProtection(Attr.BranchProtection,
+                                                       BPI, Error);
+        assert(Error.empty());
+        Scope = BPI.SignReturnAddr;
+        Key = BPI.SignKey;
+        BranchTargetEnforcement = BPI.BranchTargetEnforcement;
+      }
+    }
+
+    auto *Fn = cast<llvm::Function>(GV);
+    if (Scope != CodeGenOptions::SignReturnAddressScope::None) {
       Fn->addFnAttr("sign-return-address",
-                    Kind == CodeGenOptions::SignReturnAddressScope::All
+                    Scope == CodeGenOptions::SignReturnAddressScope::All
                         ? "all"
                         : "non-leaf");
 
-      auto Key = CGM.getCodeGenOpts().getSignReturnAddressKey();
       Fn->addFnAttr("sign-return-address-key",
                     Key == CodeGenOptions::SignReturnAddressKeyValue::AKey
                         ? "a_key"
                         : "b_key");
     }
 
-    if (CGM.getCodeGenOpts().BranchTargetEnforcement)
+    if (BranchTargetEnforcement)
       Fn->addFnAttr("branch-target-enforcement");
   }
 };
 
 class WindowsAArch64TargetCodeGenInfo : public AArch64TargetCodeGenInfo {
 public:
   WindowsAArch64TargetCodeGenInfo(CodeGenTypes &CGT, AArch64ABIInfo::ABIKind K)
       : AArch64TargetCodeGenInfo(CGT, K) {}

clang/lib/Sema/SemaDeclAttr.cpp

bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
 
   for (const auto &Feature : ParsedAttrs.Features) {
     auto CurFeature = StringRef(Feature).drop_front(); // remove + or -.
     if (!Context.getTargetInfo().isValidFeatureName(CurFeature))
       return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
              << Unsupported << None << CurFeature;
   }
 
+  TargetInfo::BranchProtectionInfo BPI;
+  StringRef Error;
+  if (!ParsedAttrs.BranchProtection.empty() &&
+      !Context.getTargetInfo().validateBranchProtection(
+          ParsedAttrs.BranchProtection, BPI, Error)) {
+    if (Error.empty())
+      return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
+             << Unsupported << None << "branch-protection";
+    else
+      return Diag(LiteralLoc, diag::err_invalid_branch_protection_spec)
+             << Error;
+  }
+
   return false;
 }
 
 static void handleTargetAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   StringRef Str;
   SourceLocation LiteralLoc;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &LiteralLoc) ||
       S.checkTargetAttr(LiteralLoc, Str))

clang/test/CodeGen/aarch64-branch-protection-attr.c

+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -triple aarch64-unknown-unknown-eabi -emit-llvm  -target-cpu generic -target-feature +v8.5a %s -o - \
+// RUN:                               | FileCheck %s --check-prefix=CHECK --check-prefix=NO-OVERRIDE
+// RUN: %clang_cc1 -triple aarch64-unknown-unknown-eabi -emit-llvm  -target-cpu generic -target-feature +v8.5a %s -o - \
+// RUN:   -msign-return-address=non-leaf -msign-return-address-key=a_key -mbranch-target-enforce \
+// RUN:                               | FileCheck %s --check-prefix=CHECK --check-prefix=OVERRIDE
+
+void missing() {}
+// NO-OVERRIDE: define void @missing() #[[#NONE:]]
+// OVERRIDE: define void @missing() #[[#STD:]]
+
+__attribute__ ((target("branch-protection=none")))
+void none() {}
+// NO-OVERRIDE: define void @none() #[[#NONE]]
+// OVERRIDE: define void @none() #[[#NONE:]]
+
+  __attribute__ ((target("branch-protection=standard")))
+void std() {}
+// NO-OVERRIDE: define void @std() #[[#STD:]]
+// OVERRIDE: define void @std() #[[#STD]]
+
+__attribute__ ((target("branch-protection=bti")))
+void btionly() {}
+// NO-OVERRIDE: define void @btionly() #[[#BTI:]]
+// OVERRIDE: define void @btionly() #[[#BTI:]]
+
+__attribute__ ((target("branch-protection=pac-ret")))
+void paconly() {}
+// NO-OVERRIDE: define void @paconly() #[[#PAC:]]
+// OVERRIDE: define void @paconly() #[[#PAC:]]
+
+__attribute__ ((target("branch-protection=pac-ret+bti")))
+void pacbti0() {}
+// NO-OVERRIDE: define void @pacbti0() #[[#PACBTI:]]
+// OVERRIDE: define void @pacbti0() #[[#PACBTI:]]
+
+__attribute__ ((target("branch-protection=bti+pac-ret")))
+void pacbti1() {}
+// NO-OVERRIDE: define void @pacbti1() #[[#PACBTI]]
+// OVERRIDE: define void @pacbti1() #[[#PACBTI]]
+
+__attribute__ ((target("branch-protection=pac-ret+leaf")))
+void leaf() {}
+// NO-OVERRIDE: define void @leaf() #[[#PACLEAF:]]
+// OVERRIDE: define void @leaf() #[[#PACLEAF:]]
+
+__attribute__ ((target("branch-protection=pac-ret+b-key")))
+void bkey() {}
+// NO-OVERRIDE: define void @bkey() #[[#PACBKEY:]]
+// OVERRIDE: define void @bkey() #[[#PACBKEY:]]
+
+__attribute__ ((target("branch-protection=pac-ret+b-key+leaf")))
+void bkeyleaf0() {}
+// NO-OVERRIDE: define void @bkeyleaf0()  #[[#PACBKEYLEAF:]]
+// OVERRIDE: define void @bkeyleaf0()  #[[#PACBKEYLEAF:]]
+
+__attribute__ ((target("branch-protection=pac-ret+leaf+b-key")))
+void bkeyleaf1() {}
+// NO-OVERRIDE: define void @bkeyleaf1()  #[[#PACBKEYLEAF]]
+// OVERRIDE: define void @bkeyleaf1()  #[[#PACBKEYLEAF]]
+
+__attribute__ ((target("branch-protection=pac-ret+leaf+bti")))
+void btileaf() {}
+// NO-OVERRIDE: define void @btileaf() #[[#BTIPACLEAF:]]
+// OVERRIDE: define void @btileaf() #[[#BTIPACLEAF:]]
+
+// CHECK-DAG: attributes #[[#NONE]]
+
+// CHECK-DAG: attributes #[[#STD]] = { {{.*}} "branch-target-enforcement" {{.*}} "sign-return-address"="non-leaf" "sign-return-address-key"="a_key"
+
+// CHECK-DAG: attributes #[[#BTI]] = { {{.*}}"branch-target-enforcement"
+
+// CHECK-DAG: attributes #[[#PAC]] = { {{.*}} "sign-return-address"="non-leaf" "sign-return-address-key"="a_key"
+
+// CHECK-DAG: attributes #[[#PACLEAF]] = { {{.*}} "sign-return-address"="all" "sign-return-address-key"="a_key"
+
+// CHECK-DAG: attributes #[[#PACBKEY]] = { {{.*}} "sign-return-address"="non-leaf" "sign-return-address-key"="b_key"
+
+// CHECK-DAG: attributes #[[#PACBKEYLEAF]] = { {{.*}} "sign-return-address"="all" "sign-return-address-key"="b_key"
+
+// CHECK-DAG: attributes #[[#BTIPACLEAF]] = { {{.*}}"branch-target-enforcement" {{.*}} "sign-return-address"="all" "sign-return-address-key"="a_key"

clang/test/Sema/attr-target.c

 //expected-warning@+1 {{unsupported 'woof' in the 'target' attribute string; 'target' attribute ignored}}
 int __attribute__((target("woof"))) bark() {  return 4; }
 // no warning, same as saying 'nothing'.
 int __attribute__((target("arch="))) turtle() { return 4; }
 //expected-warning@+1 {{unsupported architecture 'hiss' in the 'target' attribute string; 'target' attribute ignored}}
 int __attribute__((target("arch=hiss,arch=woof"))) pine_tree() { return 4; }
 //expected-warning@+1 {{duplicate 'arch=' in the 'target' attribute string; 'target' attribute ignored}}
 int __attribute__((target("arch=ivybridge,arch=haswell"))) oak_tree() { return 4; }
-
+//expected-warning@+1 {{unsupported 'branch-protection' in the 'target' attribute string; 'target' attribute ignored}}
+int __attribute__((target("branch-protection=none"))) birch_tree() { return 5; }
 

clang/test/Sema/branch-protection-attr-err.c

+// RUN: %clang_cc1 -triple aarch64 -verify -fsyntax-only %s
+
+__attribute__((target("branch-protection=foo"))) // expected-error {{invalid or misplaced branch protection specification 'foo'}}
+void badvalue0() {}
+
+__attribute__((target("branch-protection=+bti"))) // expected-error {{invalid or misplaced branch protection specification '<empty>'}}
+void badvalue1() {}
+
+__attribute__((target("branch-protection=bti+"))) // expected-error {{invalid or misplaced branch protection specification '<empty>'}}
+void badvalue2() {}
+
+__attribute__((target("branch-protection=pac-ret+bkey"))) // expected-error {{invalid or misplaced branch protection specification 'bkey'}}
+void badvalue3() {}
+
+__attribute__((target("branch-protection=bti+leaf"))) // expected-error {{invalid or misplaced branch protection specification 'leaf'}}
+void badoption0() {}
+
+__attribute__((target("branch-protection=bti+leaf+pac-ret"))) // expected-error {{invalid or misplaced branch protection specification 'leaf'}}
+void badorder0() {}
+
+__attribute__ ((target("branch-protection=pac-ret+bti+leaf")))  // expected-error {{invalid or misplaced branch protection specification 'leaf'}}
+void badorder1() {}