[llvm] Minimal unwinding information (DWARF CFI) checker (PR #145633)

[Simon Tatham via llvm-commits]

================
@@ -0,0 +1,306 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/DWARFCFIChecker/DWARFCFIAnalysis.h"
+#include "Registers.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/DWARFCFIChecker/DWARFCFIState.h"
+#include "llvm/DebugInfo/DWARF/DWARFDebugFrame.h"
+#include "llvm/MC/MCAsmInfo.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCDwarf.h"
+#include "llvm/MC/MCExpr.h"
+#include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstrInfo.h"
+#include "llvm/MC/MCRegister.h"
+#include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCStreamer.h"
+#include "llvm/MC/MCSubtargetInfo.h"
+#include "llvm/MC/TargetRegistry.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
+#include <optional>
+
+using namespace llvm;
+
+struct CFARegOffsetInfo {
+  DWARFRegNum Reg;
+  int64_t Offset;
+
+  CFARegOffsetInfo(DWARFRegNum Reg, int64_t Offset)
+      : Reg(Reg), Offset(Offset) {}
+};
+
+static std::optional<CFARegOffsetInfo>
+getCFARegOffsetInfo(const dwarf::UnwindTable::const_iterator &UnwindRow) {
+  auto CFALocation = UnwindRow->getCFAValue();
+  if (CFALocation.getLocation() !=
+      dwarf::UnwindLocation::Location::RegPlusOffset) {
+    return std::nullopt;
+  }
+
+  return CFARegOffsetInfo(CFALocation.getRegister(), CFALocation.getOffset());
+}
+
+static std::optional<DWARFRegNum>
+getUnwindRuleRefReg(const dwarf::UnwindTable::const_iterator &UnwindRow,
+                    DWARFRegNum Reg) {
+  auto MaybeLoc = UnwindRow->getRegisterLocations().getRegisterLocation(Reg);
+  assert(MaybeLoc &&
+         "The register should be tracked inside the register states");
+  auto Loc = *MaybeLoc;
+
+  switch (Loc.getLocation()) {
+  case dwarf::UnwindLocation::Location::Undefined:
+  case dwarf::UnwindLocation::Location::Constant:
+  case dwarf::UnwindLocation::Location::Unspecified:
+  case dwarf::UnwindLocation::Location::DWARFExpr:
+    // TODO: here should look into expr and find the registers.
+    return std::nullopt;
+  case dwarf::UnwindLocation::Location::Same:
+    return Reg;
+  case dwarf::UnwindLocation::Location::RegPlusOffset:
+    return Loc.getRegister();
+  case dwarf::UnwindLocation::Location::CFAPlusOffset:
+    auto MaybeCFA = getCFARegOffsetInfo(UnwindRow);
+    if (MaybeCFA)
+      return MaybeCFA->Reg;
+    return std::nullopt;
+  }
+}
+
+DWARFCFIAnalysis::DWARFCFIAnalysis(MCContext *Context, MCInstrInfo const &MCII,
+                                   bool IsEH,
+                                   ArrayRef<MCCFIInstruction> Prologue)
+    : Context(Context), MCII(MCII), MCRI(Context->getRegisterInfo()),
+      State(Context), IsEH(IsEH) {
+
+  for (auto LLVMReg : getTrackingRegs(MCRI)) {
+    if (MCRI->get(LLVMReg).IsArtificial || MCRI->get(LLVMReg).IsConstant)
+      continue;
+
+    DWARFRegNum Reg = MCRI->getDwarfRegNum(LLVMReg, IsEH);
+    // TODO: this should be `undefined` instead of `same_value`, but because
+    // initial frame state doesn't have any directives about callee saved
+    // registers, every register is tracked. After initial frame state is
+    // corrected, this should be changed.
+    State.update(MCCFIInstruction::createSameValue(nullptr, Reg));
+  }
+
+  // TODO: Ignoring PC should be in the initial frame state.
+  State.update(MCCFIInstruction::createUndefined(
+      nullptr, MCRI->getDwarfRegNum(MCRI->getProgramCounter(), IsEH)));
+
+  for (auto &&InitialFrameStateCFIDirective :
+       Context->getAsmInfo()->getInitialFrameState()) {
+    State.update(InitialFrameStateCFIDirective);
+  }
+
+  auto MaybeCurrentRow = State.getCurrentUnwindRow();
+  assert(MaybeCurrentRow && "there should be at least one row");
+  auto MaybeCFA = getCFARegOffsetInfo(*MaybeCurrentRow);
+  assert(MaybeCFA &&
+         "the CFA information should be describable in [Reg + Offset] in here");
+  auto CFA = *MaybeCFA;
+
+  // TODO: CFA register callee value is CFA's value, this should be in initial
+  // frame state.
+  State.update(MCCFIInstruction::createOffset(nullptr, CFA.Reg, 0));
+
+  // Applying the prologue after default assumptions to overwrite them.
+  for (auto &&Directive : Prologue) {
+    State.update(Directive);
+  }
+}
+
+void DWARFCFIAnalysis::update(const MCInst &Inst,
+                              ArrayRef<MCCFIInstruction> Directives) {
+  const MCInstrDesc &MCInstInfo = MCII.get(Inst.getOpcode());
+
+  auto MaybePrevRow = State.getCurrentUnwindRow();
+  assert(MaybePrevRow && "The analysis should have initialized the "
+                         "history with at least one row by now");
+  auto PrevRow = MaybePrevRow.value();
+
+  for (auto &&Directive : Directives)
+    State.update(Directive);
----------------
statham-arm wrote:

I think this is a memory safety error, isn't it?

You've just retrieved and saved `PrevRow`, which is an iterator pointing at the inside of `State.Table`, which is a typedef for `std::vector`. Then this `update` call pushes more things into `State.Table`, which might (but won't every time) cause the vector to reallocate itself at a different address. And then you expect `PrevRow` to still be valid at the bottom of the function, when you pass it to `checkCFADiff` and `checkRegDiff`.

Surely you've just invalidated the iterator by modifying the vector that it refers to?

(I'm sorry for not having spotted this in the previous review!)

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