[AArch64] Add bti note property when compiling asm files with -mbranch-protection=bti

[steplong]

Summary

Generate the .note.gnu.property section with GNU_PROPERTY_AARCH64_FEATURE_1_BTI
when compiling assembly files with -mbranch-protection=bti.

See https://reviews.llvm.org/D81930 for generating the section when compiling
C/C++ files with -mbranch-protection=bti.

[apazos]

So the intention here is to generate the property when BTI branch protection is enabled, to guarantee the generate .o indeed has the property set, without requiring to pass the flag -mmark-bti-property explicitly. This is convenient for users.

Or is there a clear reason why one would enable BIT branch protection and not have the property set? What would be that use case?

[danielkiss]

The .note.gnu.property is already generated when C/C++ files are compiled with -mbranch-protection=bti.
-mmark-bti-property is only for assembly file where the .note.gnu.property should be added manually otherwise.

Do you have any reproducer where C/C++ behaves unexpectedly?

[steplong]

In D93428#2460032, @danielkiss wrote:

The .note.gnu.property is already generated when C/C++ files are compiled with -mbranch-protection=bti.
-mmark-bti-property is only for assembly file where the .note.gnu.property should be added manually otherwise.

Do you have any reproducer where C/C++ behaves unexpectedly?

I think Ana misunderstood the reason for this patch. We haven't seen any compilation of C/C++ behaving unexpectedly. -mbranch-protection=bti generates the .nute.gnu.property for C/C++ files from our experiments, but doesn't work for assembly files. Is there a reason why assembly files have a different flag (i.e. -mmark-bti-property) to create the .note.gnu.property with the BTI entry?

[apazos]

Thanks for clarifying - so the property is being set for C/C++ files but not for assembly files. I think it should be set automatically for both when one uses clang driver to compile/assemble.

[danielkiss]

Is there a reason why assembly files have a different flag (i.e. -mmark-bti-property) to create the .note.gnu.property with the BTI entry?

In assembly the compiler can't guarantee the landing pads are in place, therefore it doesn't add it automatically.
The original concept was this the developers should add the landing pads wherever needed and by adding the note they mark the file is compatible with BTI.
After adding BTI to many assembly code it was clear the note is error prone and cumbersome to handle and I thing it provides zero protection against regression issues , so the -mmark-bti-property is introduced.
The developers still should add the landing pads but optionally they could mark the files in the build system instead of the assembly files.
The worry is if the assembly file would be marked automatically the produces binary probably won't run correctly.

[apazos]

Thanks Daniel for the explanation. Was the support added for GNU assembler as well? Is it the same flag name?

[danielkiss]

In D93428#2461343, @apazos wrote:

Thanks Daniel for the explanation.

No problem at all.

In D93428#2461343, @apazos wrote:

Was the support added for GNU assembler as well? Is it the same flag name?

Not yet, I requested the same flag from our GNU team when the flag is added to Clang.

[apazos]

Can you confirm when the GNU toolchain will have this flag supported in their assembler? Compatibility between LLVM and GNU toolchains is important.

Stephen - I think we can abandon this review. Users will need to be made aware of this additional assembler flag when building .s files with BTI enabled.

clang/lib/Driver/ToolChains/Clang.h

public:
   ClangAs(const ToolChain &TC)
       : Tool("clang::as", "clang integrated assembler", TC) {}
   void AddMIPSTargetArgs(const llvm::opt::ArgList &Args,
                          llvm::opt::ArgStringList &CmdArgs) const;
   void AddX86TargetArgs(const llvm::opt::ArgList &Args,
                         llvm::opt::ArgStringList &CmdArgs) const;
   void AddRISCVTargetArgs(const llvm::opt::ArgList &Args,
                           llvm::opt::ArgStringList &CmdArgs) const;
+  void AddAArch64TargetArgs(const llvm::opt::ArgList &Args,
+                            llvm::opt::ArgStringList &CmdArgs) const;
   bool hasGoodDiagnostics() const override { return true; }
   bool hasIntegratedAssembler() const override { return false; }
   bool hasIntegratedCPP() const override { return false; }
 
   void ConstructJob(Compilation &C, const JobAction &JA,
                     const InputInfo &Output, const InputInfoList &Inputs,
                     const llvm::opt::ArgList &TCArgs,
                     const char *LinkingOutput) const override;

clang/lib/Driver/ToolChains/Clang.cpp

void ClangAs::AddRISCVTargetArgs(const ArgList &Args,
                                ArgStringList &CmdArgs) const {
   const llvm::Triple &Triple = getToolChain().getTriple();
   StringRef ABIName = riscv::getRISCVABI(Args, Triple);
 
   CmdArgs.push_back("-target-abi");
   CmdArgs.push_back(ABIName.data());
 }
 
+void ClangAs::AddAArch64TargetArgs(const ArgList &Args,
+                                   ArgStringList &CmdArgs) const {
+  bool BranchTargetEnforce = false;
+  const auto &D = getToolChain().getDriver();
+
+  if (Arg *A = Args.getLastArg(options::OPT_mbranch_protection_EQ)) {
+    StringRef Err;
+    llvm::AArch64::ParsedBranchProtection PBP;
+
+    if (!llvm::AArch64::parseBranchProtection(A->getValue(), PBP, Err)) {
+      D.Diag(diag::err_invalid_branch_protection)
+          << Err << A->getAsString(Args);
+    } else {
+      BranchTargetEnforce = PBP.BranchTargetEnforcement;
+    }
+  }
+
+  if (Args.hasArg(options::OPT_mmark_bti_property) || BranchTargetEnforce) {
+    CmdArgs.push_back("-mllvm");
+    CmdArgs.push_back("-aarch64-mark-bti-property");
+  }
+}
+
 void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
                            const InputInfo &Output, const InputInfoList &Inputs,
                            const ArgList &Args,
                            const char *LinkingOutput) const {
   ArgStringList CmdArgs;
 
   assert(Inputs.size() == 1 && "Unexpected number of inputs.");
   const InputInfo &Input = Inputs[0];
if (Args.hasFlag(options::OPT_mdefault_build_attributes,
         CmdArgs.push_back("-mllvm");
         CmdArgs.push_back("-arm-add-build-attributes");
     }
     break;
 
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_32:
   case llvm::Triple::aarch64_be:
-    if (Args.hasArg(options::OPT_mmark_bti_property)) {
-      CmdArgs.push_back("-mllvm");
-      CmdArgs.push_back("-aarch64-mark-bti-property");
-    }
+    AddAArch64TargetArgs(Args, CmdArgs);
     break;
 
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
     AddRISCVTargetArgs(Args, CmdArgs);
     break;
   }
 

clang/test/Driver/arm64-markbti.S

 // REQUIRES: aarch64-registered-target
 
-// When -mmark-bti-property is passed the generated file object gets BTI marking.
+// When -mmark-bti-property or -mbranch-protection=bti is passed the generated file object gets BTI marking.
 // RUN: %clang -target arm64-linux-none -mmark-bti-property -c -o - %s | llvm-readobj -n - | FileCheck -check-prefix=CHECK  -check-prefix=CHECK_GEN %s
 // RUN: %clang -target arm64-linux-none -DNOTE_PRESENT -c %s -o - | llvm-readobj -n - | FileCheck -check-prefix=CHECK  -check-prefix=CHECK_PRESET %s
 // RUN: %clang -target arm64-linux-none -mmark-bti-property -DNOTE_PRESENT -c %s -o - | llvm-readobj -n - | FileCheck -check-prefix=CHECK  -check-prefix=CHECK_PRESET %s
 // RUN: %clang -target arm64-linux-none -mmark-bti-property -DNOTE_PRESENT -c %s -o - 2>&1 |  FileCheck -check-prefix=CHECK_WARNING %s
+// RUN: %clang -target arm64-linux-none -mbranch-protection=bti -c -o - %s | llvm-readobj -n - | FileCheck -check-prefix=CHECK  -check-prefix=CHECK_GEN %s
+// RUN: %clang -target arm64-linux-none -mbranch-protection=bti -DNOTE_PRESENT -c %s -o - 2>&1 |  FileCheck -check-prefix=CHECK_WARNING %s
 //
 // CHECK_WARNING: The .note.gnu.property is not emitted because it is already present.
 // CHECK: Name: .note.gnu.property
 // CHECK: Type: NT_GNU_PROPERTY_TYPE_0
 // CHECK_GEN: aarch64 feature: BTI
 // CHECK_PRESET: aarch64 feature: BTI, PAC
 
 #ifdef NOTE_PRESENT