[GS/pacret] Also recognize traps as non-returning control flow.

A substantial amount of remaining false positives in the pacret gadget
list generated is due to the analyzer not recognizing that brk
instructions stop the regular control flow.
An example reported gadget false positive is as follows:

GS-PACRET: non-protected ret found in function __BOLT_FDE_FUNCate3bb4, basic block .Ltmp12963, at address e3ca8
  The return instruction is     000e3ca8:       ret # pacret-gadget: pac-ret-gadget<Ret:MCInstBBRef<BB:.Ltmp12963:6>, Overwriting:[MCInstBBRef<BB:.Ltmp12963:1> ]>
  The 1 instructions that write to the return register after any authentication are:
  1.     000e3c94:      bl      xmalloc@PLT
  This happens in the following basic block:
    000e3c90:   mov     x0, #0x20
    000e3c94:   bl      xmalloc@PLT
    000e3c98:   mov     x1, #0x0
    000e3c9c:   str     xzr, [x0, #0x18]
    000e3ca0:   ldr     x0, [x1, #0x128]
    000e3ca4:   brk     #0x3e8
    000e3ca8:   ret # pacret-gadget: pac-ret-gadget<Ret:MCInstBBRef<BB:.Ltmp12963:6>, Overwriting:[MCInstBBRef<BB:.Ltmp12963:1> ]>

These brk instructions are typically generated from __builtin_trap();
but might also be generated otherwise. It seems that at least gcc
understands that control flow cannot reach the point after the brk
instruction, and therefore does not generate the pac-ret authentication
instructions. For some reason though, the `ret` instruction is still
generated.

In order to not generate these false positives, BOLT needs to understand
that these "traps" end control flow; similar to as if a non-returning
function is called.

This commit implements this.

Comparing a scan of a well-populated /usr/lib64 directory on Fedora 39,
this reduce the number of reported pac-ret gadgets from 46021 to 19776 a
reduction with a factor of 2.32!
Also, the number of libraries with reported pac-ret gadgets against them
reduces from 521 to 137, a reduction with a factor 3.8!

Author: kbeyls

bolt/include/bolt/Core/MCPlusBuilder.h

@@ -1733,6 +1733,12 @@ evaluateStackOffsetExpr(const MCInst &Inst, int64_t &Output,
     return false;
   }
 
+  /// Returns true if Inst is a trap instruction.
+  virtual bool isTrap(const MCInst &Inst) const {
+    llvm_unreachable("not implemented");
+    return false;
+  }
+
   /// Creates an instruction to bump the stack pointer just like a call.
   virtual bool createStackPointerIncrement(MCInst &Inst, int Size = 8,
                                            bool NoFlagsClobber = false) const {

bolt/lib/Core/BinaryFunction.cpp

@@ -2059,7 +2059,8 @@ bool BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
       addCFIPlaceholders(Offset, InsertBB);
     }
 
-    const bool IsBlockEnd = MIB->isTerminator(Instr) || MIB->isNoReturnCall(Instr);
+    const bool IsBlockEnd = MIB->isTerminator(Instr) ||
+                            MIB->isNoReturnCall(Instr) || MIB->isTrap(Instr);
     IsLastInstrNop = MIB->isNoop(Instr);
     if (!IsLastInstrNop)
       LastInstrOffset = Offset;
@@ -2138,7 +2139,7 @@ bool BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
       //
       // Conditional tail call is a special case since we don't add a taken
       // branch successor for it.
-      if (MIB->isNoReturnCall(*LastInstr))
+      if (MIB->isNoReturnCall(*LastInstr) || MIB->isTrap(*LastInstr))
         IsPrevFT = false;
       else
         IsPrevFT = !MIB->isTerminator(*LastInstr) ||
@@ -3408,15 +3409,16 @@ void BinaryFunction::postProcessBranches() {
         continue;
 
       // If it's the basic block that does not end up with a terminator - we
-      // insert a return instruction unless it's a call instruction.
+      // insert a return instruction unless it's a call instruction or a trap.
       if (LastInstrRI == BB.rend()) {
         LLVM_DEBUG(
             dbgs() << "BOLT-DEBUG: at least one instruction expected in BB "
                    << BB.getName() << " in function " << *this << '\n');
         continue;
       }
       if (!BC.MIB->isTerminator(*LastInstrRI) &&
-          !BC.MIB->isCall(*LastInstrRI)) {
+          !BC.MIB->isCall(*LastInstrRI) &&
+          !BC.MIB->isTrap(*LastInstrRI)) {
         LLVM_DEBUG(dbgs() << "BOLT-DEBUG: adding return to basic block "
                           << BB.getName() << " in function " << *this << '\n');
         MCInst ReturnInstr;

bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

@@ -1449,6 +1449,10 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
     return true;
   }
 
+  bool isTrap(const MCInst &Inst) const override {
+    return Inst.getOpcode() == AArch64::BRK;
+  }
+
   bool convertJmpToTailCall(MCInst &Inst) override {
     setTailCall(Inst);
     return true;

bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp

@@ -219,6 +219,11 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
     return true;
   }
 
+  bool isTrap(const MCInst &Inst) const override {
+    return false;
+  }
+
+
   bool createReturn(MCInst &Inst) const override {
     // TODO "c.jr ra" when RVC is enabled
     Inst.setOpcode(RISCV::JALR);

bolt/lib/Target/X86/X86MCPlusBuilder.cpp

@@ -2763,6 +2763,10 @@ class X86MCPlusBuilder : public MCPlusBuilder {
     return true;
   }
 
+  bool isTrap(const MCInst &Inst) const override {
+    return Inst.getOpcode() == X86::TRAP;
+  }
+
   bool reverseBranchCondition(MCInst &Inst, const MCSymbol *TBB,
                               MCContext *Ctx) const override {
     unsigned InvCC = getInvertedCondCode(getCondCode(Inst));

bolt/test/gadget-scanner/gs-pacret-noreturn.s

@@ -1,7 +1,7 @@
 // Check that there are no false positives related to no-return functions.
 
 // RUN: %clang %cflags -march=armv8.3-a -mbranch-protection=pac-ret %s %p/../Inputs/asm_main.c -o %t.exe
-// RUN: llvm-bolt-gadget-scanner --scanners=pacret %t.exe --noreturnfuncs="doesnotreturn/1" 2>&1 | FileCheck -check-prefix=CHECK --allow-empty %s
+// RUN: llvm-bolt-gadget-scanner --scanners=pacret %t.exe --noreturnfuncs="doesnotreturn1/1" 2>&1 | FileCheck -check-prefix=CHECK --allow-empty %s
 
 
 // Verify that we can also detect gadgets across basic blocks
@@ -17,15 +17,44 @@ f_call_returning:
 // CHECK-NEXT:  The 1 instructions that write to the return register after any authentication are:
 // CHECK-NEXT:  1.     {{[0-9a-f]+}}:      bl call_returning
 
-        .type doesnotreturn,@function
-doesnotreturn:
+        .type doesnotreturn1,@function
+doesnotreturn1:
+        stp     x29, x30, [sp, #-16]!
+        ldp     x29, x30, [sp], #16
         brk 1
-        .size doesnotreturn, .-doesnotreturn
+        ret
+        .size doesnotreturn1, .-doesnotreturn1
+// CHECK-NOT: function doesnotreturn1
+
+        .type doesnotreturn2,@function
+doesnotreturn2:
+        cmp x0, x1
+        bgt .L1
+        bl memcpy@PLT
+.L1:
+        brk 1
+        // BOLT used to insert an artificial ret here. This test case checks that no longer happens.
+        ret
+        .size doesnotreturn2, .-doesnotreturn2
+// CHECK-NOT: function doesnotreturn2
+
+        .type gadget_entirely_in_dead_code,@function
+gadget_entirely_in_dead_code:
+        stp     x29, x30, [sp, #-16]!
+        brk 1
+        ldp     x29, x30, [sp], #16
+        ret
+        .size gadget_entirely_in_dead_code, .-gadget_entirely_in_dead_code
+// CHECK-LABEL:     GS-PACRET: non-protected ret found in function gadget_entirely_in_dead_code{{(/[0-9])?}}, basic block .L{{[^,]+}}, at address
+// CHECK-NEXT:  The return instruction is     {{[0-9a-f]+}}:       ret
+// CHECK-NEXT:  The 1 instructions that write to the return register after any authentication are:
+// CHECK-NEXT:  1.     {{[0-9a-f]+}}:      ldp     x29, x30, [sp], #0x10
+
 
         .globl f_call_noreturn
         .type   f_call_noreturn,@function
 f_call_noreturn:
-        bl      doesnotreturn
+        bl      doesnotreturn1
         ret
         .size f_call_noreturn, .-f_call_noreturn
 // CHECK-NOT: function f_call_noreturn