[AArch64] Add -mmark-bti-property flag.

[danielkiss]

Summary

Writing the .note.gnu.property manually is error prone and hard to
maintain in the assembly files.
The -mmark-bti-property is for the assembler to emit the section with the
GNU_PROPERTY_AARCH64_FEATURE_1_BTI. To be used when C/C++ is compiled
with -mbranch-protection=bti.

This patch refactors the .note.gnu.property handling.

[danielkiss]

I got some comment internally, the -mbti might suggest it enables the bti feature so I'm going to rename the flag to -mmark-bti-property.

[danielkiss]

Rename the flag to -mmark-bti-property

[chill]

llvm/lib/Target/AArch64/AArch64.td

352 ↗

(On Diff #272024)

No, this is an abuse of subtarget features. Subtarget features represent characteristics of the chip, they shouldn't be used to pass arbitrary bits of information.
Possible alternatives - TargetOptions (cf. BackendUtil.cpp:initTargetOptions()) or
LLVM command-line arguments (cf. BackendUtil.cpp:setCommandLineOpts().

[danielkiss]

Fix review comments.

[danielkiss]

llvm/lib/Target/AArch64/AArch64.td

352 ↗

(On Diff #272024)

Thanks, now I learned how to pass flags across layers, that was not clear at the first time.

[chill]

llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp

106 ↗

(On Diff #285492)

No need to the .getValue() part.

llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.h

34 ↗

(On Diff #285492)

Is there a need for this data member? The option value does not change over time, and the option can be defined in AArch64TargetStreamer.cpp.

[chill]

LGTM. It'd be nice if we could get someone non-Arm to have a look too. though.

[nickdesaulniers]

I don't have any thoughts on the change per se, so just minor thoughts/generic code review.

clang/lib/Driver/ToolChains/Clang.cpp

7004

it looks like A is unused. Should we be using Args.hasFlag instead of Args.getLastArg?

llvm/lib/Target/AArch64/MCTargetDesc/AArch64TargetStreamer.cpp

56–58

The coding style allows for the curly braces to be omitted for single statement bodies.

64

ditto

68

move this to just before the SwitchSection call below?

llvm/lib/Target/AArch64/MCTargetDesc/AArch64TargetStreamer.h

43

does this need to be virtual?

[danielkiss]

@nickdesaulniers Thanks for the review, comments are addressed.

clang/include/clang/Driver/Options.td

 def mgeneral_regs_only : Flag<["-"], "mgeneral-regs-only">, Group<m_aarch64_Features_Group>,
   HelpText<"Generate code which only uses the general purpose registers (AArch64 only)">;
 def mfix_cortex_a53_835769 : Flag<["-"], "mfix-cortex-a53-835769">,
   Group<m_aarch64_Features_Group>,
   HelpText<"Workaround Cortex-A53 erratum 835769 (AArch64 only)">;
 def mno_fix_cortex_a53_835769 : Flag<["-"], "mno-fix-cortex-a53-835769">,
   Group<m_aarch64_Features_Group>,
   HelpText<"Don't workaround Cortex-A53 erratum 835769 (AArch64 only)">;
+def mmark_bti_property : Flag<["-"], "mmark-bti-property">,
+  Group<m_aarch64_Features_Group>,
+  HelpText<"Add .note.gnu.property with BTI to assembly files (AArch64 only)">;
 foreach i = {1-31} in
   def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group<m_Group>,
     HelpText<"Reserve the x"#i#" register (AArch64/RISC-V only)">;
 
 foreach i = {8-15,18} in
   def fcall_saved_x#i : Flag<["-"], "fcall-saved-x"#i>, Group<m_aarch64_Features_Group>,
     HelpText<"Make the x"#i#" register call-saved (AArch64 only)">;
 

clang/lib/Driver/ToolChains/Clang.cpp

case llvm::Triple::thumbeb:
     // only, not C/C++.
     if (Args.hasFlag(options::OPT_mdefault_build_attributes,
                      options::OPT_mno_default_build_attributes, true)) {
         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");
+    }
+    break;
+
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
     AddRISCVTargetArgs(Args, CmdArgs);
     break;
   }
 
   // Consume all the warning flags. Usually this would be handled more
   // gracefully by -cc1 (warning about unknown warning flags, etc) but -cc1as

clang/test/Driver/arm64-markbti.S

+// When -mmark-bti-property 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
+//
+// 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
+  .section .note.gnu.property, "a";
+  .balign 8;
+  .long 4;
+  .long 0x10;
+  .long 0x5
+  .asciz "GNU"
+  .long 0xc0000000
+  .long 4
+  .long 3
+  .long 0
+#endif

llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp

if (any_of(M, [](const Function &F) {
       })) {
     Flags &= ~ELF::GNU_PROPERTY_AARCH64_FEATURE_1_PAC;
   }
 
   if (Flags == 0)
     return;
 
   // Emit a .note.gnu.property section with the flags.
-  MCSection *Cur = OutStreamer->getCurrentSectionOnly();
-  MCSection *Nt = MMI->getContext().getELFSection(
-      ".note.gnu.property", ELF::SHT_NOTE, ELF::SHF_ALLOC);
-  OutStreamer->SwitchSection(Nt);
-
-  // Emit the note header.
-  emitAlignment(Align(8));
-  OutStreamer->emitInt32(4);     // data size for "GNU\0"
-  OutStreamer->emitInt32(4 * 4); // Elf_Prop size
-  OutStreamer->emitInt32(ELF::NT_GNU_PROPERTY_TYPE_0);
-  OutStreamer->emitBytes(StringRef("GNU", 4)); // note name
-
-  // Emit the PAC/BTI properties.
-  OutStreamer->emitInt32(ELF::GNU_PROPERTY_AARCH64_FEATURE_1_AND);
-  OutStreamer->emitInt32(4);     // data size
-  OutStreamer->emitInt32(Flags); // data
-  OutStreamer->emitInt32(0);     // pad
-
-  OutStreamer->endSection(Nt);
-  OutStreamer->SwitchSection(Cur);
+  if (auto *TS = static_cast<AArch64TargetStreamer *>(
+          OutStreamer->getTargetStreamer()))
+    TS->emitNoteSection(Flags);
 }
 
 void AArch64AsmPrinter::emitFunctionHeaderComment() {
   const AArch64FunctionInfo *FI = MF->getInfo<AArch64FunctionInfo>();
   Optional<std::string> OutlinerString = FI->getOutliningStyle();
   if (OutlinerString != None)
     OutStreamer->GetCommentOS() << ' ' << OutlinerString;
 }

llvm/lib/Target/AArch64/MCTargetDesc/AArch64TargetStreamer.h

public:
   /// Add a new entry to the constant pool for the current section and return an
   /// MCExpr that can be used to refer to the constant pool location.
   const MCExpr *addConstantPoolEntry(const MCExpr *, unsigned Size, SMLoc Loc);
 
   /// Callback used to implemnt the .ltorg directive.
   /// Emit contents of constant pool for the current section.
   void emitCurrentConstantPool();
 
+  /// Callback used to implement the .note.gnu.property section.
+  void emitNoteSection(unsigned Flags);
+
   /// Callback used to implement the .inst directive.
   virtual void emitInst(uint32_t Inst);
 
   virtual void EmitARM64WinCFIAllocStack(unsigned Size) {}
   virtual void EmitARM64WinCFISaveR19R20X(int Offset) {}
   virtual void EmitARM64WinCFISaveFPLR(int Offset) {}
   virtual void EmitARM64WinCFISaveFPLRX(int Offset) {}
   virtual void EmitARM64WinCFISaveReg(unsigned Reg, int Offset) {}
   virtual void EmitARM64WinCFISaveRegX(unsigned Reg, int Offset) {}
   virtual void EmitARM64WinCFISaveRegP(unsigned Reg, int Offset) {}
   virtual void EmitARM64WinCFISaveRegPX(unsigned Reg, int Offset) {}
   virtual void EmitARM64WinCFISaveLRPair(unsigned Reg, int Offset) {}
   virtual void EmitARM64WinCFISaveFReg(unsigned Reg, int Offset) {}

llvm/lib/Target/AArch64/MCTargetDesc/AArch64TargetStreamer.cpp

 //===- AArch64TargetStreamer.cpp - AArch64TargetStreamer class ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the AArch64TargetStreamer class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "AArch64TargetStreamer.h"
+#include "AArch64MCAsmInfo.h"
+#include "AArch64Subtarget.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include "llvm/MC/ConstantPools.h"
+#include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCSection.h"
+#include "llvm/MC/MCSectionELF.h"
 #include "llvm/MC/MCSubtargetInfo.h"
+#include "llvm/Support/CommandLine.h"
 
 using namespace llvm;
 
+static cl::opt<bool> MarkBTIProperty(
+    "aarch64-mark-bti-property", cl::Hidden,
+    cl::desc("Add .note.gnu.property with BTI to assembly files"),
+    cl::init(false));
+
 //
 // AArch64TargetStreamer Implemenation
 //
 AArch64TargetStreamer::AArch64TargetStreamer(MCStreamer &S)
     : MCTargetStreamer(S), ConstantPools(new AssemblerConstantPools()) {}
 
 AArch64TargetStreamer::~AArch64TargetStreamer() = default;
 
 // The constant pool handling is shared by all AArch64TargetStreamer
 // implementations.
 const MCExpr *AArch64TargetStreamer::addConstantPoolEntry(const MCExpr *Expr,
                                                           unsigned Size,
                                                           SMLoc Loc) {
   return ConstantPools->addEntry(Streamer, Expr, Size, Loc);
 }
 
 void AArch64TargetStreamer::emitCurrentConstantPool() {
   ConstantPools->emitForCurrentSection(Streamer);
 }
 
-// finish() - write out any non-empty assembler constant pools.
-void AArch64TargetStreamer::finish() { ConstantPools->emitAll(Streamer); }
+// finish() - write out any non-empty assembler constant pools and
+//   write out note.gnu.properties if need.
+void AArch64TargetStreamer::finish() {
+  ConstantPools->emitAll(Streamer);
+
+  if (MarkBTIProperty)
+    emitNoteSection(ELF::GNU_PROPERTY_AARCH64_FEATURE_1_BTI);
+}
+
+void AArch64TargetStreamer::emitNoteSection(unsigned Flags) {
+  if (Flags == 0)
+    return;
+
+  MCStreamer &OutStreamer = getStreamer();
+  MCContext &Context = OutStreamer.getContext();
+  // Emit a .note.gnu.property section with the flags.
+  MCSectionELF *Nt = Context.getELFSection(".note.gnu.property", ELF::SHT_NOTE,
+                                           ELF::SHF_ALLOC);
+  if (Nt->isRegistered()) {
+    SMLoc Loc;
+    Context.reportWarning(
+        Loc,
+        "The .note.gnu.property is not emitted because it is already present.");
+    return;
+  }
+  MCSection *Cur = OutStreamer.getCurrentSectionOnly();
+  OutStreamer.SwitchSection(Nt);
+
+  // Emit the note header.
+  OutStreamer.emitValueToAlignment(Align(8).value());
+  OutStreamer.emitIntValue(4, 4);     // data size for "GNU\0"
+  OutStreamer.emitIntValue(4 * 4, 4); // Elf_Prop size
+  OutStreamer.emitIntValue(ELF::NT_GNU_PROPERTY_TYPE_0, 4);
+  OutStreamer.emitBytes(StringRef("GNU", 4)); // note name
+
+  // Emit the PAC/BTI properties.
+  OutStreamer.emitIntValue(ELF::GNU_PROPERTY_AARCH64_FEATURE_1_AND, 4);
+  OutStreamer.emitIntValue(4, 4);     // data size
+  OutStreamer.emitIntValue(Flags, 4); // data
+  OutStreamer.emitIntValue(0, 4);     // pad
+
+  OutStreamer.endSection(Nt);
+  OutStreamer.SwitchSection(Cur);
+}
 
 void AArch64TargetStreamer::emitInst(uint32_t Inst) {
   char Buffer[4];
 
   // We can't just use EmitIntValue here, as that will swap the
   // endianness on big-endian systems (instructions are always
   // little-endian).
   for (unsigned I = 0; I < 4; ++I) {