[BOLT][RISCV] Add target symbolizer for relocations

bolt/lib/Core/BinaryFunction.cpp

@@ -1330,13 +1330,6 @@ Error BinaryFunction::disassemble() {
   // basic block.
   Labels[0] = Ctx->createNamedTempSymbol("BB0");
 
-  // Map offsets in the function to a label that should always point to the
-  // corresponding instruction. This is used for labels that shouldn't point to
-  // the start of a basic block but always to a specific instruction. This is
-  // used, for example, on RISC-V where %pcrel_lo relocations point to the
-  // corresponding %pcrel_hi.
-  LabelsMapType InstructionLabels;
-
   uint64_t Size = 0; // instruction size
   for (uint64_t Offset = 0; Offset < getSize(); Offset += Size) {
     MCInst Instruction;
@@ -1501,42 +1494,6 @@ Error BinaryFunction::disassemble() {
         if (BC.isAArch64())
           handleAArch64IndirectCall(Instruction, Offset);
       }
-    } else if (BC.isRISCV()) {
-      // Check if there's a relocation associated with this instruction.
-      for (auto Itr = Relocations.lower_bound(Offset),
-                ItrE = Relocations.lower_bound(Offset + Size);
-           Itr != ItrE; ++Itr) {
-        const Relocation &Relocation = Itr->second;
-        MCSymbol *Symbol = Relocation.Symbol;
-
-        if (Relocation::isInstructionReference(Relocation.Type)) {
-          uint64_t RefOffset = Relocation.Value - getAddress();
-          LabelsMapType::iterator LI = InstructionLabels.find(RefOffset);
-
-          if (LI == InstructionLabels.end()) {
-            Symbol = BC.Ctx->createNamedTempSymbol();
-            InstructionLabels.emplace(RefOffset, Symbol);
-          } else {
-            Symbol = LI->second;
-          }
-        }
-
-        uint64_t Addend = Relocation.Addend;
-
-        // For GOT relocations, create a reference against GOT entry ignoring
-        // the relocation symbol.
-        if (Relocation::isGOT(Relocation.Type)) {
-          assert(Relocation::isPCRelative(Relocation.Type) &&
-                 "GOT relocation must be PC-relative on RISC-V");
-          Symbol = BC.registerNameAtAddress("__BOLT_got_zero", 0, 0, 0);
-          Addend = Relocation.Value + Relocation.Offset + getAddress();
-        }
-        int64_t Value = Relocation.Value;
-        const bool Result = BC.MIB->replaceImmWithSymbolRef(
-            Instruction, Symbol, Addend, Ctx.get(), Value, Relocation.Type);
-        (void)Result;
-        assert(Result && "cannot replace immediate with relocation");
-      }
     }
 
 add_instruction:
@@ -1578,13 +1535,6 @@ Error BinaryFunction::disassemble() {
   // Scope-boundary markers are only consulted while assigning offsets above.
   DebugScopeBoundaryOffsets.clear();
 
-  for (auto [Offset, Label] : InstructionLabels) {
-    InstrMapType::iterator II = Instructions.find(Offset);
-    assert(II != Instructions.end() && "reference to non-existing instruction");
-
-    BC.MIB->setInstLabel(II->second, Label);
-  }
-
   // Reset symbolizer for the disassembler.
   BC.SymbolicDisAsm->setSymbolizer(nullptr);
 

bolt/lib/Core/Relocation.cpp

@@ -892,7 +892,7 @@ bool Relocation::isTLS(uint32_t Type) {
 }
 
 bool Relocation::isInstructionReference(uint32_t Type) {
-  if (Arch != Triple::riscv64)
+  if (Arch != Triple::riscv64 && Arch != Triple::riscv32)
     return false;
 
   switch (Type) {

bolt/lib/Target/RISCV/CMakeLists.txt

@@ -1,5 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   MC
+  MCDisassembler
   Support
   RISCVDesc
   )
@@ -19,6 +20,7 @@ endif()
 
 add_llvm_library(LLVMBOLTTargetRISCV
   RISCVMCPlusBuilder.cpp
+  RISCVMCSymbolizer.cpp
 
   NO_EXPORT
   DISABLE_LLVM_LINK_LLVM_DYLIB

bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp

@@ -12,6 +12,7 @@
 
 #include "MCTargetDesc/RISCVMCAsmInfo.h"
 #include "MCTargetDesc/RISCVMCTargetDesc.h"
+#include "RISCVMCSymbolizer.h"
 #include "bolt/Core/MCPlusBuilder.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/MC/MCContext.h"
@@ -39,6 +40,12 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
 public:
   using MCPlusBuilder::MCPlusBuilder;
 
+  std::unique_ptr<MCSymbolizer>
+  createTargetSymbolizer(BinaryFunction &Function,
+                         bool CreateNewSymbols) const override {
+    return std::make_unique<RISCVMCSymbolizer>(Function, CreateNewSymbols);
+  }
+
   bool equals(const MCSpecifierExpr &A, const MCSpecifierExpr &B,
               CompFuncTy Comp) const override {
     const auto &RISCVExprA = cast<MCSpecifierExpr>(A);

bolt/lib/Target/RISCV/RISCVMCSymbolizer.cpp

@@ -0,0 +1,150 @@
+//===- bolt/Target/RISCV/RISCVMCSymbolizer.cpp ----------------------------===//
+//
+// 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 "RISCVMCSymbolizer.h"
+#include "bolt/Core/BinaryContext.h"
+#include "bolt/Core/BinaryFunction.h"
+#include "bolt/Core/MCPlusBuilder.h"
+#include "bolt/Core/Relocation.h"
+#include "llvm/BinaryFormat/ELF.h"
+#include "llvm/MC/MCInst.h"
+
+#define DEBUG_TYPE "bolt-symbolizer"
+
+namespace llvm {
+namespace bolt {
+
+RISCVMCSymbolizer::RISCVMCSymbolizer(BinaryFunction &Function,
+                                     bool CreateNewSymbols)
+    : MCSymbolizer(*Function.getBinaryContext().Ctx, nullptr),
+      Function(Function), CreateNewSymbols(CreateNewSymbols) {
+  // Discover instruction references before decoding starts. This lets us
+  // attach a label while decoding the referenced %pcrel_hi instruction even
+  // though its %pcrel_lo user is normally decoded later.
+  for (uint64_t SearchOffset = 0; SearchOffset < Function.getSize();) {
+    const Relocation *Rel =
+        Function.getRelocationInRange(SearchOffset, Function.getSize());
+    if (!Rel)
+      break;
+
+    if (Relocation::isInstructionReference(Rel->Type)) {
+      assert(Rel->Value >= Function.getAddress() &&
+             Rel->Value < Function.getAddress() + Function.getSize() &&
+             "RISC-V instruction reference outside of function");
+      const uint64_t ReferencedOffset = Rel->Value - Function.getAddress();
+      InstructionReferences.try_emplace(ReferencedOffset, Rel);
+      if (CreateNewSymbols)
+        InstructionLabels.try_emplace(ReferencedOffset, nullptr);
+    }
+
+    SearchOffset = Rel->Offset + 1;
+  }
+}
+
+RISCVMCSymbolizer::~RISCVMCSymbolizer() {}
+
+MCSymbol *RISCVMCSymbolizer::getOrCreateInstructionLabel(uint64_t Offset) {
+  auto [It, Inserted] = InstructionLabels.try_emplace(Offset, nullptr);
+  (void)Inserted;
+  if (!It->second)
+    It->second = Ctx.createNamedTempSymbol();
+  return It->second;
+}
+
+uint64_t RISCVMCSymbolizer::getGOTValue(const Relocation &Rel) const {
+  BinaryContext &BC = Function.getBinaryContext();
+  const uint64_t HiAddress = Function.getAddress() + Rel.Offset;
+
+  // A GOT high relocation records a combined high/low value. Locate the low
+  // relocation by its reference back to this AUIPC instead of assuming that
+  // the low instruction is adjacent.
+  auto It = InstructionReferences.find(Rel.Offset);
+  if (It != InstructionReferences.end()) {
+    const Relocation *LoRel = It->second;
+    ErrorOr<uint64_t> HiContents = BC.getUnsignedValueAtAddress(HiAddress, 4);
+    ErrorOr<uint64_t> LoContents =
+        BC.getUnsignedValueAtAddress(Function.getAddress() + LoRel->Offset,
+                                     Relocation::getSizeForType(LoRel->Type));
+    assert(HiContents && LoContents &&
+           "cannot read RISC-V GOT relocation pair");
+
+    return Relocation::extractValue(ELF::R_RISCV_PCREL_HI20, *HiContents,
+                                    HiAddress) +
+           Relocation::extractValue(LoRel->Type, *LoContents,
+                                    Function.getAddress() + LoRel->Offset);
+  }
+
+  return Rel.Value;
+}
+
+bool RISCVMCSymbolizer::tryAddingSymbolicOperand(
+    MCInst &Inst, raw_ostream &CStream, int64_t Value, uint64_t InstAddress,
+    bool IsBranch, uint64_t ImmOffset, uint64_t ImmSize, uint64_t InstSize) {
+  BinaryContext &BC = Function.getBinaryContext();
+  MCContext *Ctx = BC.Ctx.get();
+  const uint64_t InstOffset = InstAddress - Function.getAddress();
+
+  // Branches and calls are resolved by BinaryFunction's target-independent
+  // control-flow handling.
+  if (BC.MIB->isBranch(Inst) || BC.MIB->isCall(Inst))
+    return false;
+
+  // Linker processing of R_RISCV_ALIGN can leave emitted relocations at an
+  // offset inside the instruction they apply to. Match the whole instruction
+  // range, as BinaryFunction::disassemble() did before this target-specific
+  // handling moved into the symbolizer.
+  const Relocation *Rel =
+      Function.getRelocationInRange(InstOffset, InstOffset + InstSize);
+  if (!Rel)
+    return false;
+
+  MCSymbol *Symbol = Rel->Symbol;
+  uint64_t Addend = Rel->Addend;
+
+  if (Relocation::isInstructionReference(Rel->Type)) {
+    if (!CreateNewSymbols)
+      return false;
+    Symbol = getOrCreateInstructionLabel(Rel->Value - Function.getAddress());
+    // The input addend reflects the original AUIPC location. The label now
+    // follows the instruction, so the assembler must derive the low bits from
+    // its new location.
+    Addend = 0;
+  }
+
+  // GOT high relocations name the object stored in the GOT, not the GOT entry
+  // addressed by AUIPC. Preserve the actual entry address using a zero-based
+  // symbol, as the RISC-V emitter reuses the input GOT.
+  if (Relocation::isGOT(Rel->Type)) {
+    assert(Relocation::isPCRelative(Rel->Type) &&
+           "GOT relocation must be PC-relative on RISC-V");
+    Symbol = BC.registerNameAtAddress("__BOLT_got_zero", 0, 0, 0);
+    Addend = getGOTValue(*Rel) + InstAddress;
+  }
+
+  assert(Symbol && "RISC-V relocation without a symbol");
+  const MCExpr *Expr = MCSymbolRefExpr::create(Symbol, *Ctx);
+  if (Addend)
+    Expr = MCBinaryExpr::createAdd(Expr, MCConstantExpr::create(Addend, *Ctx),
+                                   *Ctx);
+  Inst.addOperand(MCOperand::createExpr(
+      BC.MIB->getTargetExprFor(Inst, Expr, *Ctx, Rel->Type)));
+
+  // MC annotations must follow every real operand. Attach the instruction
+  // label only after the symbolized immediate has been appended.
+  if (InstructionLabels.find(InstOffset) != InstructionLabels.end())
+    BC.MIB->setInstLabel(Inst, getOrCreateInstructionLabel(InstOffset));
+
+  return true;
+}
+
+void RISCVMCSymbolizer::tryAddingPcLoadReferenceComment(raw_ostream &CStream,
+                                                        int64_t Value,
+                                                        uint64_t Address) {}
+
+} // namespace bolt
+} // namespace llvm

bolt/lib/Target/RISCV/RISCVMCSymbolizer.h

@@ -0,0 +1,58 @@
+//===- bolt/Target/RISCV/RISCVMCSymbolizer.h --------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef BOLT_TARGET_RISCV_RISCVMCSYMBOLIZER_H
+#define BOLT_TARGET_RISCV_RISCVMCSYMBOLIZER_H
+
+#include "bolt/Core/BinaryFunction.h"
+#include "llvm/MC/MCDisassembler/MCSymbolizer.h"
+#include <map>
+
+namespace llvm {
+namespace bolt {
+
+class RISCVMCSymbolizer : public MCSymbolizer {
+protected:
+  BinaryFunction &Function;
+  bool CreateNewSymbols{true};
+
+  /// Map function offsets referenced by %pcrel_lo relocations to labels that
+  /// must remain attached to the corresponding %pcrel_hi instructions.
+  std::map<uint64_t, MCSymbol *> InstructionLabels;
+
+  /// Map referenced instruction offsets to their %pcrel_lo relocations.
+  std::map<uint64_t, const Relocation *> InstructionReferences;
+
+  MCSymbol *getOrCreateInstructionLabel(uint64_t Offset);
+
+  /// Return the complete PC-relative value for a GOT relocation. The value
+  /// recorded when relocations are read assumes that the low instruction
+  /// immediately follows AUIPC. Reconstruct it from the matching low
+  /// relocation so linker scheduling does not affect symbolization.
+  uint64_t getGOTValue(const Relocation &Rel) const;
+
+public:
+  RISCVMCSymbolizer(BinaryFunction &Function, bool CreateNewSymbols = true);
+
+  RISCVMCSymbolizer(const RISCVMCSymbolizer &) = delete;
+  RISCVMCSymbolizer &operator=(const RISCVMCSymbolizer &) = delete;
+  ~RISCVMCSymbolizer() override;
+
+  bool tryAddingSymbolicOperand(MCInst &Inst, raw_ostream &CStream,
+                                int64_t Value, uint64_t Address, bool IsBranch,
+                                uint64_t Offset, uint64_t OpSize,
+                                uint64_t InstSize) override;
+
+  void tryAddingPcLoadReferenceComment(raw_ostream &CStream, int64_t Value,
+                                       uint64_t Address) override;
+};
+
+} // namespace bolt
+} // namespace llvm
+
+#endif

bolt/test/RISCV/reloc-bb-split-rv32.s

@@ -20,10 +20,10 @@ _start:
 /// basic block should start there.
 // CHECK-LABEL: {{^}}.LBB00
 // CHECK: nop
-// CHECK-LABEL: {{^}}.Ltmp0
-// CHECK: auipc t0, %pcrel_hi(d)
-// CHECK-NEXT: lw t0, %pcrel_lo({{.*}})(t0)
-// CHECK-NEXT: j .Ltmp0
+// CHECK: {{^}}[[BRANCH_LABEL:.Ltmp[0-9]+]]
+// CHECK: auipc t0, %pcrel_hi(d) # Label: [[HI_LABEL:.Ltmp[0-9]+]]
+// CHECK-NEXT: lw t0, %pcrel_lo([[HI_LABEL]])(t0)
+// CHECK-NEXT: j [[BRANCH_LABEL]]
   nop
 1:
   auipc t0, %pcrel_hi(d)
@@ -34,8 +34,8 @@ _start:
 /// start there.
 // CHECK-LABEL: {{^}}.LFT0
 // CHECK: nop
-// CHECK: auipc t0, %pcrel_hi(d)
-// CHECK-NEXT: lw t0, %pcrel_lo({{.*}})(t0)
+// CHECK: auipc t0, %pcrel_hi(d) # Label: [[SECOND_HI:.Ltmp[0-9]+]]
+// CHECK-NEXT: lw t0, %pcrel_lo([[SECOND_HI]])(t0)
 // CHECK-NEXT: ret
   nop
 1:

bolt/test/RISCV/reloc-bb-split.s

@@ -17,10 +17,10 @@ _start:
 /// basic block should start there.
 // CHECK-LABEL: {{^}}.LBB00
 // CHECK: nop
-// CHECK-LABEL: {{^}}.Ltmp0
-// CHECK: auipc t0, %pcrel_hi(d) # Label: .Ltmp1
-// CHECK-NEXT: ld t0, %pcrel_lo(.Ltmp1)(t0)
-// CHECK-NEXT: j .Ltmp0
+// CHECK: {{^}}[[BRANCH_LABEL:.Ltmp[0-9]+]]
+// CHECK: auipc t0, %pcrel_hi(d) # Label: [[HI_LABEL:.Ltmp[0-9]+]]
+// CHECK-NEXT: ld t0, %pcrel_lo([[HI_LABEL]])(t0)
+// CHECK-NEXT: j [[BRANCH_LABEL]]
   nop
 1:
   auipc t0, %pcrel_hi(d)
@@ -31,8 +31,8 @@ _start:
 /// start there.
 // CHECK-LABEL: {{^}}.LFT0
 // CHECK: nop
-// CHECK-NEXT: auipc t0, %pcrel_hi(d) # Label: .Ltmp2
-// CHECK-NEXT: ld t0, %pcrel_lo(.Ltmp2)(t0)
+// CHECK-NEXT: auipc t0, %pcrel_hi(d) # Label: [[SECOND_HI:.Ltmp[0-9]+]]
+// CHECK-NEXT: ld t0, %pcrel_lo([[SECOND_HI]])(t0)
 // CHECK-NEXT: ret
   nop
 1: