[llvm-objdump][RISCV] Print target of auipc+jalr calls

[jobnoorman]

Summary

Currently, the targets of auipc+jalr calls are not printed because
MCInstrAnalysis fails to analyze them. This patch implements this as follows:

• Add a new MCInstrAnalysis::evaluateBranch overload that takes two consecutive instructions as arguments. This allows the RISC-V backend to detect auipc+jalr sequences.
• While disassembling in llvm-objdump, keep track of the previous instruction and call the new evaluateBranch overload.

Note that the new evaluateBranch overload could have been avoided by making
PrevInst an argument of the existing version and giving it a default value.
However, this would have made it necessary to update nearly all targets so I
will keep this refactoring to a future patch.

[arichardson]

llvm/include/llvm/MC/MCInstrAnalysis.h

173

How about passing the optional PrevInst to to the existing evaluateBranch() instead? Having to choose which overload to implement is bit awkward and means two calls per instruction instead of just one.

[jobnoorman]

Update the evaluateBranch overload to take PrevInst as a std::optional.
The default implementation falls back to the existing evaluateBranch. This
simplifies its use in llvm-objdump as now only a single evaluateBranch call is
needed.

Thanks for the suggestion @arichardson! Note that I didn't add the std::optional
argument to the existing evaluateBranch but created a new overload instead.
This is to avoid having to update nearly all targets in this patch. I will
propose a refactoring once this gets accepted.

[MaskRay]

I think in recent years @asb has a similar patch in this area but I cannot find it...

[jobnoorman]

In D159114#4630123, @MaskRay wrote:

I think in recent years @asb has a similar patch in this area but I cannot find it...

Indeed, it's D116677. I've opened #65479 to implement the state idea discussed there and #65480 to implement it for auipc+jalr on RISC-V.

I will close this review in favor of the new PRs.

llvm/include/llvm/MC/MCInstrAnalysis.h

public:
   }
 
   /// Given a branch instruction try to get the address the branch
   /// targets. Return true on success, and the address in Target.
   virtual bool
   evaluateBranch(const MCInst &Inst, uint64_t Addr, uint64_t Size,
                  uint64_t &Target) const;
 
+  /// Given a branch instruction try to get the address the branch targets.
+  /// Return true on success, and the address in Target. If the previous
+  /// instruction is known, it may be passed in PrevInst, in which case Addr is
+  /// still the address of Inst. This is useful for targets like RISC-V where
+  /// some branches take up two instructions (e.g. auipc+jalr).
+  virtual bool evaluateBranch(const MCInst &Inst, uint64_t Addr, uint64_t Size,
+                              uint64_t &Target,
+                              const std::optional<MCInst> &PrevInst) const;
+
   /// Given an instruction tries to get the address of a memory operand. Returns
   /// the address on success.
   virtual std::optional<uint64_t>
   evaluateMemoryOperandAddress(const MCInst &Inst, const MCSubtargetInfo *STI,
                                uint64_t Addr, uint64_t Size) const;
 
   /// Given an instruction with a memory operand that could require relocation,
   /// returns the offset within the instruction of that relocation.

llvm/lib/MC/MCInstrAnalysis.cpp

 }
 
 bool MCInstrAnalysis::evaluateBranch(const MCInst & /*Inst*/, uint64_t /*Addr*/,
                                      uint64_t /*Size*/,
                                      uint64_t & /*Target*/) const {
   return false;
 }
 
+bool MCInstrAnalysis::evaluateBranch(
+    const MCInst &Inst, uint64_t Addr, uint64_t Size, uint64_t &Target,
+    const std::optional<MCInst> &PrevInst) const {
+  return evaluateBranch(Inst, Addr, Size, Target);
+}
+
 std::optional<uint64_t> MCInstrAnalysis::evaluateMemoryOperandAddress(
     const MCInst &Inst, const MCSubtargetInfo *STI, uint64_t Addr,
     uint64_t Size) const {
   return std::nullopt;
 }
 
 std::optional<uint64_t>
 MCInstrAnalysis::getMemoryOperandRelocationOffset(const MCInst &Inst,
                                                   uint64_t Size) const {
   return std::nullopt;
 }

llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp

 
 class RISCVMCInstrAnalysis : public MCInstrAnalysis {
 public:
   explicit RISCVMCInstrAnalysis(const MCInstrInfo *Info)
       : MCInstrAnalysis(Info) {}
 
   bool evaluateBranch(const MCInst &Inst, uint64_t Addr, uint64_t Size,
                       uint64_t &Target) const override {
+    return evaluateBranch(Inst, Addr, Size, Target, std::nullopt);
+  }
+
+  bool evaluateBranch(const MCInst &Inst, uint64_t Addr, uint64_t Size,
+                      uint64_t &Target,
+                      const std::optional<MCInst> &PrevInst) const override {
     if (isConditionalBranch(Inst)) {
       int64_t Imm;
       if (Size == 2)
         Imm = Inst.getOperand(1).getImm();
       else
         Imm = Inst.getOperand(2).getImm();
       Target = Addr + Imm;
       return true;
     }
 
     if (Inst.getOpcode() == RISCV::C_JAL || Inst.getOpcode() == RISCV::C_J) {
       Target = Addr + Inst.getOperand(0).getImm();
       return true;
     }
 
     if (Inst.getOpcode() == RISCV::JAL) {
       Target = Addr + Inst.getOperand(1).getImm();
       return true;
     }
 
+    // Detect auipc xi, a; jalr b(xi). Note that Addr is the address of Inst
+    // (jalr) so we have to subtract 4 from it.
+    if (PrevInst && PrevInst->getOpcode() == RISCV::AUIPC &&
+        Inst.getOpcode() == RISCV::JALR &&
+        PrevInst->getOperand(0).getReg() == Inst.getOperand(1).getReg()) {
+      Target = Addr - 4 + (PrevInst->getOperand(1).getImm() << 12) +
+               Inst.getOperand(2).getImm();
+      return true;
+    }
+
     return false;
   }
 
   bool isTerminator(const MCInst &Inst) const override {
     if (MCInstrAnalysis::isTerminator(Inst))
       return true;
 
     switch (Inst.getOpcode()) {

llvm/test/tools/llvm-objdump/ELF/RISCV/branches.s

 jal t0, bar
 # CHECK: c.jal 0x60 <bar>
 c.jal bar
 
 # CHECK: c.j 0x60 <bar>
 c.j bar
 
 # CHECK: auipc ra, 0
-# CHECK: jalr ra, 16(ra){{$}}
+# CHECK: jalr ra, 16(ra) <foo+0x58>
 call .Llocal
 
 # CHECK: auipc ra, 0
-# CHECK: jalr ra, 16(ra){{$}}
+# CHECK: jalr ra, 16(ra) <bar>
 call bar
 
 .Llocal:
 # CHECK: 58: c.nop
 # CHECK: c.nop
 # CHECK: c.nop
 # CHECK: c.nop
 nop
 nop
 nop
 nop
 
 # CHECK-LABEL: <bar>:
 bar:
 # CHECK: 60: c.nop
 nop

llvm/tools/llvm-objdump/llvm-objdump.cpp

for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
         collectLocalBranchTargets(Bytes, DT->InstrAnalysis.get(),
                                   DT->DisAsm.get(), DT->InstPrinter.get(),
                                   PrimaryTarget.SubtargetInfo.get(),
                                   SectionAddr, Index, End, AllLabels);
         collectBBAddrMapLabels(AddrToBBAddrMap, SectionAddr, Index, End,
                                BBAddrMapLabels);
       }
 
+      std::optional<MCInst> PrevInst;
       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
         // we need to dump. If the data marker is within a function, it is
         // denoted as a word/short etc.
         if (!MappingSymbols.empty()) {
           char Kind = getMappingSymbolKind(MappingSymbols, Index);
           DumpARMELFData = Kind == 'd';
for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
           // instruction information. Otherwise, try to resolve the target
           // address (jump target or memory operand address) and print it on the
           // right of the instruction.
           if (Disassembled && DT->InstrAnalysis) {
             // Branch targets are printed just after the instructions.
             llvm::raw_ostream *TargetOS = &FOS;
             uint64_t Target;
             bool PrintTarget = DT->InstrAnalysis->evaluateBranch(
-                Inst, SectionAddr + Index, Size, Target);
+                Inst, SectionAddr + Index, Size, Target, PrevInst);
             if (!PrintTarget)
               if (std::optional<uint64_t> MaybeTarget =
                       DT->InstrAnalysis->evaluateMemoryOperandAddress(
                           Inst, DT->SubtargetInfo.get(), SectionAddr + Index,
                           Size)) {
                 Target = *MaybeTarget;
                 PrintTarget = true;
                 // Do not print real address when symbolizing.
for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
                 *TargetOS << " <" << AllLabels[Target] << ">";
               }
               // By convention, each record in the comment stream should be
               // terminated.
               if (TargetOS == &CommentStream)
                 *TargetOS << "\n";
             }
           }
+
+          PrevInst = Inst;
         }
 
         assert(DT->Context->getAsmInfo());
         emitPostInstructionInfo(FOS, *DT->Context->getAsmInfo(),
                                 *DT->SubtargetInfo, CommentStream.str(), LVP);
         Comments.clear();
 
         // Hexagon does this in pretty printer