Fix handling of unreachable loops of BBs

Author: atrosinenko

bolt/lib/Passes/PAuthGadgetScanner.cpp

@@ -1342,21 +1342,42 @@ void FunctionAnalysisContext::findUnsafeUses(
     BF.dump();
   });
 
+  bool UnreachableBBReported = false;
   if (BF.hasCFG()) {
-    // Warn on basic blocks being unreachable according to BOLT, as this
-    // likely means CFG is imprecise.
+    // Warn on basic blocks being unreachable according to BOLT (at most once
+    // per BinaryFunction), as this likely means the CFG reconstructed by BOLT
+    // is imprecise. A basic block can be
+    // * reachable from an entry basic block - a hopefully correct non-empty
+    //   state is propagated to that basic block sooner or later. All basic
+    //   blocks are expected to belong to this category under normal conditions.
+    // * reachable from a "directly unreachable" BB (a basic block that has no
+    //   direct predecessors and this is not because it is an entry BB) - *some*
+    //   non-empty state is propagated to this basic block sooner or later, as
+    //   the initial state of directly unreachable basic blocks is
+    //   pessimistically initialized to "all registers are unsafe"
+    //   - a warning can be printed for the "directly unreachable" basic block
+    // * neither reachable from an entry nor from a "directly unreachable" BB
+    //   (such as if this BB is in an isolated loop of basic blocks) - the final
+    //   state is computed to be empty for this basic block
+    //   - a warning can be printed for this basic block
     for (BinaryBasicBlock &BB : BF) {
-      if (!BB.pred_empty() || BB.isEntryPoint())
+      MCInst *FirstInst = BB.getFirstNonPseudoInstr();
+      // Skip empty basic block early for simplicity.
+      if (!FirstInst)
+        continue;
+
+      bool IsDirectlyUnreachable = BB.pred_empty() && !BB.isEntryPoint();
+      bool HasNoStateComputed = Analysis->getStateBefore(*FirstInst).empty();
+      if (!IsDirectlyUnreachable && !HasNoStateComputed)
         continue;
-      // Arbitrarily attach the report to the first instruction of BB.
-      MCInst *InstToReport = BB.getFirstNonPseudoInstr();
-      if (!InstToReport)
-        continue; // BB has no real instructions
 
+      // Arbitrarily attach the report to the first instruction of BB.
       Reports.push_back(
-          make_generic_report(MCInstReference::get(InstToReport, BF),
-                              "Warning: no predecessor basic blocks detected "
-                              "(possibly incomplete CFG)"));
+          make_generic_report(MCInstReference::get(FirstInst, BF),
+                              "Warning: the function has unreachable basic "
+                              "blocks (possibly incomplete CFG)"));
+      UnreachableBBReported = true;
+      break; // One warning per function.
     }
   }
 
@@ -1365,7 +1386,13 @@ void FunctionAnalysisContext::findUnsafeUses(
       return;
 
     const SrcState &S = Analysis->getStateBefore(Inst);
-    assert(!S.empty() && "Instruction has no associated state");
+    if (S.empty()) {
+      LLVM_DEBUG(
+          { traceInst(BC, "Instruction has no state, skipping", Inst); });
+      assert(UnreachableBBReported && "Should be reported at least once");
+      (void)UnreachableBBReported;
+      return;
+    }
 
     if (auto Report = shouldReportReturnGadget(BC, Inst, S))
       Reports.push_back(*Report);

bolt/test/binary-analysis/AArch64/gs-pacret-autiasp.s

@@ -215,7 +215,7 @@ f_callclobbered_calleesaved:
         .globl  f_unreachable_instruction
         .type   f_unreachable_instruction,@function
 f_unreachable_instruction:
-// CHECK-LABEL: GS-PAUTH: Warning: no predecessor basic blocks detected (possibly incomplete CFG) in function f_unreachable_instruction, basic block {{[0-9a-zA-Z.]+}}, at address
+// CHECK-LABEL: GS-PAUTH: Warning: the function has unreachable basic blocks (possibly incomplete CFG) in function f_unreachable_instruction, basic block {{[0-9a-zA-Z.]+}}, at address
 // CHECK-NEXT:    The instruction is     {{[0-9a-f]+}}:       add     x0, x1, x2
 // CHECK-NOT:   instructions that write to the affected registers after any authentication are:
 // CHECK-LABEL: GS-PAUTH: non-protected ret found in function f_unreachable_instruction, basic block {{[0-9a-zA-Z.]+}}, at address

bolt/test/binary-analysis/AArch64/gs-pauth-calls.s

@@ -1441,7 +1441,7 @@ printed_instrs_nocfg:
         .globl  bad_unreachable_call
         .type   bad_unreachable_call,@function
 bad_unreachable_call:
-// CHECK-LABEL: GS-PAUTH: Warning: no predecessor basic blocks detected (possibly incomplete CFG) in function bad_unreachable_call, basic block {{[^,]+}}, at address
+// CHECK-LABEL: GS-PAUTH: Warning: the function has unreachable basic blocks (possibly incomplete CFG) in function bad_unreachable_call, basic block {{[^,]+}}, at address
 // CHECK-NEXT:  The instruction is     {{[0-9a-f]+}}:      blr     x0
 // CHECK-NOT:   instructions that write to the affected registers after any authentication are:
 // CHECK-LABEL: GS-PAUTH: non-protected call found in function bad_unreachable_call, basic block {{[^,]+}}, at address
@@ -1465,7 +1465,7 @@ bad_unreachable_call:
         .type   good_unreachable_call,@function
 good_unreachable_call:
 // CHECK-NOT: non-protected call{{.*}}good_unreachable_call
-// CHECK-LABEL: GS-PAUTH: Warning: no predecessor basic blocks detected (possibly incomplete CFG) in function good_unreachable_call, basic block {{[^,]+}}, at address
+// CHECK-LABEL: GS-PAUTH: Warning: the function has unreachable basic blocks (possibly incomplete CFG) in function good_unreachable_call, basic block {{[^,]+}}, at address
 // CHECK-NEXT:  The instruction is     {{[0-9a-f]+}}:      autia   x0, x1
 // CHECK-NOT: instructions that write to the affected registers after any authentication are:
 // CHECK-NOT: non-protected call{{.*}}good_unreachable_call
@@ -1485,6 +1485,33 @@ good_unreachable_call:
         ret
         .size good_unreachable_call, .-good_unreachable_call
 
+        .globl  unreachable_loop_of_bbs
+        .type   unreachable_loop_of_bbs,@function
+unreachable_loop_of_bbs:
+// CHECK-NOT: unreachable basic blocks{{.*}}unreachable_loop_of_bbs
+// CHECK-NOT: non-protected call{{.*}}unreachable_loop_of_bbs
+// CHECK-LABEL: GS-PAUTH: Warning: the function has unreachable basic blocks (possibly incomplete CFG) in function unreachable_loop_of_bbs, basic block {{[^,]+}}, at address
+// CHECK-NEXT:  The instruction is     {{[0-9a-f]+}}:      blr     x0
+// CHECK-NOT: unreachable basic blocks{{.*}}unreachable_loop_of_bbs
+// CHECK-NOT: non-protected call{{.*}}unreachable_loop_of_bbs
+        paciasp
+        stp     x29, x30, [sp, #-16]!
+        mov     x29, sp
+        b       .Lreachable_epilogue_bb
+
+.Lfirst_unreachable_bb:
+        blr     x0      // <-- this call is not analyzed
+        b       .Lsecond_unreachable_bb
+.Lsecond_unreachable_bb:
+        blr     x1      // <-- this call is not analyzed
+        b       .Lfirst_unreachable_bb
+
+.Lreachable_epilogue_bb:
+        ldp     x29, x30, [sp], #16
+        autiasp
+        ret
+        .size unreachable_loop_of_bbs, .-unreachable_loop_of_bbs
+
         .globl  main
         .type   main,@function
 main: