[llvm] 22467e2 - Add function attribute "patchable-function-prefix" to support -fpatchable-function-entry=N, M where M>0

Fangrui Song via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 23 17:03:05 PST 2020


Author: Fangrui Song
Date: 2020-01-23T17:02:27-08:00
New Revision: 22467e259507f5ead2a87d989251b4c951a587e4

URL: https://github.com/llvm/llvm-project/commit/22467e259507f5ead2a87d989251b4c951a587e4
DIFF: https://github.com/llvm/llvm-project/commit/22467e259507f5ead2a87d989251b4c951a587e4.diff

LOG: Add function attribute "patchable-function-prefix" to support -fpatchable-function-entry=N,M where M>0

Similar to the function attribute `prefix` (prefix data),
"patchable-function-prefix" inserts data (M NOPs) before the function
entry label.

-fpatchable-function-entry=2,1 (1 NOP before entry, 1 NOP after entry)
will look like:

```
  .type	foo, at function
.Ltmp0:               # @foo
  nop
foo:
.Lfunc_begin0:
  # optional `bti c` (AArch64 Branch Target Identification) or
  # `endbr64` (Intel Indirect Branch Tracking)
  nop

  .section  __patchable_function_entries,"awo", at progbits,get,unique,0
  .p2align  3
  .quad .Ltmp0
```

-fpatchable-function-entry=N,0 + -mbranch-protection=bti/-fcf-protection=branch has two reasonable
placements (https://gcc.gnu.org/ml/gcc-patches/2020-01/msg01185.html):

```
(a)         (b)

func:       func:
.Ltmp0:     bti c
  bti c     .Ltmp0:
  nop       nop
```

(a) needs no additional code. If the consensus is to go for (b), we will
need more code in AArch64BranchTargets.cpp / X86IndirectBranchTracking.cpp .

Differential Revision: https://reviews.llvm.org/D73070

Added: 
    

Modified: 
    llvm/include/llvm/CodeGen/AsmPrinter.h
    llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
    llvm/lib/IR/Verifier.cpp
    llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
    llvm/lib/Target/ARM/ARMMCInstLower.cpp
    llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
    llvm/test/CodeGen/AArch64/patchable-function-entry.ll
    llvm/test/Verifier/invalid-patchable-function-entry.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/CodeGen/AsmPrinter.h b/llvm/include/llvm/CodeGen/AsmPrinter.h
index b374fd3d80af..a860ce2773e1 100644
--- a/llvm/include/llvm/CodeGen/AsmPrinter.h
+++ b/llvm/include/llvm/CodeGen/AsmPrinter.h
@@ -113,6 +113,9 @@ class AsmPrinter : public MachineFunctionPass {
 
   ProfileSummaryInfo *PSI;
 
+  /// The symbol for the entry in __patchable_function_entires.
+  MCSymbol *CurrentPatchableFunctionEntrySym = nullptr;
+
   /// The symbol for the current function. This is recalculated at the beginning
   /// of each call to runOnMachineFunction().
   MCSymbol *CurrentFnSym = nullptr;
@@ -449,6 +452,9 @@ class AsmPrinter : public MachineFunctionPass {
   /// instructions in verbose mode.
   virtual void emitImplicitDef(const MachineInstr *MI) const;
 
+  /// Emit N NOP instructions.
+  void emitNops(unsigned N);
+
   //===------------------------------------------------------------------===//
   // Symbol Lowering Routines.
   //===------------------------------------------------------------------===//

diff  --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
index 4d2bcd1ed57c..83e6ef5fa8ac 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
@@ -706,6 +706,21 @@ void AsmPrinter::EmitFunctionHeader() {
     }
   }
 
+  // Emit M NOPs for -fpatchable-function-entry=N,M where M>0. We arbitrarily
+  // place prefix data before NOPs.
+  unsigned PatchableFunctionPrefix = 0;
+  (void)F.getFnAttribute("patchable-function-prefix")
+      .getValueAsString()
+      .getAsInteger(10, PatchableFunctionPrefix);
+  if (PatchableFunctionPrefix) {
+    CurrentPatchableFunctionEntrySym =
+        OutContext.createLinkerPrivateTempSymbol();
+    OutStreamer->EmitLabel(CurrentPatchableFunctionEntrySym);
+    emitNops(PatchableFunctionPrefix);
+  } else {
+    CurrentPatchableFunctionEntrySym = CurrentFnBegin;
+  }
+
   // Emit the function descriptor. This is a virtual function to allow targets
   // to emit their specific function descriptor.
   if (MAI->needsFunctionDescriptors())
@@ -1157,7 +1172,7 @@ void AsmPrinter::EmitFunctionBody() {
     // unspecified.
     if (Noop.getOpcode()) {
       OutStreamer->AddComment("avoids zero-length function");
-      OutStreamer->EmitInstruction(Noop, getSubtargetInfo());
+      emitNops(1);
     }
   }
 
@@ -2787,6 +2802,13 @@ void AsmPrinter::printOffset(int64_t Offset, raw_ostream &OS) const {
     OS << Offset;
 }
 
+void AsmPrinter::emitNops(unsigned N) {
+  MCInst Nop;
+  MF->getSubtarget().getInstrInfo()->getNoop(Nop);
+  for (; N; --N)
+    EmitToStreamer(*OutStreamer, Nop);
+}
+
 //===----------------------------------------------------------------------===//
 // Symbol Lowering Routines.
 //===----------------------------------------------------------------------===//
@@ -3189,11 +3211,14 @@ void AsmPrinter::recordSled(MCSymbol *Sled, const MachineInstr &MI,
 
 void AsmPrinter::emitPatchableFunctionEntries() {
   const Function &F = MF->getFunction();
-  unsigned PatchableFunctionEntry = 0;
+  unsigned PatchableFunctionPrefix = 0, PatchableFunctionEntry = 0;
+  (void)F.getFnAttribute("patchable-function-prefix")
+      .getValueAsString()
+      .getAsInteger(10, PatchableFunctionPrefix);
   (void)F.getFnAttribute("patchable-function-entry")
       .getValueAsString()
       .getAsInteger(10, PatchableFunctionEntry);
-  if (!PatchableFunctionEntry)
+  if (!PatchableFunctionPrefix && !PatchableFunctionEntry)
     return;
   const unsigned PointerSize = getPointerSize();
   if (TM.getTargetTriple().isOSBinFormatELF()) {
@@ -3222,7 +3247,7 @@ void AsmPrinter::emitPatchableFunctionEntries() {
           "__patchable_function_entries", ELF::SHT_PROGBITS, Flags));
     }
     EmitAlignment(Align(PointerSize));
-    OutStreamer->EmitSymbolValue(CurrentFnBegin, PointerSize);
+    OutStreamer->EmitSymbolValue(CurrentPatchableFunctionEntrySym, PointerSize);
   }
 }
 

diff  --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index d15b70d71b47..61707cc83504 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -1852,16 +1852,25 @@ void Verifier::verifyFunctionAttrs(FunctionType *FT, AttributeList Attrs,
       CheckFailed("invalid value for 'frame-pointer' attribute: " + FP, V);
   }
 
+  if (Attrs.hasFnAttribute("patchable-function-prefix")) {
+    StringRef S = Attrs
+                      .getAttribute(AttributeList::FunctionIndex,
+                                    "patchable-function-prefix")
+                      .getValueAsString();
+    unsigned N;
+    if (S.getAsInteger(10, N))
+      CheckFailed(
+          "\"patchable-function-prefix\" takes an unsigned integer: " + S, V);
+  }
   if (Attrs.hasFnAttribute("patchable-function-entry")) {
-    StringRef S0 = Attrs
-                       .getAttribute(AttributeList::FunctionIndex,
-                                     "patchable-function-entry")
-                       .getValueAsString();
-    StringRef S = S0;
+    StringRef S = Attrs
+                      .getAttribute(AttributeList::FunctionIndex,
+                                    "patchable-function-entry")
+                      .getValueAsString();
     unsigned N;
     if (S.getAsInteger(10, N))
       CheckFailed(
-          "\"patchable-function-entry\" takes an unsigned integer: " + S0, V);
+          "\"patchable-function-entry\" takes an unsigned integer: " + S, V);
   }
 }
 

diff  --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index 00e321f9b850..b8953583a310 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -250,8 +250,7 @@ void AArch64AsmPrinter::LowerPATCHABLE_FUNCTION_ENTER(const MachineInstr &MI)
             .getValueAsString()
             .getAsInteger(10, Num))
       return;
-    for (; Num; --Num)
-      EmitToStreamer(*OutStreamer, MCInstBuilder(AArch64::HINT).addImm(0));
+    emitNops(Num);
     return;
   }
 

diff  --git a/llvm/lib/Target/ARM/ARMMCInstLower.cpp b/llvm/lib/Target/ARM/ARMMCInstLower.cpp
index c92689f4942e..8e01b998d900 100644
--- a/llvm/lib/Target/ARM/ARMMCInstLower.cpp
+++ b/llvm/lib/Target/ARM/ARMMCInstLower.cpp
@@ -207,10 +207,7 @@ void ARMAsmPrinter::EmitSled(const MachineInstr &MI, SledKind Kind)
   EmitToStreamer(*OutStreamer, MCInstBuilder(ARM::Bcc).addImm(20)
     .addImm(ARMCC::AL).addReg(0));
 
-  MCInst Noop;
-  Subtarget->getInstrInfo()->getNoop(Noop);
-  for (int8_t I = 0; I < NoopsInSledCount; I++)
-    OutStreamer->EmitInstruction(Noop, getSubtargetInfo());
+  emitNops(NoopsInSledCount);
 
   OutStreamer->EmitLabel(Target);
   recordSled(CurSled, MI, Kind);

diff  --git a/llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll b/llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
index 00b9b2329fd5..8d889a73f9f5 100644
--- a/llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
+++ b/llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll
@@ -1,23 +1,43 @@
 ; RUN: llc -mtriple=aarch64 %s -o - | FileCheck --check-prefixes=CHECK %s
 
-define i32 @f0() "patchable-function-entry"="0" "branch-target-enforcement" {
+define void @f0() "patchable-function-entry"="0" "branch-target-enforcement" {
 ; CHECK-LABEL: f0:
-; CHECK-NEXT:  .Lfunc_begin0:
-; CHECK:       %bb.0:
+; CHECK-NEXT: .Lfunc_begin0:
+; CHECK:      // %bb.0:
 ; CHECK-NEXT:  hint #34
-; CHECK-NEXT:  mov w0, wzr
-; CHECK-NOT:   .section __patchable_function_entries
-  ret i32 0
+; CHECK-NEXT:  ret
+; CHECK-NOT:  .section __patchable_function_entries
+  ret void
 }
 
-define i32 @f1() "patchable-function-entry"="1" "branch-target-enforcement" {
+;; -fpatchable-function-entry=1 -mbranch-protection=bti
+define void @f1() "patchable-function-entry"="1" "branch-target-enforcement" {
 ; CHECK-LABEL: f1:
-; CHECK-NEXT:  .Lfunc_begin1:
+; CHECK-NEXT: .Lfunc_begin1:
 ; CHECK:       hint #34
 ; CHECK-NEXT:  nop
-; CHECK-NEXT:  mov w0, wzr
-; CHECK:       .section __patchable_function_entries,"awo", at progbits,f1,unique,0
-; CHECK-NEXT:  .p2align 3
-; CHECK-NEXT:  .xword .Lfunc_begin1
-  ret i32 0
+; CHECK-NEXT:  ret
+; CHECK:      .section __patchable_function_entries,"awo", at progbits,f1,unique,0
+; CHECK-NEXT: .p2align 3
+; CHECK-NEXT: .xword .Lfunc_begin1
+  ret void
+}
+
+;; -fpatchable-function-entry=2,1 -mbranch-protection=bti
+define void @f2_1() "patchable-function-entry"="1" "patchable-function-prefix"="1" "branch-target-enforcement" {
+; CHECK-LABEL: .type f2_1, at function
+; CHECK-NEXT: .Ltmp0:
+; CHECK-NEXT:  nop
+; CHECK-NEXT: f2_1:
+; CHECK-NEXT: .Lfunc_begin2:
+; CHECK:      // %bb.0:
+; CHECK-NEXT:  hint #34
+; CHECK-NEXT:  nop
+; CHECK-NEXT:  ret
+; CHECK:      .Lfunc_end2:
+; CHECK-NEXT: .size f2_1, .Lfunc_end2-f2_1
+; CHECK:      .section __patchable_function_entries,"awo", at progbits,f1,unique,0
+; CHECK-NEXT: .p2align 3
+; CHECK-NEXT: .xword .Ltmp0
+  ret void
 }

diff  --git a/llvm/test/CodeGen/AArch64/patchable-function-entry.ll b/llvm/test/CodeGen/AArch64/patchable-function-entry.ll
index 6d84f23ba5ef..5ae9d88a2896 100644
--- a/llvm/test/CodeGen/AArch64/patchable-function-entry.ll
+++ b/llvm/test/CodeGen/AArch64/patchable-function-entry.ll
@@ -25,6 +25,10 @@ define i32 @f1() "patchable-function-entry"="1" {
   ret i32 0
 }
 
+;; Without -function-sections, f2 is in the same text section as f1.
+;; They share the __patchable_function_entries section.
+;; With -function-sections, f1 and f2 are in 
diff erent text sections.
+;; Use separate __patchable_function_entries.
 define void @f2() "patchable-function-entry"="2" {
 ; CHECK-LABEL: f2:
 ; CHECK-NEXT: .Lfunc_begin2:
@@ -63,3 +67,39 @@ define void @f5() "patchable-function-entry"="5" comdat {
   %frame = alloca i8, i32 16
   ret void
 }
+
+;; -fpatchable-function-entry=3,2
+;; "patchable-function-prefix" emits data before the function entry label.
+define void @f3_2() "patchable-function-entry"="1" "patchable-function-prefix"="2" {
+; CHECK-LABEL: .type f3_2, at function
+; CHECK-NEXT: .Ltmp1: // @f3_2
+; CHECK-NEXT:  nop
+; CHECK-NEXT:  nop
+; CHECK-NEXT: f3_2:
+; CHECK:      // %bb.0:
+; CHECK-NEXT:  nop
+; CHECK-NEXT:  ret
+;; .size does not include the prefix.
+; CHECK:      .Lfunc_end5:
+; CHECK-NEXT: .size f3_2, .Lfunc_end5-f3_2
+; NOFSECT     .section __patchable_function_entries,"awo", at progbits,f1,unique,0
+; FSECT:      .section __patchable_function_entries,"awo", at progbits,f3_2,unique,4
+; CHECK:      .p2align 3
+; CHECK-NEXT: .xword .Ltmp1
+  ret void
+}
+
+;; When prefix data is used, arbitrarily place NOPs after prefix data.
+define void @prefix() "patchable-function-entry"="0" "patchable-function-prefix"="1" prefix i32 1 {
+; CHECK-LABEL: .type prefix, at function
+; CHECK-NEXT: .word 1 // @prefix
+; CHECK:      .Ltmp2:
+; CHECK:       nop
+; CHECK-NEXT: prefix:
+;; Emit a __patchable_function_entries entry even if "patchable-function-entry" is 0.
+; NOFSECT     .section __patchable_function_entries,"awo", at progbits,prefix,unique,0
+; FSECT:      .section __patchable_function_entries,"awo", at progbits,prefix,unique,5
+; CHECK:      .p2align 3
+; CHECK-NEXT: .xword .Ltmp2
+  ret void
+}

diff  --git a/llvm/test/Verifier/invalid-patchable-function-entry.ll b/llvm/test/Verifier/invalid-patchable-function-entry.ll
index bd70ce0fc046..e74037a28abe 100644
--- a/llvm/test/Verifier/invalid-patchable-function-entry.ll
+++ b/llvm/test/Verifier/invalid-patchable-function-entry.ll
@@ -9,3 +9,13 @@ define void @f() "patchable-function-entry" { ret void }
 define void @fa() "patchable-function-entry"="a" { ret void }
 define void @f_1() "patchable-function-entry"="-1" { ret void }
 define void @f3comma() "patchable-function-entry"="3," { ret void }
+
+; CHECK: "patchable-function-prefix" takes an unsigned integer:
+; CHECK: "patchable-function-prefix" takes an unsigned integer: a
+; CHECK: "patchable-function-prefix" takes an unsigned integer: -1
+; CHECK: "patchable-function-prefix" takes an unsigned integer: 3,
+
+define void @g() "patchable-function-prefix" { ret void }
+define void @ga() "patchable-function-prefix"="a" { ret void }
+define void @g_1() "patchable-function-prefix"="-1" { ret void }
+define void @g3comma() "patchable-function-prefix"="3," { ret void }


        


More information about the llvm-commits mailing list