[BOLT] Print stats in MarkRAStates, InsertNegateRAState

- print statistics on the output about function getting Ignored in
  MarkRAStates, and on rewriting OpNegateRAState in
  InsertNegateRAStatePass.
- improve error messages in MarkRAStates

Author: bgergely0

bolt/include/bolt/Passes/InsertNegateRAStatePass.h

@@ -28,6 +28,7 @@ class InsertNegateRAState : public BinaryFunctionPass {
   void runOnFunction(BinaryFunction &BF);
 
 private:
+  uint64_t FunctionsModified{0};
   /// Because states are tracked as MCAnnotations on individual instructions,
   /// newly inserted instructions do not have a state associated with them.
   /// New states are "inherited" from the last known state.

bolt/include/bolt/Passes/MarkRAStates.h

@@ -25,7 +25,10 @@ class MarkRAStates : public BinaryFunctionPass {
 
   /// Pass entry point
   Error runOnFunctions(BinaryContext &BC) override;
-  void runOnFunction(BinaryFunction &BF);
+  bool runOnFunction(BinaryFunction &BF);
+
+private:
+  uint64_t FunctionsIgnored{0};
 };
 
 } // namespace bolt

bolt/lib/Passes/InsertNegateRAStatePass.cpp

@@ -115,11 +115,12 @@ void InsertNegateRAState::inferUnknownStates(BinaryFunction &BF) {
 
 Error InsertNegateRAState::runOnFunctions(BinaryContext &BC) {
   ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
-    if (BF.containedNegateRAState()) {
+    if (BF.containedNegateRAState() && !BF.isIgnored()) {
       // We can skip functions which did not include negate-ra-state CFIs. This
       // includes code using pac-ret hardening as well, if the binary is
       // compiled with `-fno-exceptions -fno-unwind-tables
       // -fno-asynchronous-unwind-tables`
+      FunctionsModified++;
       runOnFunction(BF);
     }
   };
@@ -128,6 +129,11 @@ Error InsertNegateRAState::runOnFunctions(BinaryContext &BC) {
       BC, ParallelUtilities::SchedulingPolicy::SP_TRIVIAL, WorkFun, nullptr,
       "InsertNegateRAStatePass");
 
+  BC.outs() << "BOLT-INFO: rewritten pac-ret DWARF info in "
+            << FunctionsModified << " out of " << BC.getBinaryFunctions().size()
+            << " functions "
+            << format("(%.2lf%%).\n", (100.0 * FunctionsModified) /
+                                          BC.getBinaryFunctions().size());
   return Error::success();
 }
 

bolt/lib/Passes/MarkRAStates.cpp

@@ -31,10 +31,7 @@ using namespace llvm;
 namespace llvm {
 namespace bolt {
 
-void MarkRAStates::runOnFunction(BinaryFunction &BF) {
-
-  if (BF.isIgnored())
-    return;
+bool MarkRAStates::runOnFunction(BinaryFunction &BF) {
 
   BinaryContext &BC = BF.getBinaryContext();
 
@@ -49,10 +46,9 @@ void MarkRAStates::runOnFunction(BinaryFunction &BF) {
         // annotation.
         BF.setIgnored();
         BC.outs() << "BOLT-INFO: inconsistent RAStates in function "
-                  << BF.getPrintName() << "\n";
-        BC.outs()
-            << "BOLT-INFO: ptr sign/auth inst without .cfi_negate_ra_state\n";
-        return;
+                  << BF.getPrintName()
+                  << ": ptr sign/auth inst without .cfi_negate_ra_state\n";
+        return false;
       }
     }
   }
@@ -72,11 +68,10 @@ void MarkRAStates::runOnFunction(BinaryFunction &BF) {
         if (RAState) {
           // RA signing instructions should only follow unsigned RA state.
           BC.outs() << "BOLT-INFO: inconsistent RAStates in function "
-                    << BF.getPrintName() << "\n";
-          BC.outs() << "BOLT-INFO: ptr signing inst encountered in Signed RA "
-                       "state.\n";
+                    << BF.getPrintName()
+                    << ": ptr signing inst encountered in Signed RA state\n";
           BF.setIgnored();
-          return;
+          return false;
         }
         // The signing instruction itself is unsinged, but the next will be
         // signed.
@@ -85,11 +80,11 @@ void MarkRAStates::runOnFunction(BinaryFunction &BF) {
         if (!RAState) {
           // RA authenticating instructions should only follow signed RA state.
           BC.outs() << "BOLT-INFO: inconsistent RAStates in function "
-                    << BF.getPrintName() << "\n";
-          BC.outs() << "BOLT-INFO: ptr authenticating inst encountered in "
-                       "Unsigned RA state.\n";
+                    << BF.getPrintName()
+                    << ": ptr authenticating inst encountered in Unsigned RA "
+                       "state\n";
           BF.setIgnored();
-          return;
+          return false;
         }
         // The authenticating instruction itself is signed, but the next will be
         // unsigned.
@@ -120,23 +115,36 @@ void MarkRAStates::runOnFunction(BinaryFunction &BF) {
       }
     }
   }
+  return true;
 }
 
 Error MarkRAStates::runOnFunctions(BinaryContext &BC) {
   ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
-    if (BF.containedNegateRAState()) {
+    if (BF.containedNegateRAState() && !BF.isIgnored()) {
       // We can skip functions which did not include negate-ra-state CFIs. This
       // includes code using pac-ret hardening as well, if the binary is
       // compiled with `-fno-exceptions -fno-unwind-tables
       // -fno-asynchronous-unwind-tables`
-      runOnFunction(BF);
+      if (!runOnFunction(BF)) {
+        FunctionsIgnored++;
+      }
     }
   };
 
   ParallelUtilities::runOnEachFunction(
       BC, ParallelUtilities::SchedulingPolicy::SP_TRIVIAL, WorkFun, nullptr,
       "MarkRAStates");
 
+  int Total = llvm::count_if(
+      BC.getBinaryFunctions(),
+      [&](std::pair<const unsigned long, BinaryFunction> &P) {
+        return P.second.containedNegateRAState() && !P.second.isIgnored();
+      });
+  BC.outs() << "BOLT-INFO: MarkRAStates ran on " << Total
+            << " functions. Ignored " << FunctionsIgnored << " functions "
+            << format("(%.2lf%%)", (100.0 * FunctionsIgnored) / Total)
+            << " because of CFI inconsistencies.\n";
+
   return Error::success();
 }
 

bolt/test/AArch64/negate-ra-state-incorrect.s

@@ -10,14 +10,9 @@
 # RUN: %clang %cflags  %t.o -o %t.exe -Wl,-q
 # RUN: llvm-bolt %t.exe -o %t.exe.bolt --no-threads | FileCheck %s --check-prefix=CHECK-BOLT
 
-# CHECK-BOLT: BOLT-INFO: inconsistent RAStates in function foo
-# CHECK-BOLT-NEXT: BOLT-INFO: ptr authenticating inst encountered in Unsigned RA state.
-
-# CHECK-BOLT: BOLT-INFO: inconsistent RAStates in function bar
-# CHECK-BOLT-NEXT: BOLT-INFO: ptr signing inst encountered in Signed RA state
-
-# CHECK-BOLT: BOLT-INFO: inconsistent RAStates in function baz
-# CHECK-BOLT-NEXT: BOLT-INFO: ptr sign/auth inst without .cfi_negate_ra_state
+# CHECK-BOLT: BOLT-INFO: inconsistent RAStates in function foo: ptr authenticating inst encountered in Unsigned RA state
+# CHECK-BOLT: BOLT-INFO: inconsistent RAStates in function bar: ptr signing inst encountered in Signed RA state
+# CHECK-BOLT: BOLT-INFO: inconsistent RAStates in function baz: ptr sign/auth inst without .cfi_negate_ra_state
 
 # Check that the incorrect functions got ignored, so they are not in the new .text section
 # RUN: llvm-objdump %t.exe.bolt -d -j .text | FileCheck %s --check-prefix=CHECK-OBJDUMP