[GadgetScanner/pacret] Handle noreturn functions.

Author: kbeyls

bolt/include/bolt/Core/MCPlusBuilder.h

@@ -16,6 +16,7 @@
 
 #include "bolt/Core/MCPlus.h"
 #include "bolt/Core/Relocation.h"
+#include "bolt/Utils/CommandLineOpts.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/StringMap.h"
@@ -27,6 +28,7 @@
 #include "llvm/MC/MCInstrAnalysis.h"
 #include "llvm/MC/MCInstrDesc.h"
 #include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCSymbol.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -546,6 +548,27 @@ class MCPlusBuilder {
     return Analysis->isCall(Inst) || isTailCall(Inst);
   }
 
+  virtual std::optional<StringRef> getCalleeName(const MCInst &Inst) const {
+    assert(isCall(Inst));
+    if (MCPlus::getNumPrimeOperands(Inst) != 1 || !Inst.getOperand(0).isExpr())
+      return {};
+
+    const MCSymbol *CalleeSymbol = getTargetSymbol(Inst);
+    assert(CalleeSymbol != nullptr);
+    return CalleeSymbol->getName();
+  }
+
+  virtual bool isNoReturnCall(const MCInst &Inst) const {
+    if (!isCall(Inst))
+      return false;
+    auto calleeName = getCalleeName(Inst);
+    if (calleeName)
+      for (std::string &Name : opts::AssumeNoReturnFunctions)
+        if (calleeName->equals(Name))
+          return true;
+    return false;
+  }
+
   virtual bool isReturn(const MCInst &Inst) const {
     return Analysis->isReturn(Inst);
   }

bolt/include/bolt/Utils/CommandLineOpts.h

@@ -62,6 +62,13 @@ extern llvm::cl::opt<bool> TimeOpts;
 extern llvm::cl::opt<bool> UseOldText;
 extern llvm::cl::opt<bool> UpdateDebugSections;
 
+extern llvm::cl::list<std::string> AssumeNoReturnFunctions;
+extern llvm::cl::opt<std::string> AssumeNoReturnFunctionsFile;
+
+/// Reads names from FunctionNamesFile and adds them to FunctionNames.
+void populateFunctionNames(const llvm::cl::opt<std::string> &FunctionNamesFile,
+                           llvm::cl::list<std::string> &FunctionNames);
+
 // The default verbosity level (0) is pretty terse, level 1 is fairly
 // verbose and usually prints some informational message for every
 // function processed.  Level 2 is for the noisiest of messages and

bolt/lib/Core/BinaryFunction.cpp

@@ -2155,7 +2155,8 @@ Error BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
       addCFIPlaceholders(Offset, InsertBB);
     }
 
-    const bool IsBlockEnd = MIB->isTerminator(Instr);
+    const bool IsBlockEnd =
+        MIB->isTerminator(Instr) || MIB->isNoReturnCall(Instr);
     IsLastInstrNop = MIB->isNoop(Instr);
     if (!IsLastInstrNop)
       LastInstrOffset = Offset;
@@ -2242,8 +2243,11 @@ Error BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
       //
       // Conditional tail call is a special case since we don't add a taken
       // branch successor for it.
-      IsPrevFT = !MIB->isTerminator(*LastInstr) ||
-                 MIB->getConditionalTailCall(*LastInstr);
+      if (MIB->isNoReturnCall(*LastInstr))
+        IsPrevFT = false;
+      else
+        IsPrevFT = !MIB->isTerminator(*LastInstr) ||
+                   MIB->getConditionalTailCall(*LastInstr);
     } else if (BB->succ_size() == 1) {
       IsPrevFT = MIB->isConditionalBranch(*LastInstr);
     } else {

bolt/lib/Passes/BinaryPasses.cpp

@@ -1852,17 +1852,16 @@ Error InlineMemcpy::runOnFunctions(BinaryContext &BC) {
       for (auto II = BB.begin(); II != BB.end(); ++II) {
         MCInst &Inst = *II;
 
-        if (!BC.MIB->isCall(Inst) || MCPlus::getNumPrimeOperands(Inst) != 1 ||
-            !Inst.getOperand(0).isExpr())
+        if (!BC.MIB->isCall(Inst))
           continue;
-
-        const MCSymbol *CalleeSymbol = BC.MIB->getTargetSymbol(Inst);
-        if (CalleeSymbol->getName() != "memcpy" &&
-            CalleeSymbol->getName() != "memcpy@PLT" &&
-            CalleeSymbol->getName() != "_memcpy8")
+        std::optional<StringRef> CalleeName = BC.MIB->getCalleeName(Inst);
+        if (!CalleeName)
+          continue;
+        if (*CalleeName != "memcpy" && *CalleeName != "memcpy@PLT" &&
+            *CalleeName != "_memcpy8")
           continue;
 
-        const bool IsMemcpy8 = (CalleeSymbol->getName() == "_memcpy8");
+        const bool IsMemcpy8 = (*CalleeName == "_memcpy8");
         const bool IsTailCall = BC.MIB->isTailCall(Inst);
 
         const InstructionListType NewCode =
@@ -1951,13 +1950,12 @@ Error SpecializeMemcpy1::runOnFunctions(BinaryContext &BC) {
       for (auto II = CurBB->begin(); II != CurBB->end(); ++II) {
         MCInst &Inst = *II;
 
-        if (!BC.MIB->isCall(Inst) || MCPlus::getNumPrimeOperands(Inst) != 1 ||
-            !Inst.getOperand(0).isExpr())
+        if (!BC.MIB->isCall(Inst))
           continue;
-
-        const MCSymbol *CalleeSymbol = BC.MIB->getTargetSymbol(Inst);
-        if (CalleeSymbol->getName() != "memcpy" &&
-            CalleeSymbol->getName() != "memcpy@PLT")
+        std::optional<StringRef> CalleeName = BC.MIB->getCalleeName(Inst);
+        if (!CalleeName)
+          continue;
+        if (*CalleeName != "memcpy" && *CalleeName != "memcpy@PLT")
           continue;
 
         if (BC.MIB->isTailCall(Inst))

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -269,6 +269,9 @@ MCPlusBuilder *createMCPlusBuilder(const Triple::ArchType Arch,
                                    const MCInstrInfo *Info,
                                    const MCRegisterInfo *RegInfo,
                                    const MCSubtargetInfo *STI) {
+  opts::populateFunctionNames(opts::AssumeNoReturnFunctionsFile,
+                              opts::AssumeNoReturnFunctions);
+
 #ifdef X86_AVAILABLE
   if (Arch == Triple::x86_64)
     return createX86MCPlusBuilder(Analysis, Info, RegInfo, STI);
@@ -2929,18 +2932,12 @@ void RewriteInstance::handleRelocation(const SectionRef &RelocatedSection,
 
 void RewriteInstance::selectFunctionsToProcess() {
   // Extend the list of functions to process or skip from a file.
-  auto populateFunctionNames = [](cl::opt<std::string> &FunctionNamesFile,
-                                  cl::list<std::string> &FunctionNames) {
-    if (FunctionNamesFile.empty())
-      return;
-    std::ifstream FuncsFile(FunctionNamesFile, std::ios::in);
-    std::string FuncName;
-    while (std::getline(FuncsFile, FuncName))
-      FunctionNames.push_back(FuncName);
-  };
-  populateFunctionNames(opts::FunctionNamesFile, opts::ForceFunctionNames);
-  populateFunctionNames(opts::SkipFunctionNamesFile, opts::SkipFunctionNames);
-  populateFunctionNames(opts::FunctionNamesFileNR, opts::ForceFunctionNamesNR);
+  opts::populateFunctionNames(opts::FunctionNamesFile,
+                              opts::ForceFunctionNames);
+  opts::populateFunctionNames(opts::SkipFunctionNamesFile,
+                              opts::SkipFunctionNames);
+  opts::populateFunctionNames(opts::FunctionNamesFileNR,
+                              opts::ForceFunctionNamesNR);
 
   // Make a set of functions to process to speed up lookups.
   std::unordered_set<std::string> ForceFunctionsNR(

bolt/lib/Utils/CommandLineOpts.cpp

@@ -11,7 +11,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "bolt/Utils/CommandLineOpts.h"
-#include "VCSVersion.inc"
+#include "llvm/Support/VCSRevision.h"
+#include <fstream>
 
 using namespace llvm;
 
@@ -206,11 +207,31 @@ cl::opt<bool> UpdateDebugSections(
     cl::desc("update DWARF debug sections of the executable"),
     cl::cat(BoltCategory));
 
+cl::list<std::string> AssumeNoReturnFunctions(
+    "noreturnfuncs", cl::CommaSeparated,
+    cl::desc("List which function names to assume are no-return"),
+    cl::value_desc("func1,func2,func3,..."), cl::Hidden, cl::cat(BoltCategory));
+
+cl::opt<std::string> AssumeNoReturnFunctionsFile(
+    "noreturnfuncs-file",
+    cl::desc("file with list of functions to assume are no-return"), cl::Hidden,
+    cl::cat(BoltCategory));
+
 cl::opt<unsigned>
     Verbosity("v", cl::desc("set verbosity level for diagnostic output"),
               cl::init(0), cl::ZeroOrMore, cl::cat(BoltCategory),
               cl::sub(cl::SubCommand::getAll()));
 
+void populateFunctionNames(const cl::opt<std::string> &FunctionNamesFile,
+                           cl::list<std::string> &FunctionNames) {
+  if (FunctionNamesFile.empty())
+    return;
+  std::ifstream FuncsFile(FunctionNamesFile, std::ios::in);
+  std::string FuncName;
+  while (std::getline(FuncsFile, FuncName))
+    FunctionNames.push_back(FuncName);
+}
+
 bool processAllFunctions() {
   if (opts::AggregateOnly)
     return false;

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

@@ -0,0 +1,31 @@
+// 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 %t.exe --noreturnfuncs="doesnotreturn/1" 2>&1 | FileCheck -check-prefix=CHECK --allow-empty %s
+
+
+// Verify that we can also detect gadgets across basic blocks
+
+        .globl f_call_returning
+        .type   f_call_returning,@function
+f_call_returning:
+        bl      call_returning
+        ret
+        .size f_call_returning, .-f_call_returning
+// CHECK-LABEL:     GS-PACRET: non-protected ret found in function f_call_returning, 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]+}}:      bl call_returning
+
+        .type doesnotreturn,@function
+doesnotreturn:
+        brk 1
+        .size doesnotreturn, .-doesnotreturn
+
+        .globl f_call_noreturn
+        .type   f_call_noreturn,@function
+f_call_noreturn:
+        bl      doesnotreturn
+        ret
+        .size f_call_noreturn, .-f_call_noreturn
+// CHECK-NOT: function f_call_noreturn