[llvm] [BOLT] Improve DWARF CFI generation for pac-ret binaries (PR #163381)

[Peter Waller via llvm-commits]

================
@@ -104,32 +118,120 @@ void InsertNegateRAState::coverFunctionFragmentStart(BinaryFunction &BF,
                          MCCFIInstruction::createNegateRAState(nullptr));
 }
 
-void InsertNegateRAState::inferUnknownStates(BinaryFunction &BF) {
+std::optional<bool>
+InsertNegateRAState::getFirstKnownRAState(BinaryContext &BC,
+                                          BinaryBasicBlock &BB) {
+  for (const MCInst &Inst : BB) {
+    if (BC.MIB->isCFI(Inst))
+      continue;
+    auto RAStateOpt = BC.MIB->getRAState(Inst);
+    if (RAStateOpt)
+      return RAStateOpt;
+  }
+  return std::nullopt;
+}
+
+void InsertNegateRAState::fillUnknownStateInBB(BinaryContext &BC,
+                                               BinaryBasicBlock &BB) {
+
+  auto First = BB.getFirstNonPseudo();
+  if (First == BB.end())
+    return;
+  // If the first instruction has unknown RAState, we should copy the first
+  // known RAState.
+  auto RAStateOpt = BC.MIB->getRAState(*First);
+  if (!RAStateOpt) {
+    auto FirstRAState = getFirstKnownRAState(BC, BB);
+    if (!FirstRAState)
+      // We fill unknown BBs later.
+      return;
+
+    BC.MIB->setRAState(*First, *FirstRAState);
+  }
+
+  // At this point we know the RAState of the first instruction,
+  // so we can propagate the RAStates to all subsequent unknown instructions.
+  MCInst Prev = *First;
+  for (auto It = BB.begin() + 1; It != BB.end(); ++It) {
+    MCInst &Inst = *It;
+    if (BC.MIB->isCFI(Inst))
+      continue;
+
+    // No need to check for nullopt: we only entered this loop after the first
+    // instruction had its RAState set, and RAState is always set for the
+    // previous instruction in the previous iteration of the loop.
+    auto PrevRAState = BC.MIB->getRAState(Prev);
----------------
peterwaller-arm wrote:

The LLVM styleguide recommends use of auto only where it makes code clearer:

https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

Since the type is an optional<bool> and optional<bool> has its own `bool()` type-cast operator which means `has_value`, I think use of `optional<bool>` in conjunction with `auto` makes it a bit harder to parse intent.

Perhaps I might even write `if (!RAState.has_value())` below to make it explicit you're checking the presence of the state rather than the state.

I'd eliminate most of your use of auto (spelling out the optional type). Use of auto for iterators is generally OK though.



https://github.com/llvm/llvm-project/pull/163381