Merge remote-tracking branch 'origin/main' into vplan-all-regions-as-transform

Author: fhahn

.ci/compute_projects.py

@@ -200,6 +200,11 @@ def _get_modified_projects(modified_files: list[str]) -> Set[str]:
         # documentation builds.
         if len(path_parts) > 2 and path_parts[1] == "docs":
             continue
+        # Exclude files for the gn build. We do not test it within premerge
+        # and changes occur often enough that they otherwise take up
+        # capacity.
+        if len(path_parts) > 3 and path_parts[:3] == ("llvm", "utils", "gn"):
+            continue
         modified_projects.add(pathlib.Path(modified_file).parts[0])
     return modified_projects
 

.ci/compute_projects_test.py

@@ -179,6 +179,15 @@ def test_exclude_docs(self):
         self.assertEqual(env_variables["runtimes_to_build"], "")
         self.assertEqual(env_variables["runtimes_check_targets"], "")
 
+    def test_exclude_gn(self):
+        env_variables = compute_projects.get_env_variables(
+            ["llvm/utils/gn/build/BUILD.gn"], "Linux"
+        )
+        self.assertEqual(env_variables["projects_to_build"], "")
+        self.assertEqual(env_variables["project_check_targets"], "")
+        self.assertEqual(env_variables["runtimes_to_build"], "")
+        self.assertEqual(env_variables["runtimes_check_targets"], "")
+
 
 if __name__ == "__main__":
     unittest.main()

.github/workflows/libclang-abi-tests.yml

@@ -104,7 +104,7 @@ jobs:
       - name: Install abi-compliance-checker
         run: |
           sudo apt-get update
-          sudo apt-get install abi-dumper autoconf pkg-config
+          sudo apt-get install -y abi-dumper autoconf pkg-config
       - name: Install universal-ctags
         run: |
           git clone https://github.com/universal-ctags/ctags.git
@@ -157,7 +157,7 @@ jobs:
       - name: Install abi-compliance-checker
         run: |
           sudo apt-get update
-          sudo apt-get install abi-compliance-checker
+          sudo apt-get install -y abi-compliance-checker
       - name: Compare ABI
         run: |
           for lib in ${{ needs.abi-dump-setup.outputs.ABI_LIBS }}; do

.github/workflows/llvm-tests.yml

@@ -92,7 +92,7 @@ jobs:
       - name: Install abi-compliance-checker
         run: |
           sudo apt-get update
-          sudo apt-get install abi-dumper autoconf pkg-config
+          sudo apt-get -y install abi-dumper autoconf pkg-config
       - name: Install universal-ctags
         run: |
           git clone https://github.com/universal-ctags/ctags.git
@@ -166,7 +166,7 @@ jobs:
       - name: Install abi-compliance-checker
         run: |
           sudo apt-get update
-          sudo apt-get install abi-compliance-checker
+          sudo apt-get -y install abi-compliance-checker
       - name: Compare ABI
         run: |
           if [ -s symbol-list/llvm.symbols ]; then

bolt/include/bolt/Core/MCPlusBuilder.h

@@ -577,6 +577,16 @@ class MCPlusBuilder {
     return getNoRegister();
   }
 
+  /// Returns the register used as call destination, or no-register, if not
+  /// an indirect call. Sets IsAuthenticatedInternally if the instruction
+  /// accepts a signed pointer as its operand and authenticates it internally.
+  virtual MCPhysReg
+  getRegUsedAsCallDest(const MCInst &Inst,
+                       bool &IsAuthenticatedInternally) const {
+    llvm_unreachable("not implemented");
+    return getNoRegister();
+  }
+
   virtual bool isTerminator(const MCInst &Inst) const;
 
   virtual bool isNoop(const MCInst &Inst) const {
@@ -1266,7 +1276,11 @@ class MCPlusBuilder {
 
   /// Return MCSymbol extracted from the expression.
   virtual const MCSymbol *getTargetSymbol(const MCExpr *Expr) const {
-    if (auto *SymbolRefExpr = dyn_cast<const MCSymbolRefExpr>(Expr))
+    if (auto *BinaryExpr = dyn_cast<const MCBinaryExpr>(Expr))
+      return getTargetSymbol(BinaryExpr->getLHS());
+
+    auto *SymbolRefExpr = dyn_cast<const MCSymbolRefExpr>(Expr);
+    if (SymbolRefExpr && SymbolRefExpr->getKind() == MCSymbolRefExpr::VK_None)
       return &SymbolRefExpr->getSymbol();
 
     return nullptr;
@@ -1737,6 +1751,15 @@ class MCPlusBuilder {
     return {};
   }
 
+  /// Create a sequence of instructions to compare contents of a register
+  /// \p RegNo to immediate \Imm and jump to \p Target if they are different.
+  virtual InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm,
+                                           const MCSymbol *Target,
+                                           MCContext *Ctx) const {
+    llvm_unreachable("not implemented");
+    return {};
+  }
+
   /// Creates inline memcpy instruction. If \p ReturnEnd is true, then return
   /// (dest + n) instead of dest.
   virtual InstructionListType createInlineMemcpy(bool ReturnEnd) const {

bolt/include/bolt/Passes/PAuthGadgetScanner.h

@@ -248,6 +248,9 @@ struct FunctionAnalysisResult {
 };
 
 class Analysis : public BinaryFunctionPass {
+  /// Only search for pac-ret violations.
+  bool PacRetGadgetsOnly;
+
   void runOnFunction(BinaryFunction &Function,
                      MCPlusBuilder::AllocatorIdTy AllocatorId);
   FunctionAnalysisResult findGadgets(BinaryFunction &BF,
@@ -261,7 +264,8 @@ class Analysis : public BinaryFunctionPass {
   std::mutex AnalysisResultsMutex;
 
 public:
-  explicit Analysis() : BinaryFunctionPass(false) {}
+  explicit Analysis(bool PacRetGadgetsOnly)
+      : BinaryFunctionPass(false), PacRetGadgetsOnly(PacRetGadgetsOnly) {}
 
   const char *getName() const override { return "pauth-gadget-scanner"; }
 

bolt/include/bolt/Utils/CommandLineOpts.h

@@ -81,9 +81,9 @@ extern llvm::cl::opt<unsigned> Verbosity;
 /// Return true if we should process all functions in the binary.
 bool processAllFunctions();
 
-enum GadgetScannerKind { GS_PACRET, GS_ALL };
+enum GadgetScannerKind { GS_PACRET, GS_PAUTH, GS_ALL };
 
-extern llvm::cl::list<GadgetScannerKind> GadgetScannersToRun;
+extern llvm::cl::bits<GadgetScannerKind> GadgetScannersToRun;
 
 } // namespace opts
 

bolt/lib/Core/BinaryFunction.cpp

@@ -1219,6 +1219,11 @@ void BinaryFunction::handleIndirectBranch(MCInst &Instruction, uint64_t Size,
   case IndirectBranchType::UNKNOWN:
     // Keep processing. We'll do more checks and fixes in
     // postProcessIndirectBranches().
+    if (opts::Verbosity > 2) {
+      outs() << "BOLT-WARNING: failed to match indirect branch, "
+             << getPrintName() << " at 0x" << Twine::utohexstr(Offset)
+             << " offset\n";
+    }
     UnknownIndirectBranchOffsets.emplace(Offset);
     break;
   }

bolt/lib/Passes/PAuthGadgetScanner.cpp

@@ -401,11 +401,11 @@ class PacRetAnalysis
 
 public:
   std::vector<MCInstReference>
-  getLastClobberingInsts(const MCInst Ret, BinaryFunction &BF,
-                         const ArrayRef<MCPhysReg> UsedDirtyRegs) const {
+  getLastClobberingInsts(const MCInst &Inst, BinaryFunction &BF,
+                         const ArrayRef<MCPhysReg> UsedDirtyRegs) {
     if (RegsToTrackInstsFor.empty())
       return {};
-    auto MaybeState = getStateAt(Ret);
+    auto MaybeState = getStateBefore(Inst);
     if (!MaybeState)
       llvm_unreachable("Expected State to be present");
     const State &S = *MaybeState;
@@ -453,6 +453,29 @@ shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst,
   return std::make_shared<GadgetReport>(RetKind, Inst, RetReg);
 }
 
+static std::shared_ptr<Report>
+shouldReportCallGadget(const BinaryContext &BC, const MCInstReference &Inst,
+                       const State &S) {
+  static const GadgetKind CallKind("non-protected call found");
+  if (!BC.MIB->isCall(Inst) && !BC.MIB->isBranch(Inst))
+    return nullptr;
+
+  bool IsAuthenticated = false;
+  MCPhysReg DestReg = BC.MIB->getRegUsedAsCallDest(Inst, IsAuthenticated);
+  if (IsAuthenticated || DestReg == BC.MIB->getNoRegister())
+    return nullptr;
+
+  LLVM_DEBUG({
+    traceInst(BC, "Found call inst", Inst);
+    traceReg(BC, "Call destination reg", DestReg);
+    traceRegMask(BC, "SafeToDerefRegs", S.SafeToDerefRegs);
+  });
+  if (S.SafeToDerefRegs[DestReg])
+    return nullptr;
+
+  return std::make_shared<GadgetReport>(CallKind, Inst, DestReg);
+}
+
 FunctionAnalysisResult
 Analysis::findGadgets(BinaryFunction &BF,
                       MCPlusBuilder::AllocatorIdTy AllocatorId) {
@@ -469,7 +492,7 @@ Analysis::findGadgets(BinaryFunction &BF,
   for (BinaryBasicBlock &BB : BF) {
     for (int64_t I = 0, E = BB.size(); I < E; ++I) {
       MCInstReference Inst(&BB, I);
-      const State &S = *PRA.getStateAt(Inst);
+      const State &S = *PRA.getStateBefore(Inst);
 
       // If non-empty state was never propagated from the entry basic block
       // to Inst, assume it to be unreachable and report a warning.
@@ -481,6 +504,12 @@ Analysis::findGadgets(BinaryFunction &BF,
 
       if (auto Report = shouldReportReturnGadget(BC, Inst, S))
         Result.Diagnostics.push_back(Report);
+
+      if (PacRetGadgetsOnly)
+        continue;
+
+      if (auto Report = shouldReportCallGadget(BC, Inst, S))
+        Result.Diagnostics.push_back(Report);
     }
   }
   return Result;

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -247,12 +247,14 @@ static cl::opt<bool> WriteBoltInfoSection(
     "bolt-info", cl::desc("write bolt info section in the output binary"),
     cl::init(true), cl::Hidden, cl::cat(BoltOutputCategory));
 
-cl::list<GadgetScannerKind>
-    GadgetScannersToRun("scanners", cl::desc("which gadget scanners to run"),
-                        cl::values(clEnumValN(GS_PACRET, "pacret", "pac-ret"),
-                                   clEnumValN(GS_ALL, "all", "all")),
-                        cl::ZeroOrMore, cl::CommaSeparated,
-                        cl::cat(BinaryAnalysisCategory));
+cl::bits<GadgetScannerKind> GadgetScannersToRun(
+    "scanners", cl::desc("which gadget scanners to run"),
+    cl::values(
+        clEnumValN(GS_PACRET, "pacret",
+                   "pac-ret: return address protection (subset of \"pauth\")"),
+        clEnumValN(GS_PAUTH, "pauth", "All Pointer Authentication scanners"),
+        clEnumValN(GS_ALL, "all", "All implemented scanners")),
+    cl::ZeroOrMore, cl::CommaSeparated, cl::cat(BinaryAnalysisCategory));
 
 } // namespace opts
 
@@ -3539,12 +3541,18 @@ void RewriteInstance::runBinaryAnalyses() {
   // FIXME: add a pass that warns about which functions do not have CFG,
   // and therefore, analysis is most likely to be less accurate.
   using GSK = opts::GadgetScannerKind;
-  // if no command line option was given, act as if "all" was specified.
-  if (opts::GadgetScannersToRun.empty())
-    opts::GadgetScannersToRun.addValue(GSK::GS_ALL);
-  for (GSK ScannerToRun : opts::GadgetScannersToRun) {
-    if (ScannerToRun == GSK::GS_PACRET || ScannerToRun == GSK::GS_ALL)
-      Manager.registerPass(std::make_unique<PAuthGadgetScanner::Analysis>());
+  using PAuthScanner = PAuthGadgetScanner::Analysis;
+
+  // If no command line option was given, act as if "all" was specified.
+  bool RunAll = !opts::GadgetScannersToRun.getBits() ||
+                opts::GadgetScannersToRun.isSet(GSK::GS_ALL);
+
+  if (RunAll || opts::GadgetScannersToRun.isSet(GSK::GS_PAUTH)) {
+    Manager.registerPass(
+        std::make_unique<PAuthScanner>(/*OnlyPacRetChecks=*/false));
+  } else if (RunAll || opts::GadgetScannersToRun.isSet(GSK::GS_PACRET)) {
+    Manager.registerPass(
+        std::make_unique<PAuthScanner>(/*OnlyPacRetChecks=*/true));
   }
 
   BC->logBOLTErrorsAndQuitOnFatal(Manager.runPasses());

bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

@@ -277,6 +277,33 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
     }
   }
 
+  MCPhysReg
+  getRegUsedAsCallDest(const MCInst &Inst,
+                       bool &IsAuthenticatedInternally) const override {
+    assert(isCall(Inst) || isBranch(Inst));
+    IsAuthenticatedInternally = false;
+
+    switch (Inst.getOpcode()) {
+    case AArch64::BR:
+    case AArch64::BLR:
+      return Inst.getOperand(0).getReg();
+    case AArch64::BRAA:
+    case AArch64::BRAB:
+    case AArch64::BRAAZ:
+    case AArch64::BRABZ:
+    case AArch64::BLRAA:
+    case AArch64::BLRAB:
+    case AArch64::BLRAAZ:
+    case AArch64::BLRABZ:
+      IsAuthenticatedInternally = true;
+      return Inst.getOperand(0).getReg();
+    default:
+      if (isIndirectCall(Inst) || isIndirectBranch(Inst))
+        llvm_unreachable("Unhandled indirect branch");
+      return getNoRegister();
+    }
+  }
+
   bool isADRP(const MCInst &Inst) const override {
     return Inst.getOpcode() == AArch64::ADRP;
   }
@@ -862,20 +889,12 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
     if (AArchExpr && AArchExpr->getSubExpr())
       return getTargetSymbol(AArchExpr->getSubExpr());
 
-    auto *BinExpr = dyn_cast<MCBinaryExpr>(Expr);
-    if (BinExpr)
-      return getTargetSymbol(BinExpr->getLHS());
-
-    auto *SymExpr = dyn_cast<MCSymbolRefExpr>(Expr);
-    if (SymExpr && SymExpr->getKind() == MCSymbolRefExpr::VK_None)
-      return &SymExpr->getSymbol();
-
-    return nullptr;
+    return MCPlusBuilder::getTargetSymbol(Expr);
   }
 
   const MCSymbol *getTargetSymbol(const MCInst &Inst,
                                   unsigned OpNum = 0) const override {
-    if (!getSymbolRefOperandNum(Inst, OpNum))
+    if (!OpNum && !getSymbolRefOperandNum(Inst, OpNum))
       return nullptr;
 
     const MCOperand &Op = Inst.getOperand(OpNum);
@@ -1342,17 +1361,47 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
 
   int getUncondBranchEncodingSize() const override { return 28; }
 
+  // This helper function creates the snippet of code that compares a register
+  // RegNo with an immedaite Imm, and jumps to Target if they are equal.
+  // cmp RegNo, #Imm
+  // b.eq Target
+  // where cmp is an alias for subs, which results in the code below:
+  // subs xzr, RegNo, #Imm
+  // b.eq Target.
   InstructionListType createCmpJE(MCPhysReg RegNo, int64_t Imm,
                                   const MCSymbol *Target,
                                   MCContext *Ctx) const override {
     InstructionListType Code;
     Code.emplace_back(MCInstBuilder(AArch64::SUBSXri)
-                          .addReg(RegNo)
+                          .addReg(AArch64::XZR)
                           .addReg(RegNo)
                           .addImm(Imm)
                           .addImm(0));
     Code.emplace_back(MCInstBuilder(AArch64::Bcc)
+                          .addImm(AArch64CC::EQ)
+                          .addExpr(MCSymbolRefExpr::create(
+                              Target, MCSymbolRefExpr::VK_None, *Ctx)));
+    return Code;
+  }
+
+  // This helper function creates the snippet of code that compares a register
+  // RegNo with an immedaite Imm, and jumps to Target if they are not equal.
+  // cmp RegNo, #Imm
+  // b.ne Target
+  // where cmp is an alias for subs, which results in the code below:
+  // subs xzr, RegNo, #Imm
+  // b.ne Target.
+  InstructionListType createCmpJNE(MCPhysReg RegNo, int64_t Imm,
+                                   const MCSymbol *Target,
+                                   MCContext *Ctx) const override {
+    InstructionListType Code;
+    Code.emplace_back(MCInstBuilder(AArch64::SUBSXri)
+                          .addReg(AArch64::XZR)
+                          .addReg(RegNo)
                           .addImm(Imm)
+                          .addImm(0));
+    Code.emplace_back(MCInstBuilder(AArch64::Bcc)
+                          .addImm(AArch64CC::NE)
                           .addExpr(MCSymbolRefExpr::create(
                               Target, MCSymbolRefExpr::VK_None, *Ctx)));
     return Code;

bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp

@@ -338,15 +338,7 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
     if (RISCVExpr && RISCVExpr->getSubExpr())
       return getTargetSymbol(RISCVExpr->getSubExpr());
 
-    auto *BinExpr = dyn_cast<MCBinaryExpr>(Expr);
-    if (BinExpr)
-      return getTargetSymbol(BinExpr->getLHS());
-
-    auto *SymExpr = dyn_cast<MCSymbolRefExpr>(Expr);
-    if (SymExpr && SymExpr->getKind() == MCSymbolRefExpr::VK_None)
-      return &SymExpr->getSymbol();
-
-    return nullptr;
+    return MCPlusBuilder::getTargetSymbol(Expr);
   }
 
   const MCSymbol *getTargetSymbol(const MCInst &Inst,