[JITLink] Adopt forEachRelocation() helper in ELF RISCV backend (NFC)

[sgraenitz]

Summary

Following D109516, this patch re-uses the new helper function for ELF relocation traversal in the RISCV backend.

[sgraenitz]

The patch aims to be NFC (except debug output format). Please ping me if I introduced a functional change by accident.

[jrtc27]

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

303

std::move of a lambda is a strange thing to do

[StephenFan]

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

297

x64 -> riscv

[sgraenitz]

Account for modifications in the underlying patch

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

297

Good catch! Actually, I made a copy/paste mistake here (from x86-64). This check didn't exist for RISCV. It'd be a functional change and thus I am going to drop it (even in case it's accurate for RISCV).

303

Agree. It's been a leftover from my previous std::function sketches. Latest version passes the member function pointer directly.

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

case ELF::R_RISCV_CALL_PLT:
       return EdgeKind_riscv::R_RISCV_CALL_PLT;
     }
 
     return make_error<JITLinkError>("Unsupported riscv relocation:" +
                                     formatv("{0:d}", Type));
   }
 
   Error addRelocations() override {
+    LLVM_DEBUG(dbgs() << "Processing relocations:\n");
     using Base = ELFLinkGraphBuilder<ELFT>;
-    LLVM_DEBUG(dbgs() << "Adding relocations\n");
 
-    // TODO a partern is forming of iterate some sections but only give me
-    // ones I am interested, I should abstract that concept some where
-    for (auto &SecRef : Base::Sections) {
-      if (SecRef.sh_type != ELF::SHT_RELA && SecRef.sh_type != ELF::SHT_REL)
-        continue;
-      auto RelSectName = Base::Obj.getSectionName(SecRef);
-      if (!RelSectName)
-        return RelSectName.takeError();
+    for (const auto &RelSect : Base::Sections) {
+      auto Process = [&](const auto &Rel, const auto &Target, Section &GS) {
+        // Sanity check the section we read relocation entries from.
+        if (RelSect.sh_type == ELF::SHT_REL)
+          return make_error<StringError>(
+              "No SHT_REL in valid x64 ELF object files",
+              inconvertibleErrorCode());
 
-      LLVM_DEBUG({
-        dbgs() << "Adding relocations from section " << *RelSectName << "\n";
-      });
+        return addSingleRelocation(Rel, Target, GS);
+      };
 
-      auto UpdateSection = Base::Obj.getSection(SecRef.sh_info);
-      if (!UpdateSection)
-        return UpdateSection.takeError();
-
-      auto UpdateSectionName = Base::Obj.getSectionName(**UpdateSection);
-      if (!UpdateSectionName)
-        return UpdateSectionName.takeError();
-      // Don't process relocations for debug sections.
-      if (Base::isDwarfSection(*UpdateSectionName)) {
-        LLVM_DEBUG({
-          dbgs() << "  Target is dwarf section " << *UpdateSectionName
-                 << ". Skipping.\n";
-        });
-        continue;
-      } else
-        LLVM_DEBUG({
-          dbgs() << "  For target section " << *UpdateSectionName << "\n";
-        });
+      if (Error Err = Base::forEachRelocation(RelSect, std::move(Process)))
+        return Err;
+    }
 
-      auto *JITSection = Base::G->findSectionByName(*UpdateSectionName);
-      if (!JITSection)
-        return make_error<llvm::StringError>(
-            "Refencing a section that wasn't added to graph" +
-                *UpdateSectionName,
-            llvm::inconvertibleErrorCode());
-
-      auto Relocations = Base::Obj.relas(SecRef);
-      if (!Relocations)
-        return Relocations.takeError();
+    return Error::success();
+  }
 
-      for (const auto &Rela : *Relocations) {
-        auto Type = Rela.getType(false);
+  Error addSingleRelocation(const typename ELFT::Rela &Rel,
+                            const typename ELFT::Shdr &FixupSect,
+                            Section &GraphSection) {
+    using Base = ELFLinkGraphBuilder<ELFT>;
 
-        LLVM_DEBUG({
-          dbgs() << "Relocation Type: " << Type << "\n"
-                 << "Name: " << Base::Obj.getRelocationTypeName(Type) << "\n";
-        });
+    uint32_t SymbolIndex = Rel.getSymbol(false);
+    auto ObjSymbol = Base::Obj.getRelocationSymbol(Rel, Base::SymTabSec);
+    if (!ObjSymbol)
+      return ObjSymbol.takeError();
+
+    Symbol *GraphSymbol = Base::getGraphSymbol(SymbolIndex);
+    if (!GraphSymbol)
+      return make_error<StringError>(
+          formatv("Could not find symbol at given index, did you add it to "
+                  "JITSymbolTable? index: {0}, shndx: {1} Size of table: {2}",
+                  SymbolIndex, (*ObjSymbol)->st_shndx,
+                  Base::GraphSymbols.size()),
+          inconvertibleErrorCode());
 
-        auto SymbolIndex = Rela.getSymbol(false);
-        auto Symbol = Base::Obj.getRelocationSymbol(Rela, Base::SymTabSec);
-        if (!Symbol)
-          return Symbol.takeError();
-
-        auto BlockToFix = *(JITSection->blocks().begin());
-        auto *TargetSymbol = Base::getGraphSymbol(SymbolIndex);
-
-        if (!TargetSymbol) {
-          return make_error<llvm::StringError>(
-              "Could not find symbol at given index, did you add it to "
-              "JITSymbolTable? index: " +
-                  std::to_string(SymbolIndex) + ", shndx: " +
-                  std::to_string((*Symbol)->st_shndx) + " Size of table: " +
-                  std::to_string(Base::GraphSymbols.size()),
-              llvm::inconvertibleErrorCode());
-        }
-        int64_t Addend = Rela.r_addend;
-        JITTargetAddress FixupAddress =
-            (*UpdateSection)->sh_addr + Rela.r_offset;
+    uint32_t Type = Rel.getType(false);
+    Expected<riscv::EdgeKind_riscv> Kind = getRelocationKind(Type);
+    if (!Kind)
+      return Kind.takeError();
 
-        LLVM_DEBUG({
-          dbgs() << "Processing relocation at "
-                 << format("0x%016" PRIx64, FixupAddress) << "\n";
-        });
-        auto Kind = getRelocationKind(Type);
-        if (!Kind)
-          return Kind.takeError();
+    int64_t Addend = Rel.r_addend;
+    Block *BlockToFix = *(GraphSection.blocks().begin());
+    JITTargetAddress FixupAddress = FixupSect.sh_addr + Rel.r_offset;
+    Edge::OffsetT Offset = FixupAddress - BlockToFix->getAddress();
+    Edge GE(*Kind, Offset, *GraphSymbol, Addend);
+      dbgs() << "    ";
+      printEdge(dbgs(), *BlockToFix, GE, riscv::getEdgeKindName(*Kind));
+      dbgs() << "\n";
 
-        BlockToFix->addEdge(*Kind, FixupAddress - BlockToFix->getAddress(),
-                            *TargetSymbol, Addend);
-      }
-    }
+    BlockToFix->addEdge(std::move(GE));
     return Error::success();
   }
 
 public:
   ELFLinkGraphBuilder_riscv(StringRef FileName,
                             const object::ELFFile<ELFT> &Obj, const Triple T)
       : ELFLinkGraphBuilder<ELFT>(Obj, std::move(T), FileName,
                                   riscv::getEdgeKindName) {}