[BOLT] Review changes

- rename kSigned/kUnsigned to kRASigned/kRAUnsigned
- const& in MarkRAStates
- spelling
- change flag to hidden

Author: bgergely0

bolt/include/bolt/Core/MCPlus.h

@@ -72,8 +72,8 @@ class MCAnnotation {
     kLabel,               /// MCSymbol pointing to this instruction.
     kSize,                /// Size of the instruction.
     kDynamicBranch,       /// Jit instruction patched at runtime.
-    kSigned,              /// Inst is in a range where RA is signed.
-    kUnsigned,            /// Inst is in a range where RA is unsigned.
+    kRASigned,            /// Inst is in a range where RA is signed.
+    kRAUnsigned,          /// Inst is in a range where RA is unsigned.
     kRememberState,       /// Inst has rememberState CFI.
     kRestoreState,        /// Inst has restoreState CFI.
     kNegateState,         /// Inst has OpNegateRAState CFI.

bolt/lib/Core/MCPlusBuilder.cpp

@@ -187,21 +187,21 @@ bool MCPlusBuilder::hasRestoreState(const MCInst &Inst) const {
 }
 
 void MCPlusBuilder::setRASigned(MCInst &Inst) const {
-  assert(!hasAnnotation(Inst, MCAnnotation::kSigned));
-  setAnnotationOpValue(Inst, MCAnnotation::kSigned, true);
+  assert(!hasAnnotation(Inst, MCAnnotation::kRASigned));
+  setAnnotationOpValue(Inst, MCAnnotation::kRASigned, true);
 }
 
 bool MCPlusBuilder::isRASigned(const MCInst &Inst) const {
-  return hasAnnotation(Inst, MCAnnotation::kSigned);
+  return hasAnnotation(Inst, MCAnnotation::kRASigned);
 }
 
 void MCPlusBuilder::setRAUnsigned(MCInst &Inst) const {
-  assert(!hasAnnotation(Inst, MCAnnotation::kUnsigned));
-  setAnnotationOpValue(Inst, MCAnnotation::kUnsigned, true);
+  assert(!hasAnnotation(Inst, MCAnnotation::kRAUnsigned));
+  setAnnotationOpValue(Inst, MCAnnotation::kRAUnsigned, true);
 }
 
 bool MCPlusBuilder::isRAUnsigned(const MCInst &Inst) const {
-  return hasAnnotation(Inst, MCAnnotation::kUnsigned);
+  return hasAnnotation(Inst, MCAnnotation::kRAUnsigned);
 }
 
 bool MCPlusBuilder::isRAStateUnknown(const MCInst &Inst) const {

bolt/lib/Passes/InsertNegateRAStatePass.cpp

@@ -92,9 +92,7 @@ void InsertNegateRAState::inferUnknownStates(BinaryFunction &BF) {
   bool FirstIter = true;
   MCInst PrevInst;
   for (BinaryBasicBlock &BB : BF) {
-    for (auto It = BB.begin(); It != BB.end(); ++It) {
-
-      MCInst &Inst = *It;
+    for (MCInst &Inst : BB) {
       if (BC.MIB->isCFI(Inst))
         continue;
 

bolt/lib/Passes/MarkRAStates.cpp

@@ -35,9 +35,8 @@ bool MarkRAStates::runOnFunction(BinaryFunction &BF) {
 
   BinaryContext &BC = BF.getBinaryContext();
 
-  for (BinaryBasicBlock &BB : BF) {
-    for (auto It = BB.begin(); It != BB.end(); ++It) {
-      MCInst &Inst = *It;
+  for (const BinaryBasicBlock &BB : BF) {
+    for (const MCInst &Inst : BB) {
       if ((BC.MIB->isPSignOnLR(Inst) ||
            (BC.MIB->isPAuthOnLR(Inst) && !BC.MIB->isPAuthAndRet(Inst))) &&
           !BC.MIB->hasNegateRAState(Inst)) {
@@ -58,9 +57,7 @@ bool MarkRAStates::runOnFunction(BinaryFunction &BF) {
   RAStateStack.push(RAState);
 
   for (BinaryBasicBlock &BB : BF) {
-    for (auto It = BB.begin(); It != BB.end(); ++It) {
-
-      MCInst &Inst = *It;
+    for (MCInst &Inst : BB) {
       if (BC.MIB->isCFI(Inst))
         continue;
 
@@ -150,7 +147,7 @@ Error MarkRAStates::runOnFunctions(BinaryContext &BC) {
     BC.outs() << "BOLT-INFO: MarkRAStates ran on " << Total
               << " functions. Ignored " << FunctionsIgnored << " functions "
               << format("(%.2lf%%)", (100.0 * FunctionsIgnored) / Total)
-              << " because of CFI inconsistencies.\n";
+              << " because of CFI inconsistencies\n";
   }
 
   return Error::success();

bolt/lib/Rewrite/BinaryPassManager.cpp

@@ -283,7 +283,7 @@ cl::opt<bool>
     UpdateBranchProtection("update-branch-protection",
                            cl::desc("Rewrites pac-ret DWARF CFI instructions "
                                     "(AArch64-only, on by default)"),
-                           cl::init(true), cl::cat(BoltOptCategory));
+                           cl::init(true), cl::Hidden, cl::cat(BoltCategory));
 } // namespace opts
 
 namespace llvm {

bolt/lib/Rewrite/RewriteInstance.cpp

@@ -3526,7 +3526,7 @@ void RewriteInstance::disassembleFunctions() {
       if (!opts::UpdateBranchProtection) {
         BC->errs()
             << "BOLT-ERROR: --update-branch-protection is set to false, but "
-            << Function.getPrintName() << " contains .cfi-negate-ra-state.\n";
+            << Function.getPrintName() << " contains .cfi-negate-ra-state\n";
         exit(1);
       }
     }

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

@@ -2,7 +2,7 @@
 # RUN: %clang %cflags  %t.o -o %t.exe -Wl,-q
 # RUN: not llvm-bolt %t.exe -o %t.exe.bolt --update-branch-protection=false 2>&1 | FileCheck %s
 
-# CHECK: BOLT-ERROR: --update-branch-protection is set to false, but foo contains .cfi-negate-ra-state.
+# CHECK: BOLT-ERROR: --update-branch-protection is set to false, but foo contains .cfi-negate-ra-state
 
   .text
   .globl  foo