[llvm] [MC][NFC] Allow MCInstrAnalysis to store state (PR #65479)

[Job Noorman via llvm-commits]

https://github.com/mtvec created https://github.com/llvm/llvm-project/pull/65479:

Currently, all the analysis functions provided by `MCInstrAnalysis` work on a single instruction. On some targets, this limits the kind of instructions that can be successfully analyzed as common constructs may need multiple instructions.

For example, a typical call sequence on RISC-V uses a auipc+jalr pair. In order to analyse the jalr inside `evaluateBranch`, information about the corresponding auipc is needed. Similarly, AArch64 uses adrp+ldr pairs to access globals.

This patch proposes to add state to `MCInstrAnalysis` to support these use cases. Two new virtual methods are added:
- `updateState`: takes an instruction and its address. This methods should be called by clients on every instruction and allows targets to store whatever information they need to analyse future instructions.
- `resetState`: clears the state whenever it becomes irrelevant. Clients could call this, for example, when starting to disassemble a new function.

Note that the default implementations do nothing so this patch is NFC. No actual state is stored inside `MCInstrAnalysis`; deciding the structure of the state is left to the targets.

This patch also modifies llvm-objdump to use the new interface.

This patch is an alternative to [D116677](https://reviews.llvm.org/D116677) and the idea of storing state in `MCInstrAnalysis` was first discussed there.

>From 524fed05e5f102f44ff2f1b3060b88f0633c3d81 Mon Sep 17 00:00:00 2001
From: Job Noorman <jnoorman at igalia.com>
Date: Wed, 6 Sep 2023 11:29:28 +0200
Subject: [PATCH] [MC][NFC] Allow MCInstrAnalysis to store state

Currently, all the analysis functions provided by `MCInstrAnalysis` work
on a single instruction. On some targets, this limits the kind of
instructions that can be successfully analyzed as common constructs may
need multiple instructions.

For example, a typical call sequence on RISC-V uses a auipc+jalr pair.
In order to analyse the jalr inside `evaluateBranch`, information about
the corresponding auipc is needed. Similarly, AArch64 uses adrp+ldr
pairs to access globals.

This patch proposes to add state to `MCInstrAnalysis` to support these
use cases. Two new virtual methods are added:
- `updateState`: takes an instruction and its address. This methods
  should be called by clients on every instruction and allows targets to
  store whatever information they need to analyse future instructions.
- `resetState`: clears the state whenever it becomes irrelevant. Clients
  could call this, for example, when starting to disassemble a new
  function.

Note that the default implementations do nothing so this patch is NFC.
No actual state is stored inside `MCInstrAnalysis`; deciding the
structure of the state is left to the targets.

This patch also modifies llvm-objdump to use the new interface.

This patch is an alternative to D116677 and the idea of storing state in
`MCInstrAnalysis` was first discussed there.
---
 llvm/include/llvm/MC/MCInstrAnalysis.h   | 15 +++++++++++++++
 llvm/tools/llvm-objdump/llvm-objdump.cpp | 18 +++++++++++++-----
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/MC/MCInstrAnalysis.h b/llvm/include/llvm/MC/MCInstrAnalysis.h
index c3c675c39c5590c..dac12af599e6f34 100644
--- a/llvm/include/llvm/MC/MCInstrAnalysis.h
+++ b/llvm/include/llvm/MC/MCInstrAnalysis.h
@@ -37,6 +37,21 @@ class MCInstrAnalysis {
   MCInstrAnalysis(const MCInstrInfo *Info) : Info(Info) {}
   virtual ~MCInstrAnalysis() = default;
 
+  /// Clear the internal state. See updateState for more information.
+  virtual void resetState() {}
+
+  /// Update internal state with \p Inst at \p Addr.
+  ///
+  /// For some types a analyses, inspecting a single instruction is not
+  /// sufficient. Some examples are auipc/jalr pairs on RISC-V or adrp/ldr pairs
+  /// on AArch64. To support inspecting multiple instructions, targets may keep
+  /// track of an internal state while analysing instructions. Clients should
+  /// call updateState for every instruction which allows later calls to one of
+  /// the analysis functions to take previous instructions into account.
+  /// Whenever state becomes irrelevant (e.g., when starting to disassemble a
+  /// new function), clients should call resetState to clear it.
+  virtual void updateState(const MCInst &Inst, uint64_t Addr) {}
+
   virtual bool isBranch(const MCInst &Inst) const {
     return Info->get(Inst.getOpcode()).isBranch();
   }
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
index 96d74d6e2d5e865..ac1b5878ce1d0a1 100644
--- a/llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ b/llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -842,7 +842,7 @@ class DisassemblerTarget {
   std::unique_ptr<const MCSubtargetInfo> SubtargetInfo;
   std::shared_ptr<MCContext> Context;
   std::unique_ptr<MCDisassembler> DisAsm;
-  std::shared_ptr<const MCInstrAnalysis> InstrAnalysis;
+  std::shared_ptr<MCInstrAnalysis> InstrAnalysis;
   std::shared_ptr<MCInstPrinter> InstPrinter;
   PrettyPrinter *Printer;
 
@@ -1265,15 +1265,18 @@ collectBBAddrMapLabels(const std::unordered_map<uint64_t, BBAddrMap> &AddrToBBAd
   }
 }
 
-static void collectLocalBranchTargets(
-    ArrayRef<uint8_t> Bytes, const MCInstrAnalysis *MIA, MCDisassembler *DisAsm,
-    MCInstPrinter *IP, const MCSubtargetInfo *STI, uint64_t SectionAddr,
-    uint64_t Start, uint64_t End, std::unordered_map<uint64_t, std::string> &Labels) {
+static void
+collectLocalBranchTargets(ArrayRef<uint8_t> Bytes, MCInstrAnalysis *MIA,
+                          MCDisassembler *DisAsm, MCInstPrinter *IP,
+                          const MCSubtargetInfo *STI, uint64_t SectionAddr,
+                          uint64_t Start, uint64_t End,
+                          std::unordered_map<uint64_t, std::string> &Labels) {
   // So far only supports PowerPC and X86.
   if (!STI->getTargetTriple().isPPC() && !STI->getTargetTriple().isX86())
     return;
 
   Labels.clear();
+  MIA->resetState();
   unsigned LabelCount = 0;
   Start += SectionAddr;
   End += SectionAddr;
@@ -1298,6 +1301,7 @@ static void collectLocalBranchTargets(
           !Labels.count(Target) &&
           !(STI->getTargetTriple().isPPC() && Target == Index))
         Labels[Target] = ("L" + Twine(LabelCount++)).str();
+      MIA->updateState(Inst, Index);
     }
     Index += Size;
   }
@@ -1939,6 +1943,8 @@ disassembleObject(ObjectFile &Obj, const ObjectFile &DbgObj,
                                BBAddrMapLabels);
       }
 
+      DT->InstrAnalysis->resetState();
+
       while (Index < End) {
         // ARM and AArch64 ELF binaries can interleave data and text in the
         // same section. We rely on the markers introduced to understand what
@@ -2156,6 +2162,8 @@ disassembleObject(ObjectFile &Obj, const ObjectFile &DbgObj,
                 *TargetOS << "\n";
             }
           }
+
+          DT->InstrAnalysis->updateState(Inst, SectionAddr + Index);
         }
 
         assert(DT->Context->getAsmInfo());