Skip to content

Conversation

@bgergely0
Copy link
Contributor

Original names were "working titles". After initial patches are merged,
I'd like to rename these passes to names that reflect their intent
better and show their relationship to each other:

InsertNegateRAStatePass renamed to PointerAuthCFIFixup,
MarkRAStates renamed to PointerAuthCFIAnalyzer.

Copy link
Contributor Author

bgergely0 commented Oct 22, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

@bgergely0 bgergely0 marked this pull request as ready for review October 22, 2025 13:14
@llvmbot llvmbot added the BOLT label Oct 22, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 22, 2025

@llvm/pr-subscribers-bolt

Author: Gergely Bálint (bgergely0)

Changes

Original names were "working titles". After initial patches are merged,
I'd like to rename these passes to names that reflect their intent
better and show their relationship to each other:

InsertNegateRAStatePass renamed to PointerAuthCFIFixup,
MarkRAStates renamed to PointerAuthCFIAnalyzer.


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

10 Files Affected:

  • (modified) bolt/docs/PacRetDesign.md (+12-11)
  • (renamed) bolt/include/bolt/Passes/PointerAuthCFIAnalyzer.h (+7-7)
  • (renamed) bolt/include/bolt/Passes/PointerAuthCFIFixup.h (+7-7)
  • (modified) bolt/lib/Passes/CMakeLists.txt (+2-2)
  • (renamed) bolt/lib/Passes/PointerAuthCFIAnalyzer.cpp (+9-8)
  • (renamed) bolt/lib/Passes/PointerAuthCFIFixup.cpp (+15-15)
  • (modified) bolt/lib/Rewrite/BinaryPassManager.cpp (+4-4)
  • (modified) bolt/test/AArch64/negate-ra-state-incorrect.s (+1-1)
  • (modified) bolt/test/AArch64/negate-ra-state.s (+4-4)
  • (modified) bolt/test/AArch64/pacret-split-funcs.s (+2-2)
diff --git a/bolt/docs/PacRetDesign.md b/bolt/docs/PacRetDesign.md
index c7c76cac3a100..0de2da50f8fd6 100644
--- a/bolt/docs/PacRetDesign.md
+++ b/bolt/docs/PacRetDesign.md
@@ -104,9 +104,9 @@ negate-ra-state CFIs will become invalid during BasicBlock reordering.
 ## Solution design
 
 The implementation introduces two new passes:
-1. `MarkRAStatesPass`: assigns the RA state to each instruction based on the CFIs
-    in the input binary
-2. `InsertNegateRAStatePass`: reads those assigned instruction RA states after
+1. `PointerAuthCFIAnalyzer`: assigns the RA state to each instruction based on
+    the CFI in the input binary
+2. `PointerAuthCFIFixup`: reads those assigned instruction RA states after
     optimizations, and emits `DW_CFA_AARCH64_negate_ra_state` CFIs at the correct
     places: wherever there is a state change between two consecutive instructions
     in the layout order.
@@ -129,7 +129,7 @@ instruction.
 This special case is handled by adding an `initialRAState` bool to each BinaryFunction.
 If the `Offset` the CFI refers to is zero, we don't store an annotation, but set
 the `initialRAState` in `FillCFIInfoFor`. This information is then used in
-`MarkRAStates`.
+`PointerAuthCFIAnalyzer`.
 
 ### Binaries without DWARF info
 
@@ -146,7 +146,7 @@ In summary:
 - pointer auth is used, and we have DWARF CFIs: passes run, and rewrite the
   negate-ra-state CFI.
 
-### MarkRAStates pass
+### PointerAuthCFIAnalyzer pass
 
 This pass runs before optimizations reorder anything.
 
@@ -173,9 +173,9 @@ what we have before the pass, and after it.
 | autiasp                       | negate-ra-state | signed   |
 | ret                           |                 | unsigned |
 
-##### Error handling in MarkRAState Pass:
+##### Error handling in PointerAuthCFIAnalyzer pass:
 
-Whenever the MarkRAStates pass finds inconsistencies in the current
+Whenever the PointerAuthCFIAnalyzer pass finds inconsistencies in the current
 BinaryFunction, it marks the function as ignored using `BF.setIgnored()`. BOLT
 will not optimize this function but will emit it unchanged in the original section
 (`.bolt.org.text`).
@@ -188,16 +188,17 @@ The inconsistencies are as follows:
 Users will be informed about the number of ignored functions in the pass, the
 exact functions ignored, and the found inconsistency.
 
-### InsertNegateRAStatePass
+### PointerAuthCFIFixup
 
-This pass runs after optimizations. It performns the _inverse_ of MarkRAState pa s:
+This pass runs after optimizations. It performns the _inverse_ of PointerAuthCFIAnalyzer
+pass:
 1. it reads the RA state annotations attached to the instructions, and
 2. whenever the state changes, it adds a PseudoInstruction that holds an
    OpNegateRAState CFI.
 
 ##### Covering newly generated instructions:
 
-Some BOLT passes can add new Instructions. In InsertNegateRAStatePass, we have
+Some BOLT passes can add new Instructions. In PointerAuthCFIFixup, we have
 to know what RA state these have.
 
 > [!important]
@@ -230,7 +231,7 @@ freely. The only special case is function splitting. When a function is split,
 the split part becomes a new function in the emitted binary. For unwinding to
 work, it needs to "replay" all CFIs that lead up to the split point. BOLT does
 this for other CFIs. As negate-ra-state is not read (only stored as an Annotation),
-we have to do this manually in InsertNegateRAStatePass. Here, if the split part
+we have to do this manually in PointerAuthCFIFixup. Here, if the split part
 starts with an instruction that has Signed RA state, we add a negate-ra-state CFI
 to indicate this.
 
diff --git a/bolt/include/bolt/Passes/MarkRAStates.h b/bolt/include/bolt/Passes/PointerAuthCFIAnalyzer.h
similarity index 59%
rename from bolt/include/bolt/Passes/MarkRAStates.h
rename to bolt/include/bolt/Passes/PointerAuthCFIAnalyzer.h
index 675ab9727142b..e63de077fad18 100644
--- a/bolt/include/bolt/Passes/MarkRAStates.h
+++ b/bolt/include/bolt/Passes/PointerAuthCFIAnalyzer.h
@@ -1,4 +1,4 @@
-//===- bolt/Passes/MarkRAStates.cpp ---------------------------------===//
+//===- bolt/Passes/PointerAuthCFIAnalyzer.h -------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,22 +6,22 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This file implements the MarkRAStates class.
+// This file implements the PointerAuthCFIAnalyzer class.
 //
 //===----------------------------------------------------------------------===//
-#ifndef BOLT_PASSES_MARK_RA_STATES
-#define BOLT_PASSES_MARK_RA_STATES
+#ifndef BOLT_PASSES_POINTER_AUTH_CFI_ANALYZER
+#define BOLT_PASSES_POINTER_AUTH_CFI_ANALYZER
 
 #include "bolt/Passes/BinaryPasses.h"
 
 namespace llvm {
 namespace bolt {
 
-class MarkRAStates : public BinaryFunctionPass {
+class PointerAuthCFIAnalyzer : public BinaryFunctionPass {
 public:
-  explicit MarkRAStates() : BinaryFunctionPass(false) {}
+  explicit PointerAuthCFIAnalyzer() : BinaryFunctionPass(false) {}
 
-  const char *getName() const override { return "mark-ra-states"; }
+  const char *getName() const override { return "pointer-auth-cfi-analyzer"; }
 
   /// Pass entry point
   Error runOnFunctions(BinaryContext &BC) override;
diff --git a/bolt/include/bolt/Passes/InsertNegateRAStatePass.h b/bolt/include/bolt/Passes/PointerAuthCFIFixup.h
similarity index 87%
rename from bolt/include/bolt/Passes/InsertNegateRAStatePass.h
rename to bolt/include/bolt/Passes/PointerAuthCFIFixup.h
index b4b428207b657..debd24b63516f 100644
--- a/bolt/include/bolt/Passes/InsertNegateRAStatePass.h
+++ b/bolt/include/bolt/Passes/PointerAuthCFIFixup.h
@@ -1,4 +1,4 @@
-//===- bolt/Passes/InsertNegateRAStatePass.h ------------------------------===//
+//===- bolt/Passes/PointerAuthCFIFixup.h ----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,22 +6,22 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This file implements the InsertNegateRAStatePass class.
+// This file implements the PointerAuthCFIFixup class.
 //
 //===----------------------------------------------------------------------===//
-#ifndef BOLT_PASSES_INSERT_NEGATE_RA_STATE_PASS
-#define BOLT_PASSES_INSERT_NEGATE_RA_STATE_PASS
+#ifndef BOLT_PASSES_POINTER_AUTH_CFI_FIXUP
+#define BOLT_PASSES_POINTER_AUTH_CFI_FIXUP
 
 #include "bolt/Passes/BinaryPasses.h"
 
 namespace llvm {
 namespace bolt {
 
-class InsertNegateRAState : public BinaryFunctionPass {
+class PointerAuthCFIFixup : public BinaryFunctionPass {
 public:
-  explicit InsertNegateRAState() : BinaryFunctionPass(false) {}
+  explicit PointerAuthCFIFixup() : BinaryFunctionPass(false) {}
 
-  const char *getName() const override { return "insert-negate-ra-state-pass"; }
+  const char *getName() const override { return "pointer-auth-cfi-fixup"; }
 
   /// Pass entry point
   Error runOnFunctions(BinaryContext &BC) override;
diff --git a/bolt/lib/Passes/CMakeLists.txt b/bolt/lib/Passes/CMakeLists.txt
index d7519518f186f..fb2f74f660177 100644
--- a/bolt/lib/Passes/CMakeLists.txt
+++ b/bolt/lib/Passes/CMakeLists.txt
@@ -17,18 +17,18 @@ add_llvm_library(LLVMBOLTPasses
   IdenticalCodeFolding.cpp
   IndirectCallPromotion.cpp
   Inliner.cpp
-  InsertNegateRAStatePass.cpp
   Instrumentation.cpp
   JTFootprintReduction.cpp
   LongJmp.cpp
   LoopInversionPass.cpp
   LivenessAnalysis.cpp
   MCF.cpp
-  MarkRAStates.cpp
   PatchEntries.cpp
   PAuthGadgetScanner.cpp
   PettisAndHansen.cpp
   PLTCall.cpp
+  PointerAuthCFIAnalyzer.cpp
+  PointerAuthCFIFixup.cpp
   ProfileQualityStats.cpp
   RegAnalysis.cpp
   RegReAssign.cpp
diff --git a/bolt/lib/Passes/MarkRAStates.cpp b/bolt/lib/Passes/PointerAuthCFIAnalyzer.cpp
similarity index 91%
rename from bolt/lib/Passes/MarkRAStates.cpp
rename to bolt/lib/Passes/PointerAuthCFIAnalyzer.cpp
index 81d5a2257888a..247a03d0bbec9 100644
--- a/bolt/lib/Passes/MarkRAStates.cpp
+++ b/bolt/lib/Passes/PointerAuthCFIAnalyzer.cpp
@@ -1,4 +1,5 @@
-//===- bolt/Passes/MarkRAStates.cpp ---------------------------------===//
+//===- bolt/Passes/PointerAuthCFIAnalyzer.cpp
+//---------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,7 +7,7 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// This file implements the MarkRAStates class.
+// This file implements the PointerAuthCFIAnalyzer class.
 // Three CFIs have an influence on the RA State of an instruction:
 // - NegateRAState flips the RA State,
 // - RememberState pushes the RA State to a stack,
@@ -16,10 +17,10 @@
 // the RA State of each instruction, and save it as new MCAnnotations. The new
 // annotations are Signing, Signed, Authenticating and Unsigned. After
 // optimizations, .cfi_negate_ra_state CFIs are added to the places where the
-// state changes in InsertNegateRAStatePass.
+// state changes in PointerAuthCFIFixup.
 //
 //===----------------------------------------------------------------------===//
-#include "bolt/Passes/MarkRAStates.h"
+#include "bolt/Passes/PointerAuthCFIAnalyzer.h"
 #include "bolt/Core/BinaryFunction.h"
 #include "bolt/Core/ParallelUtilities.h"
 #include <cstdlib>
@@ -31,7 +32,7 @@ using namespace llvm;
 namespace llvm {
 namespace bolt {
 
-bool MarkRAStates::runOnFunction(BinaryFunction &BF) {
+bool PointerAuthCFIAnalyzer::runOnFunction(BinaryFunction &BF) {
 
   BinaryContext &BC = BF.getBinaryContext();
 
@@ -107,7 +108,7 @@ bool MarkRAStates::runOnFunction(BinaryFunction &BF) {
   return true;
 }
 
-Error MarkRAStates::runOnFunctions(BinaryContext &BC) {
+Error PointerAuthCFIAnalyzer::runOnFunctions(BinaryContext &BC) {
   std::atomic<uint64_t> FunctionsIgnored{0};
   ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
     if (!runOnFunction(BF)) {
@@ -129,8 +130,8 @@ Error MarkRAStates::runOnFunctions(BinaryContext &BC) {
 
   ParallelUtilities::runOnEachFunction(
       BC, ParallelUtilities::SchedulingPolicy::SP_INST_LINEAR, WorkFun,
-      SkipPredicate, "MarkRAStates");
-  BC.outs() << "BOLT-INFO: MarkRAStates ran on " << Total
+      SkipPredicate, "PointerAuthCFIAnalyzer");
+  BC.outs() << "BOLT-INFO: PointerAuthCFIAnalyzer ran on " << Total
             << " functions. Ignored " << FunctionsIgnored << " functions "
             << format("(%.2lf%%)", (100.0 * FunctionsIgnored) / Total)
             << " because of CFI inconsistencies\n";
diff --git a/bolt/lib/Passes/InsertNegateRAStatePass.cpp b/bolt/lib/Passes/PointerAuthCFIFixup.cpp
similarity index 92%
rename from bolt/lib/Passes/InsertNegateRAStatePass.cpp
rename to bolt/lib/Passes/PointerAuthCFIFixup.cpp
index dce634ae88d5a..15b1e2b3bff61 100644
--- a/bolt/lib/Passes/InsertNegateRAStatePass.cpp
+++ b/bolt/lib/Passes/PointerAuthCFIFixup.cpp
@@ -1,4 +1,4 @@
-//===- bolt/Passes/InsertNegateRAStatePass.cpp ----------------------------===//
+//===- bolt/Passes/PointerAuthCFIFixup.cpp --------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -11,7 +11,7 @@
 // instructions are different.
 //
 //===----------------------------------------------------------------------===//
-#include "bolt/Passes/InsertNegateRAStatePass.h"
+#include "bolt/Passes/PointerAuthCFIFixup.h"
 #include "bolt/Core/BinaryFunction.h"
 #include "bolt/Core/ParallelUtilities.h"
 #include <cstdlib>
@@ -21,7 +21,7 @@ using namespace llvm;
 namespace llvm {
 namespace bolt {
 
-void InsertNegateRAState::runOnFunction(BinaryFunction &BF) {
+void PointerAuthCFIFixup::runOnFunction(BinaryFunction &BF) {
   BinaryContext &BC = BF.getBinaryContext();
 
   if (BF.getState() == BinaryFunction::State::Empty)
@@ -30,7 +30,7 @@ void InsertNegateRAState::runOnFunction(BinaryFunction &BF) {
   if (BF.getState() != BinaryFunction::State::CFG &&
       BF.getState() != BinaryFunction::State::CFG_Finalized) {
     BC.outs() << "BOLT-INFO: no CFG for " << BF.getPrintName()
-              << " in InsertNegateRAStatePass\n";
+              << " in PointerAuthCFIFixup\n";
     return;
   }
 
@@ -71,7 +71,7 @@ void InsertNegateRAState::runOnFunction(BinaryFunction &BF) {
   }
 }
 
-void InsertNegateRAState::inferUnknownStates(BinaryFunction &BF) {
+void PointerAuthCFIFixup::inferUnknownStates(BinaryFunction &BF) {
   BinaryContext &BC = BF.getBinaryContext();
 
   // Fill in missing RAStates in simple cases (inside BBs).
@@ -85,7 +85,7 @@ void InsertNegateRAState::inferUnknownStates(BinaryFunction &BF) {
   fillUnknownBlocksInCFG(BF);
 }
 
-void InsertNegateRAState::coverFunctionFragmentStart(BinaryFunction &BF,
+void PointerAuthCFIFixup::coverFunctionFragmentStart(BinaryFunction &BF,
                                                      FunctionFragment &FF) {
   BinaryContext &BC = BF.getBinaryContext();
   if (FF.empty())
@@ -117,7 +117,7 @@ void InsertNegateRAState::coverFunctionFragmentStart(BinaryFunction &BF,
 }
 
 std::optional<bool>
-InsertNegateRAState::getFirstKnownRAState(BinaryContext &BC,
+PointerAuthCFIFixup::getFirstKnownRAState(BinaryContext &BC,
                                           BinaryBasicBlock &BB) {
   for (const MCInst &Inst : BB) {
     if (BC.MIB->isCFI(Inst))
@@ -129,7 +129,7 @@ InsertNegateRAState::getFirstKnownRAState(BinaryContext &BC,
   return std::nullopt;
 }
 
-void InsertNegateRAState::fillUnknownStateInBB(BinaryContext &BC,
+void PointerAuthCFIFixup::fillUnknownStateInBB(BinaryContext &BC,
                                                BinaryBasicBlock &BB) {
 
   auto First = BB.getFirstNonPseudo();
@@ -171,7 +171,7 @@ void InsertNegateRAState::fillUnknownStateInBB(BinaryContext &BC,
   }
 }
 
-bool InsertNegateRAState::isUnknownBlock(BinaryContext &BC,
+bool PointerAuthCFIFixup::isUnknownBlock(BinaryContext &BC,
                                          BinaryBasicBlock &BB) {
   for (const MCInst &Inst : BB) {
     if (BC.MIB->isCFI(Inst))
@@ -183,7 +183,7 @@ bool InsertNegateRAState::isUnknownBlock(BinaryContext &BC,
   return true;
 }
 
-void InsertNegateRAState::markUnknownBlock(BinaryContext &BC,
+void PointerAuthCFIFixup::markUnknownBlock(BinaryContext &BC,
                                            BinaryBasicBlock &BB, bool State) {
   // If we call this when an Instruction has either kRASigned or kRAUnsigned
   // annotation, setRASigned or setRAUnsigned would fail.
@@ -196,7 +196,7 @@ void InsertNegateRAState::markUnknownBlock(BinaryContext &BC,
   }
 }
 
-std::optional<bool> InsertNegateRAState::getRAStateByCFG(BinaryBasicBlock &BB,
+std::optional<bool> PointerAuthCFIFixup::getRAStateByCFG(BinaryBasicBlock &BB,
                                                          BinaryFunction &BF) {
   BinaryContext &BC = BF.getBinaryContext();
 
@@ -242,7 +242,7 @@ std::optional<bool> InsertNegateRAState::getRAStateByCFG(BinaryBasicBlock &BB,
   return NeighborRAState;
 }
 
-void InsertNegateRAState::fillUnknownStubs(BinaryFunction &BF) {
+void PointerAuthCFIFixup::fillUnknownStubs(BinaryFunction &BF) {
   BinaryContext &BC = BF.getBinaryContext();
   bool FirstIter = true;
   MCInst PrevInst;
@@ -281,7 +281,7 @@ void InsertNegateRAState::fillUnknownStubs(BinaryFunction &BF) {
     }
   }
 }
-void InsertNegateRAState::fillUnknownBlocksInCFG(BinaryFunction &BF) {
+void PointerAuthCFIFixup::fillUnknownBlocksInCFG(BinaryFunction &BF) {
   BinaryContext &BC = BF.getBinaryContext();
 
   auto fillUnknowns = [&](BinaryFunction &BF) -> std::pair<int, bool> {
@@ -317,7 +317,7 @@ void InsertNegateRAState::fillUnknownBlocksInCFG(BinaryFunction &BF) {
   }
 }
 
-Error InsertNegateRAState::runOnFunctions(BinaryContext &BC) {
+Error PointerAuthCFIFixup::runOnFunctions(BinaryContext &BC) {
   std::atomic<uint64_t> FunctionsModified{0};
   ParallelUtilities::WorkFuncTy WorkFun = [&](BinaryFunction &BF) {
     FunctionsModified++;
@@ -334,7 +334,7 @@ Error InsertNegateRAState::runOnFunctions(BinaryContext &BC) {
 
   ParallelUtilities::runOnEachFunction(
       BC, ParallelUtilities::SchedulingPolicy::SP_INST_LINEAR, WorkFun,
-      SkipPredicate, "InsertNegateRAStatePass");
+      SkipPredicate, "PointerAuthCFIFixup");
 
   BC.outs() << "BOLT-INFO: rewritten pac-ret DWARF info in "
             << FunctionsModified << " out of " << BC.getBinaryFunctions().size()
diff --git a/bolt/lib/Rewrite/BinaryPassManager.cpp b/bolt/lib/Rewrite/BinaryPassManager.cpp
index 782137e807662..f98cb9b3c33ba 100644
--- a/bolt/lib/Rewrite/BinaryPassManager.cpp
+++ b/bolt/lib/Rewrite/BinaryPassManager.cpp
@@ -19,15 +19,15 @@
 #include "bolt/Passes/IdenticalCodeFolding.h"
 #include "bolt/Passes/IndirectCallPromotion.h"
 #include "bolt/Passes/Inliner.h"
-#include "bolt/Passes/InsertNegateRAStatePass.h"
 #include "bolt/Passes/Instrumentation.h"
 #include "bolt/Passes/JTFootprintReduction.h"
 #include "bolt/Passes/LongJmp.h"
 #include "bolt/Passes/LoopInversionPass.h"
 #include "bolt/Passes/MCF.h"
-#include "bolt/Passes/MarkRAStates.h"
 #include "bolt/Passes/PLTCall.h"
 #include "bolt/Passes/PatchEntries.h"
+#include "bolt/Passes/PointerAuthCFIAnalyzer.h"
+#include "bolt/Passes/PointerAuthCFIFixup.h"
 #include "bolt/Passes/ProfileQualityStats.h"
 #include "bolt/Passes/RegReAssign.h"
 #include "bolt/Passes/ReorderData.h"
@@ -362,7 +362,7 @@ Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
   BinaryFunctionPassManager Manager(BC);
 
   if (BC.isAArch64())
-    Manager.registerPass(std::make_unique<MarkRAStates>());
+    Manager.registerPass(std::make_unique<PointerAuthCFIAnalyzer>());
 
   Manager.registerPass(
       std::make_unique<EstimateEdgeCounts>(PrintEstimateEdgeCounts));
@@ -524,7 +524,7 @@ Error BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
     // relocations out of range and crash during linking.
     Manager.registerPass(std::make_unique<LongJmpPass>(PrintLongJmp));
 
-    Manager.registerPass(std::make_unique<InsertNegateRAState>());
+    Manager.registerPass(std::make_unique<PointerAuthCFIFixup>());
   }
 
   // This pass should always run last.*
diff --git a/bolt/test/AArch64/negate-ra-state-incorrect.s b/bolt/test/AArch64/negate-ra-state-incorrect.s
index 14d2c384a877d..390b3b824d6bc 100644
--- a/bolt/test/AArch64/negate-ra-state-incorrect.s
+++ b/bolt/test/AArch64/negate-ra-state-incorrect.s
@@ -1,4 +1,4 @@
-# This test checks that MarkRAStates pass ignores functions with
+# This test checks that PointerAuthCFIAnalyzer pass ignores functions with
 # malformed .cfi_negate_ra_state sequences in the input binary.
 
 # The cases checked are:
diff --git a/bolt/test/AArch64/negate-ra-state.s b/bolt/test/AArch64/negate-ra-state.s
index 30786d4ef9f70..d3206294a4f7e 100644
--- a/bolt/test/AArch64/negate-ra-state.s
+++ b/bolt/test/AArch64/negate-ra-state.s
@@ -9,13 +9,13 @@
 # RUN: llvm-bolt %t.exe -o %t.exe.bolt --no-threads --print-all | FileCheck %s --check-prefix=CHECK-BOLT
 
 # Check that the negate-ra-state at the start of bar is not discarded.
-# If it was discarded, MarkRAState would report bar as having inconsistent RAStates.
+# If it was discarded, PointerAuthCFIAnalyzer would report bar as having inconsistent RAStates.
 # This is testing the handling of initialRAState on the BinaryFunction.
 # CHECK-BOLT-NOT: BOLT-INFO: inconsistent RAStates in function foo
 # CHECK-BOLT-NOT: BOLT-INFO: inconsistent RAStates in function bar
 
 # Check that OpNegateRAState CFIs are generated correctly.
-# CHECK-BOLT: Binary Function "foo" after insert-negate-ra-state-pass {
+# CHECK-BOLT: Binary Function "foo" after pointer-auth-cfi-fixup {
 # CHECK-BOLT:         paciasp
 # CHECK-BOLT-NEXT:    OpNegateRAState
 
@@ -23,7 +23,7 @@
 # CHECK-BOLT-NEXT:     0:  OpNegateRAState
 # CHECK-BOLT-NEXT: End of Function "foo"
 
-# CHECK-BOLT: Binary Function "bar" after insert-negate-ra-state-pass {
+# CHECK-BOLT: Binary Function "bar" after pointer-auth-cfi-fixup {
 # CHECK-BOLT:         OpNegateRAState
 # CHECK-BOLT-NEXT:    mov     x1, #0x0
 # CHECK-BOLT-NEXT:    mov     x1, #0x1
@@ -37,7 +37,7 @@
 # CHECK-BOLT-NEXT: End of Function "bar"
 
 # End of negate-ra-state insertion logs for foo and bar.
-# CHECK: Binary Function "_start" after insert-negate-ra-...
[truncated]

@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-rename-pac-cfi-passes branch 2 times, most recently from 7113d9b to a05ee35 Compare October 22, 2025 14:37
@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-infer-unknown-states branch from 1ee52ed to 195e99c Compare October 28, 2025 11:52
@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-rename-pac-cfi-passes branch from a05ee35 to 66365dd Compare October 28, 2025 11:53
@github-actions
Copy link

github-actions bot commented Oct 28, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-rename-pac-cfi-passes branch from 66365dd to e6791c1 Compare October 28, 2025 12:18
@peterwaller-arm
Copy link
Contributor

I found a handful of references to MarkRAStates and one reference to InsertNegateRAStatePass:

bolt/lib/Core/Exceptions.cpp
575:        // out the state of each instruction in MarkRAStates Pass.
586:        // out the state of each instruction in MarkRAStates Pass.
655:        // instructions are worked out in MarkRAStates based on these
663:          // MarkRAStates' RAStateStack.

llvm/utils/gn/secondary/bolt/lib/Passes/BUILD.gn
33:    "InsertNegateRAStatePass.cpp",
40:    "MarkRAStates.cpp",

I used:

rg 'BOLT_PASSES_INSERT_NEGATE_RA_STATE_PASS|BOLT_PASSES_MARK_RA_STATES|InsertNegateRAStatePass|InsertNegateRAState|MarkRAStatesPass|MarkRAStates|MarkRAState|insert\-negate\-ra\-state\-pass|mark\-ra\-states'

To check for any references to names removed in your patch.

@bgergely0
Copy link
Contributor Author

Thanks for catching these!

For the BUILD.gn, I'm not sure if I should change these: they were added by "LLVM GN Syncbot" in 008294c0ccec9, I assume it is able to change them as well?

@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-rename-pac-cfi-passes branch from e6791c1 to 3d0b614 Compare October 28, 2025 14:12
Copy link
Contributor Author

Updated the rest.

@peterwaller-arm
Copy link
Contributor

There is a sync bot but you save it creating a commit if you fix it here. It will save some thrashing if it is a part of your patch in the unlucky event it gets reverted. It's not mandatory to fix the gn build but there is also not hard to keep it up to date in this case.

@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-rename-pac-cfi-passes branch 3 times, most recently from 46d7fee to c12cf77 Compare October 31, 2025 15:28
@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-infer-unknown-states branch from 790e325 to 9f5be17 Compare October 31, 2025 15:28
@bgergely0 bgergely0 marked this pull request as draft October 31, 2025 15:36
@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-rename-pac-cfi-passes branch from c12cf77 to 2b0fe49 Compare October 31, 2025 15:42
@bgergely0 bgergely0 marked this pull request as ready for review October 31, 2025 15:55
@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-rename-pac-cfi-passes branch from 2b0fe49 to f2f3e86 Compare November 3, 2025 13:10
@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-infer-unknown-states branch from 9f5be17 to ad33b18 Compare November 3, 2025 13:10
Original names were "working titles". After initial patches are merged,
I'd like to rename these passes to names that reflect their intent
better and show their relationship to each other:

InsertNegateRAStatePass renamed to PointerAuthCFIFixup,
MarkRAStates renamed to PointerAuthCFIAnalyzer.
@bgergely0 bgergely0 force-pushed the users/bgergely0/bolt-rename-pac-cfi-passes branch from f2f3e86 to 77a0b64 Compare November 3, 2025 13:25
Copy link
Member

@paschalis-mpeis paschalis-mpeis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for the renaming, Gergely.

Could you rework a bit the commit description?

Since everything now falls under PointerAuth*, should we also rename the PacRetDesign.md for consistency? We could also consider prefixing tests with pauth-* to keep them close to each other?

Regarding BUILD.gn: IIRC the last time I modified it, I was told ~
it's better to let the syncbot do it, though a manual change shouldn't cause any issues.

Copy link
Contributor Author

bgergely0 commented Nov 3, 2025

I will make the commit message a bit more formal.

I agree that naming all the unittests a bit more similarly is good addition to this.

One piece of doubt that I have since I opened this PR: the term CFI is overloaded, it can mean Call Frame Information and Control Flow Integrity. This is even more problematic than usually in this case, where we talk about the Call Frame Information related to a Control Flow Integrity mechanism, namely Pointer Authentication.

Another way to name these would be PointerAuthDwarf* (or capital DWARF*), to signal that these passes take care of the DWARF CFIs in the binary.

Thoughts?

@paschalis-mpeis
Copy link
Member

One piece of doubt that I have since I opened this PR: the term CFI is overloaded, it can mean Call Frame Information and Control Flow Integrity. This is even more problematic than usually in this case, where we talk about the Call Frame Information related to a Control Flow Integrity mechanism, namely Pointer Authentication.

Another way to name these would be PointerAuthDwarf* (or capital DWARF*), to signal that these passes take care of the DWARF CFIs in the binary.

That is a good point. Generalizing to DWARF may help, but I don't have strong opinions.
If the name feels too long, can consider PAuth<..>.

It may be worth briefly describing both terms in the doc (as a hint / callout near the top) and expanding the CFI acronyms in both *Analyzer and *Fixup headers (ie here).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants