[BOLT] Gadget scanner: analyze functions without CFG information

[atrosinenko]

Support simple analysis of the functions for which BOLT is unable to
reconstruct the CFG. This patch is inspired by the approach implemented
by Kristof Beyls in the original prototype of gadget scanner, but a
CFG-unaware counterpart of the data-flow analysis is implemented
instead of separate version of gadget detector, as multiple gadget kinds
are detected now.

[atrosinenko]

• [BOLT] Gadget scanner: prevent false positives due to jump tables #138884
• [BOLT] Introduce helpers to match MCInsts one at a time (NFC) #138883
• [BOLT] Factor out MCInstReference from gadget scanner (NFC) #138655
• [BOLT] Gadget scanner: account for BRK when searching for auth oracles #137975
• [BOLT] Gadget scanner: detect untrusted LR before tail call #137224
• [BOLT] Gadget scanner: improve handling of unreachable basic blocks #136183
• [BOLT] Gadget scanner: do not crash on debug-printing CFI instructions #136151
• [BOLT] Gadget scanner: detect authentication oracles #135663
• [BOLT] Gadget scanner: clarify MCPlusBuilder callbacks interface #136147
• [BOLT] Gadget scanner: refactor issue reporting #135662
• [BOLT] Gadget scanner: use more appropriate types (NFC) #135661
• [BOLT] Gadget scanner: detect signing oracles #134146
• [BOLT] Gadget scanner: analyze functions without CFG information #133461 👈 (View in Graphite)
• [BOLT] Gadget scanner: refine class names and debug output (NFC) #135073
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Anatoly Trosinenko (atrosinenko)

Changes

Support simple analysis of the functions for which BOLT is unable to
reconstruct the CFG. This patch is inspired by the approach implemented
by Kristof Beyls in the original prototype of gadget scanner, but a
CFG-unaware counterpart of the data-flow analysis is implemented
instead of separate version of gadget detector, as multiple gadget kinds
are detected now.

---

Patch is 41.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133461.diff

5 Files Affected:

• (modified) bolt/include/bolt/Core/BinaryFunction.h (+13)
• (modified) bolt/include/bolt/Passes/PAuthGadgetScanner.h (+24)
• (modified) bolt/lib/Passes/PAuthGadgetScanner.cpp (+189-77)
• (modified) bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s (+15)
• (modified) bolt/test/binary-analysis/AArch64/gs-pauth-calls.s (+594)

diff --git a/bolt/include/bolt/Core/BinaryFunction.h b/bolt/include/bolt/Core/BinaryFunction.h
index a92cb466c5992..b57fe1e4d35d9 100644
--- a/bolt/include/bolt/Core/BinaryFunction.h
+++ b/bolt/include/bolt/Core/BinaryFunction.h
@@ -793,6 +793,19 @@ class BinaryFunction {
     return iterator_range<const_cfi_iterator>(cie_begin(), cie_end());
   }
 
+  /// Iterate over instructions (only if CFG is unavailable or not built yet).
+  iterator_range<InstrMapType::iterator> instrs() {
+    assert(!hasCFG() && "Iterate over basic blocks instead");
+    return make_range(Instructions.begin(), Instructions.end());
+  }
+  iterator_range<InstrMapType::const_iterator> instrs() const {
+    assert(!hasCFG() && "Iterate over basic blocks instead");
+    return make_range(Instructions.begin(), Instructions.end());
+  }
+
+  /// Returns whether there are any labels at Offset.
+  bool hasLabelAt(unsigned Offset) const { return Labels.count(Offset) != 0; }
+
   /// Iterate over all jump tables associated with this function.
   iterator_range<std::map<uint64_t, JumpTable *>::const_iterator>
   jumpTables() const {
diff --git a/bolt/include/bolt/Passes/PAuthGadgetScanner.h b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
index 622e6721dea55..aa44f8c565639 100644
--- a/bolt/include/bolt/Passes/PAuthGadgetScanner.h
+++ b/bolt/include/bolt/Passes/PAuthGadgetScanner.h
@@ -67,6 +67,14 @@ struct MCInstInBFReference {
   uint64_t Offset;
   MCInstInBFReference(BinaryFunction *BF, uint64_t Offset)
       : BF(BF), Offset(Offset) {}
+
+  static MCInstInBFReference get(const MCInst *Inst, BinaryFunction &BF) {
+    for (auto &I : BF.instrs())
+      if (Inst == &I.second)
+        return MCInstInBFReference(&BF, I.first);
+    return {};
+  }
+
   MCInstInBFReference() : BF(nullptr), Offset(0) {}
   bool operator==(const MCInstInBFReference &RHS) const {
     return BF == RHS.BF && Offset == RHS.Offset;
@@ -106,6 +114,12 @@ struct MCInstReference {
   MCInstReference(BinaryFunction *BF, uint32_t Offset)
       : MCInstReference(MCInstInBFReference(BF, Offset)) {}
 
+  static MCInstReference get(const MCInst *Inst, BinaryFunction &BF) {
+    if (BF.hasCFG())
+      return MCInstInBBReference::get(Inst, BF);
+    return MCInstInBFReference::get(Inst, BF);
+  }
+
   bool operator<(const MCInstReference &RHS) const {
     if (ParentKind != RHS.ParentKind)
       return ParentKind < RHS.ParentKind;
@@ -140,6 +154,16 @@ struct MCInstReference {
     llvm_unreachable("");
   }
 
+  operator bool() const {
+    switch (ParentKind) {
+    case BasicBlockParent:
+      return U.BBRef.BB != nullptr;
+    case FunctionParent:
+      return U.BFRef.BF != nullptr;
+    }
+    llvm_unreachable("");
+  }
+
   uint64_t getAddress() const {
     switch (ParentKind) {
     case BasicBlockParent:
diff --git a/bolt/lib/Passes/PAuthGadgetScanner.cpp b/bolt/lib/Passes/PAuthGadgetScanner.cpp
index 86897937c95fe..bce279a4beed9 100644
--- a/bolt/lib/Passes/PAuthGadgetScanner.cpp
+++ b/bolt/lib/Passes/PAuthGadgetScanner.cpp
@@ -124,6 +124,27 @@ class TrackedRegisters {
   }
 };
 
+// Without CFG, we reset gadget scanning state when encountering an
+// unconditional branch. Note that BC.MIB->isUnconditionalBranch neither
+// considers indirect branches nor annotated tail calls as unconditional.
+static bool isStateTrackingBoundary(const BinaryContext &BC,
+                                    const MCInst &Inst) {
+  const MCInstrDesc &Desc = BC.MII->get(Inst.getOpcode());
+  // Adapted from llvm::MCInstrDesc::isUnconditionalBranch().
+  return Desc.isBranch() && Desc.isBarrier();
+}
+
+template <typename T> static void iterateOverInstrs(BinaryFunction &BF, T Fn) {
+  if (BF.hasCFG()) {
+    for (BinaryBasicBlock &BB : BF)
+      for (int64_t I = 0, E = BB.size(); I < E; ++I)
+        Fn(MCInstInBBReference(&BB, I));
+  } else {
+    for (auto I : BF.instrs())
+      Fn(MCInstInBFReference(&BF, I.first));
+  }
+}
+
 // The security property that is checked is:
 // When a register is used as the address to jump to in a return instruction,
 // that register must be safe-to-dereference. It must either
@@ -265,21 +286,24 @@ void PacStatePrinter::print(raw_ostream &OS, const State &S) const {
   OS << ">";
 }
 
-class PacRetAnalysis
-    : public DataflowAnalysis<PacRetAnalysis, State, /*Backward=*/false,
-                              PacStatePrinter> {
-  using Parent =
-      DataflowAnalysis<PacRetAnalysis, State, false, PacStatePrinter>;
-  friend Parent;
-
+class PacRetAnalysis {
 public:
-  PacRetAnalysis(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId,
+  PacRetAnalysis(BinaryFunction &BF,
                  const std::vector<MCPhysReg> &RegsToTrackInstsFor)
-      : Parent(BF, AllocId), NumRegs(BF.getBinaryContext().MRI->getNumRegs()),
+      : BC(BF.getBinaryContext()), NumRegs(BC.MRI->getNumRegs()),
         RegsToTrackInstsFor(RegsToTrackInstsFor) {}
+
   virtual ~PacRetAnalysis() {}
 
+  static std::shared_ptr<PacRetAnalysis>
+  create(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId,
+         const std::vector<MCPhysReg> &RegsToTrackInstsFor);
+
+  virtual void run() = 0;
+  virtual ErrorOr<const State &> getStateBefore(const MCInst &Inst) const = 0;
+
 protected:
+  BinaryContext &BC;
   const unsigned NumRegs;
   /// RegToTrackInstsFor is the set of registers for which the dataflow analysis
   /// must compute which the last set of instructions writing to it are.
@@ -296,8 +320,6 @@ class PacRetAnalysis
     return S.LastInstWritingReg[Index];
   }
 
-  void preflight() {}
-
   State createEntryState() {
     State S(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
     for (MCPhysReg Reg : BC.MIB->getTrustedLiveInRegs())
@@ -305,36 +327,6 @@ class PacRetAnalysis
     return S;
   }
 
-  State getStartingStateAtBB(const BinaryBasicBlock &BB) {
-    if (BB.isEntryPoint())
-      return createEntryState();
-
-    return State();
-  }
-
-  State getStartingStateAtPoint(const MCInst &Point) { return State(); }
-
-  void doConfluence(State &StateOut, const State &StateIn) {
-    PacStatePrinter P(BC);
-    LLVM_DEBUG({
-      dbgs() << " PacRetAnalysis::Confluence(\n";
-      dbgs() << "   State 1: ";
-      P.print(dbgs(), StateOut);
-      dbgs() << "\n";
-      dbgs() << "   State 2: ";
-      P.print(dbgs(), StateIn);
-      dbgs() << ")\n";
-    });
-
-    StateOut.merge(StateIn);
-
-    LLVM_DEBUG({
-      dbgs() << "   merged state: ";
-      P.print(dbgs(), StateOut);
-      dbgs() << "\n";
-    });
-  }
-
   BitVector getClobberedRegs(const MCInst &Point) const {
     BitVector Clobbered(NumRegs, false);
     // Assume a call can clobber all registers, including callee-saved
@@ -443,8 +435,6 @@ class PacRetAnalysis
     return Next;
   }
 
-  StringRef getAnnotationName() const { return StringRef("PacRetAnalysis"); }
-
 public:
   std::vector<MCInstReference>
   getLastClobberingInsts(const MCInst &Inst, BinaryFunction &BF,
@@ -463,14 +453,139 @@ class PacRetAnalysis
     }
     std::vector<MCInstReference> Result;
     for (const MCInst *Inst : LastWritingInsts) {
-      MCInstInBBReference Ref = MCInstInBBReference::get(Inst, BF);
-      assert(Ref.BB != nullptr && "Expected Inst to be found");
+      MCInstReference Ref = MCInstReference::get(Inst, BF);
+      assert(Ref && "Expected Inst to be found");
       Result.push_back(MCInstReference(Ref));
     }
     return Result;
   }
 };
 
+class PacRetDFAnalysis
+    : public PacRetAnalysis,
+      public DataflowAnalysis<PacRetDFAnalysis, State, /*Backward=*/false,
+                              PacStatePrinter> {
+  using DFParent =
+      DataflowAnalysis<PacRetDFAnalysis, State, false, PacStatePrinter>;
+  friend DFParent;
+
+  using PacRetAnalysis::BC;
+  using PacRetAnalysis::computeNext;
+
+public:
+  PacRetDFAnalysis(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId,
+                   const std::vector<MCPhysReg> &RegsToTrackInstsFor)
+      : PacRetAnalysis(BF, RegsToTrackInstsFor), DFParent(BF, AllocId) {}
+
+  ErrorOr<const State &> getStateBefore(const MCInst &Inst) const override {
+    return DFParent::getStateBefore(Inst);
+  }
+
+  void run() override { DFParent::run(); }
+
+protected:
+  void preflight() {}
+
+  State getStartingStateAtBB(const BinaryBasicBlock &BB) {
+    if (BB.isEntryPoint())
+      return createEntryState();
+
+    return State();
+  }
+
+  State getStartingStateAtPoint(const MCInst &Point) { return State(); }
+
+  void doConfluence(State &StateOut, const State &StateIn) {
+    PacStatePrinter P(BC);
+    LLVM_DEBUG({
+      dbgs() << " PacRetAnalysis::Confluence(\n";
+      dbgs() << "   State 1: ";
+      P.print(dbgs(), StateOut);
+      dbgs() << "\n";
+      dbgs() << "   State 2: ";
+      P.print(dbgs(), StateIn);
+      dbgs() << ")\n";
+    });
+
+    StateOut.merge(StateIn);
+
+    LLVM_DEBUG({
+      dbgs() << "   merged state: ";
+      P.print(dbgs(), StateOut);
+      dbgs() << "\n";
+    });
+  }
+
+  StringRef getAnnotationName() const { return "PacRetAnalysis"; }
+};
+
+class NoCFGPacRetAnalysis : public PacRetAnalysis {
+  BinaryFunction &BF;
+  MCPlusBuilder::AllocatorIdTy AllocId;
+  unsigned StateAnnotationIndex;
+
+  void cleanStateAnnotations() {
+    for (auto &I : BF.instrs())
+      BC.MIB->removeAnnotation(I.second, StateAnnotationIndex);
+  }
+
+  /// Creates a State with all registers marked unsafe (not to be confused
+  /// with empty state).
+  State createUnsafeState() const {
+    return State(NumRegs, RegsToTrackInstsFor.getNumTrackedRegisters());
+  }
+
+public:
+  NoCFGPacRetAnalysis(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId,
+                      const std::vector<MCPhysReg> &RegsToTrackInstsFor)
+      : PacRetAnalysis(BF, RegsToTrackInstsFor), BF(BF), AllocId(AllocId) {
+    StateAnnotationIndex =
+        BC.MIB->getOrCreateAnnotationIndex("NoCFGPacRetAnalysis");
+  }
+
+  void run() override {
+    State S = createEntryState();
+    for (auto &I : BF.instrs()) {
+      MCInst &Inst = I.second;
+
+      // If there is a label before this instruction, it is possible that it
+      // can be jumped-to, thus conservatively resetting S.
+      if (BF.hasLabelAt(I.first))
+        S = createUnsafeState();
+
+      // Check if we need to remove an old annotation (this is the case if
+      // this is the second, detailed, run of the analysis).
+      if (BC.MIB->hasAnnotation(Inst, StateAnnotationIndex))
+        BC.MIB->removeAnnotation(Inst, StateAnnotationIndex);
+      // Attach the state *before* this instruction.
+      BC.MIB->addAnnotation(Inst, StateAnnotationIndex, S, AllocId);
+
+      // Compute the state after this instruction.
+      // If this instruction is an unconditional branch (incl. indirect ones),
+      // reset the state.
+      if (isStateTrackingBoundary(BC, Inst))
+        S = createUnsafeState();
+      else
+        S = computeNext(Inst, S);
+    }
+  }
+
+  ErrorOr<const State &> getStateBefore(const MCInst &Inst) const override {
+    return BC.MIB->getAnnotationAs<State>(Inst, StateAnnotationIndex);
+  }
+
+  ~NoCFGPacRetAnalysis() { cleanStateAnnotations(); }
+};
+
+std::shared_ptr<PacRetAnalysis>
+PacRetAnalysis::create(BinaryFunction &BF, MCPlusBuilder::AllocatorIdTy AllocId,
+                       const std::vector<MCPhysReg> &RegsToTrackInstsFor) {
+  if (BF.hasCFG())
+    return std::make_shared<PacRetDFAnalysis>(BF, AllocId, RegsToTrackInstsFor);
+  return std::make_shared<NoCFGPacRetAnalysis>(BF, AllocId,
+                                               RegsToTrackInstsFor);
+}
+
 static std::shared_ptr<Report>
 shouldReportReturnGadget(const BinaryContext &BC, const MCInstReference &Inst,
                          const State &S) {
@@ -527,37 +642,34 @@ Analysis::findGadgets(BinaryFunction &BF,
                       MCPlusBuilder::AllocatorIdTy AllocatorId) {
   FunctionAnalysisResult Result;
 
-  PacRetAnalysis PRA(BF, AllocatorId, {});
-  PRA.run();
+  auto PRA = PacRetAnalysis::create(BF, AllocatorId, {});
+  PRA->run();
   LLVM_DEBUG({
     dbgs() << " After PacRetAnalysis:\n";
     BF.dump();
   });
 
   BinaryContext &BC = BF.getBinaryContext();
-  for (BinaryBasicBlock &BB : BF) {
-    for (int64_t I = 0, E = BB.size(); I < E; ++I) {
-      MCInstReference Inst(&BB, I);
-      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.
-      if (S.empty()) {
-        Result.Diagnostics.push_back(std::make_shared<GenericReport>(
-            Inst, "Warning: unreachable instruction found"));
-        continue;
-      }
-
-      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);
+  iterateOverInstrs(BF, [&](MCInstReference 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.
+    if (S.empty()) {
+      Result.Diagnostics.push_back(std::make_shared<GenericReport>(
+          Inst, "Warning: unreachable instruction found"));
+      return;
     }
-  }
+
+    if (auto Report = shouldReportReturnGadget(BC, Inst, S))
+      Result.Diagnostics.push_back(Report);
+
+    if (PacRetGadgetsOnly)
+      return;
+
+    if (auto Report = shouldReportCallGadget(BC, Inst, S))
+      Result.Diagnostics.push_back(Report);
+  });
   return Result;
 }
 
@@ -573,8 +685,8 @@ void Analysis::computeDetailedInfo(BinaryFunction &BF,
   std::vector<MCPhysReg> RegsToTrackVec(RegsToTrack.begin(), RegsToTrack.end());
 
   // Re-compute the analysis with register tracking.
-  PacRetAnalysis PRWIA(BF, AllocatorId, RegsToTrackVec);
-  PRWIA.run();
+  auto PRWIA = PacRetAnalysis::create(BF, AllocatorId, RegsToTrackVec);
+  PRWIA->run();
   LLVM_DEBUG({
     dbgs() << " After detailed PacRetAnalysis:\n";
     BF.dump();
@@ -585,7 +697,7 @@ void Analysis::computeDetailedInfo(BinaryFunction &BF,
     LLVM_DEBUG(
         { traceInst(BC, "Attaching clobbering info to", Report->Location); });
     (void)BC;
-    Report->setOverwritingInstrs(PRWIA.getLastClobberingInsts(
+    Report->setOverwritingInstrs(PRWIA->getLastClobberingInsts(
         Report->Location, BF, Report->getAffectedRegisters()));
   }
 }
@@ -598,9 +710,6 @@ void Analysis::runOnFunction(BinaryFunction &BF,
     BF.dump();
   });
 
-  if (!BF.hasCFG())
-    return;
-
   FunctionAnalysisResult FAR = findGadgets(BF, AllocatorId);
   if (FAR.Diagnostics.empty())
     return;
@@ -686,8 +795,11 @@ void GadgetReport::generateReport(raw_ostream &OS,
   };
   if (OverwritingInstrs.size() == 1) {
     const MCInstReference OverwInst = OverwritingInstrs[0];
-    assert(OverwInst.ParentKind == MCInstReference::BasicBlockParent);
-    reportFoundGadgetInSingleBBSingleOverwInst(OS, BC, OverwInst, Location);
+    // Printing the details for the MCInstReference::FunctionParent case
+    // is not implemented not to overcomplicate the code, as most functions
+    // are expected to have CFG information.
+    if (OverwInst.ParentKind == MCInstReference::BasicBlockParent)
+      reportFoundGadgetInSingleBBSingleOverwInst(OS, BC, OverwInst, Location);
   }
 }
 
diff --git a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
index d506ec13f4895..2193d40131478 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s
@@ -223,6 +223,21 @@ f_unreachable_instruction:
         ret
         .size f_unreachable_instruction, .-f_unreachable_instruction
 
+// Expected false positive: without CFG, the state is reset to all-unsafe
+// after an unconditional branch.
+
+        .globl  state_is_reset_after_indirect_branch_nocfg
+        .type   state_is_reset_after_indirect_branch_nocfg,@function
+state_is_reset_after_indirect_branch_nocfg:
+// CHECK-LABEL: GS-PAUTH: non-protected ret found in function state_is_reset_after_indirect_branch_nocfg, at address
+// CHECK-NEXT:  The instruction is     {{[0-9a-f]+}}:         ret
+// CHECK-NEXT:  The 0 instructions that write to the affected registers after any authentication are:
+        adr     x2, 1f
+        br      x2
+1:
+        ret
+        .size state_is_reset_after_indirect_branch_nocfg, .-state_is_reset_after_indirect_branch_nocfg
+
 /// Now do a basic sanity check on every different Authentication instruction:
 
         .globl  f_autiasp
diff --git a/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s b/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s
index cdbee9a620936..2512dba4da447 100644
--- a/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s
+++ b/bolt/test/binary-analysis/AArch64/gs-pauth-calls.s
@@ -423,6 +423,261 @@ bad_indirect_call_mem_chain_of_auts_multi_bb:
         ret
         .size bad_indirect_call_mem_chain_of_auts_multi_bb, .-bad_indirect_call_mem_chain_of_auts_multi_bb
 
+// Tests for CFG-unaware analysis.
+
+        .globl  good_direct_call_nocfg
+        .type   good_direct_call_nocfg,@function
+good_direct_call_nocfg:
+// CHECK-NOT: good_direct_call_nocfg
+        paciasp
+        stp     x29, x30, [sp, #-16]!
+        mov     x29, sp
+
+        bl      callee_ext
+
+        adr     x2, 1f
+        br      x2
+1:
+        ldp     x29, x30, [sp], #16
+        autiasp
+        ret
+        .size good_direct_call_nocfg, .-good_direct_call_nocfg
+
+        .globl  good_indirect_call_arg_nocfg
+        .type   good_indirect_call_arg_nocfg,@function
+good_indirect_call_arg_nocfg:
+// CHECK-NOT: good_indirect_call_arg_nocfg
+        paciasp
+        stp     x29, x30, [sp, #-16]!
+        mov     x29, sp
+
+        autia   x0, x1
+        blr     x0
+
+        adr     x2, 1f
+        br      x2
+1:
+        ldp     x29, x30, [sp], #16
+        autiasp
+        ret
+        .size good_indirect_call_arg_nocfg, .-good_indirect_call_arg_nocfg
+
+        .globl  good_indirect_call_mem_nocfg
+        .type   good_indirect_call_mem_nocfg,@function
+good_indirect_call_mem_nocfg:
+// CHECK-NOT: good_indirect_call_mem_nocfg
+        paciasp
+        stp     x29, x30, [sp, #-16]!
+        mov     x29, sp
+
+        ldr     x16, [x0]
+        autia   x16, x0
+        blr     x16
+
+        adr     x2, 1f
+        br      x2
+1:
+        ldp     x29, x30, [sp], #16
+        autiasp
+        ret
+        .size good_indirect_call_mem_nocfg, .-good_indirect_call_mem_nocfg
+
+        .globl  good_indirect_call_arg_v83_nocfg
+        .type   good_indirect_call_arg_v83_nocfg,@function
+good_indirect_call_arg_v83_nocfg:
+// CHECK-NOT: good_indirect_call_arg_v83_nocfg
+        paciasp
+        stp     x29, x30, [sp, #-16]!
+        mov     x29, sp
+
+        blraa   x0, x1
+
+        adr     x2, 1f
+        br      x2
+1:
+        ldp     x29, x30, [sp], #16
+        autiasp
+        ret
+        .size good_indirect_call_arg_v83_nocfg, .-good_indirect_call_arg_v83_nocfg
+
+        .globl  good_indirect_call_mem_v83_nocfg
+        .type   good_indirect_call_mem_v83_nocfg,@function
+good_indirect_call_mem_v83_nocfg:
+// CHECK-NOT: good_indirect_call_mem_v83_nocfg
+        paciasp
+        stp     x29, x30, [sp, #-16]!
+        mov     x29, sp
+
+        ldr     x16, [x0]
+        blraa   x16, x0
+
+        adr     x2, 1f
+        br      x2
+1:
+        ldp     x29, x30, [sp], #16
+        autiasp
+        ret
+        .size good_indirect_call_mem_v83_nocfg, .-good_indirect_call_mem_v83_nocfg
+
+        .globl  bad_indirect_call_arg_nocfg
+        .type   bad_indirect_call_arg_nocfg,@function
+bad_indirect_call_arg_nocfg:
+// CHECK-LABEL: GS-PAUTH: non-protected call found in function bad_indirect_call_arg_nocfg, at address
+// CHECK-NEXT:  The instruction is     {{[0-9a-f]+}}:         blr     x0
+// CHECK-NEXT:  The 0 instructions that write to the affected registers after any authentication are:
+        paciasp
+        stp     x29, x30, [sp, #-16]!
+        mov     x29, sp
+
+        blr     x0
+
+        adr     x2, 1f
+        br      x2
+1:
+        ldp     x29, x30, [sp], #16
+        autiasp
+        ret
+        .size bad_indirect_call_arg_nocfg, .-bad_indirect_call_arg_nocfg
+
+        .globl  obscure_indirect_call_arg_nocfg
+        .type ...
[truncated]

[atrosinenko]

Sorry, missed the collapsed file in "Files changed" view.

Aside from addressing the comments, added two more test cases documenting known issues in ceb9d04

[kbeyls]

LGTM, thanks!

[atrosinenko]

@maksfb Oh, forgot to mention this explicitly: unlike many other PRs from this series, this one exposes a few more pieces of information from BinaryFunction, namely the flat list of instructions when CFG is not available and information about whether there are any labels before the particular instruction.

[atrosinenko]

Is it acceptable to extend BinaryFunction's interface this way?

[atrosinenko]

@maksfb Is it appropriate to add instrs() and hasLabelAt(Offset) methods to BinaryFunction's interface this way?

[atrosinenko]

Ping.

@maksfb could you please look at the extension to the interface of BinaryFunction? This tiny fraction of the patch is probably worth reviewing by a core developer.

[atrosinenko]

@maksfb This PR introduces instrs() and hasLabelAt(Offset) accessors to the interface of BinaryFunction. Are these an appropriate way to expose the contents of a BinaryFunction without CFG information?