[JITLink] Adopt forEachRelocation() helper in ELF x86-64 backend (NFC)

[sgraenitz]

Summary

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

[sgraenitz]

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

[sgraenitz]

Account for modifications in the underlying patch && address pre-merge checks

[sgraenitz]

Ping! I was hoping to land this together with D109522.

[StephenFan]

LGTM. Thanks @sgraenitz!

[foad]

llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp

317–318

This causes a warning when I do a release build with clang 10.0.0:

ELF_x86_64.cpp:308:13: warning: enumeration value 'PCRel32TLV' not handled in switch [-Wswitch]

[GMNGeoffrey]

llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp

317–318

I'm also seeing this and build with -Werror

[GMNGeoffrey]

llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp

317–318

Looks like a bad merge with https://reviews.llvm.org/D109293 (thanks @bkramer for pointing this out). I'll send a fix

llvm/lib/ExecutionEngine/JITLink/ELF_x86_64.cpp

 //===---- ELF_x86_64.cpp -JIT linker implementation for ELF/x86-64 ----===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 // ELF/x86-64 jit-link implementation.

 
 namespace llvm {
 namespace jitlink {
 
 // This should become a template as the ELFFile is so a lot of this could become
 // generic
 class ELFLinkGraphBuilder_x86_64 : public ELFLinkGraphBuilder<object::ELF64LE> {
 private:
+  using ELFT = object::ELF64LE;
 
   static Expected<ELF_x86_64_Edges::ELFX86RelocationKind>
   getRelocationKind(const uint32_t Type) {
     switch (Type) {
     case ELF::R_X86_64_32S:
       return ELF_x86_64_Edges::ELFX86RelocationKind::Pointer32Signed;
     case ELF::R_X86_64_PC32:
       return ELF_x86_64_Edges::ELFX86RelocationKind::PCRel32;
case ELF::R_X86_64_PLT32:
       return ELF_x86_64_Edges::ELFX86RelocationKind::Branch32;
     }
     return make_error<JITLinkError>("Unsupported x86-64 relocation type " +
                                     formatv("{0:d}: ", Type) +
                                     getELFX86_64RelocName(Type));
   }
 
   Error addRelocations() override {
-    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 : Sections) {
-      if (SecRef.sh_type != ELF::SHT_RELA && SecRef.sh_type != ELF::SHT_REL)
-        continue;
-      // TODO can the elf obj file do this for me?
-      if (SecRef.sh_type == ELF::SHT_REL)
-        return make_error<llvm::StringError>("Shouldn't have REL in x64",
-                                             llvm::inconvertibleErrorCode());
-
-      auto RelSectName = Obj.getSectionName(SecRef);
-      if (!RelSectName)
-        return RelSectName.takeError();
+    LLVM_DEBUG(dbgs() << "Processing relocations:\n");
+    using Base = ELFLinkGraphBuilder<ELFT>;
 
-      LLVM_DEBUG({
-        dbgs() << "Adding relocations from section " << *RelSectName << "\n";
-      });
-
-      auto UpdateSection = Obj.getSection(SecRef.sh_info);
-      if (!UpdateSection)
-        return UpdateSection.takeError();
-
-      auto UpdateSectionName = Obj.getSectionName(**UpdateSection);
-      if (!UpdateSectionName)
-        return UpdateSectionName.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());
 
-      // Don't process relocations for debug sections.
-      if (isDwarfSection(*UpdateSectionName)) {
-        LLVM_DEBUG({
-          dbgs() << "  Target is dwarf section " << *UpdateSectionName
-                 << ". Skipping.\n";
-        });
-        continue;
-      } else
-        LLVM_DEBUG({
-          dbgs() << "  For target section " << *UpdateSectionName << "\n";
-        });
+        return addSingleRelocation(Rel, Target, GS);
+      };
 
-      auto JITSection = G->findSectionByName(*UpdateSectionName);
-      if (!JITSection)
-        return make_error<llvm::StringError>(
-            "Refencing a a section that wasn't added to graph" +
-                *UpdateSectionName,
-            llvm::inconvertibleErrorCode());
-
-      auto Relocations = Obj.relas(SecRef);
-      if (!Relocations)
-        return Relocations.takeError();
+      if (Error Err = Base::forEachRelocation(RelSect, std::move(Process)))
+        return Err;
+    }
 
-      for (const auto &Rela : *Relocations) {
-        auto Type = Rela.getType(false);
+    return Error::success();
+  }
 
-        LLVM_DEBUG({
-          dbgs() << "Relocation Type: " << Type << "\n"
-                 << "Name: " << Obj.getRelocationTypeName(Type) << "\n";
-        });
-        auto SymbolIndex = Rela.getSymbol(false);
-        auto Symbol = Obj.getRelocationSymbol(Rela, SymTabSec);
-        if (!Symbol)
-          return Symbol.takeError();
-
-        auto BlockToFix = *(JITSection->blocks().begin());
-        auto *TargetSymbol = 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(GraphSymbols.size()),
-              llvm::inconvertibleErrorCode());
-        }
-        int64_t Addend = Rela.r_addend;
-        JITTargetAddress FixupAddress =
-            (*UpdateSection)->sh_addr + Rela.r_offset;
+  Error addSingleRelocation(const typename ELFT::Rela &Rel,
+                            const typename ELFT::Shdr &FixupSection,
+                            Section &GraphSection) {
+    using Base = ELFLinkGraphBuilder<ELFT>;
+
+    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());
 
-        LLVM_DEBUG({
-          dbgs() << "Processing relocation at "
-                 << format("0x%016" PRIx64, FixupAddress) << "\n";
-        });
-        auto ELFRelocKind = getRelocationKind(Type);
-        if (!ELFRelocKind)
-          return ELFRelocKind.takeError();
+    // Sanity check the relocation kind.
+    auto ELFRelocKind = getRelocationKind(Rel.getType(false));
 
-        Edge::Kind Kind = Edge::Invalid;
-        switch (*ELFRelocKind) {
-        case PCRel32:
-          Kind = x86_64::Delta32;
-          break;
-        case Delta64:
-          Kind = x86_64::Delta64;
-          break;
-        case Pointer32Signed:
-          Kind = x86_64::Pointer32Signed;
-          break;
-        case Pointer64:
-          Kind = x86_64::Pointer64;
-          break;
-        case PCRel32GOTLoad: {
-          Kind = x86_64::RequestGOTAndTransformToDelta32;
-          break;
-        }
-        case PCRel32REXGOTLoadRelaxable: {
-          Kind = x86_64::RequestGOTAndTransformToPCRel32GOTLoadREXRelaxable;
-          Addend = 0;
-          break;
-        }
-        case PCRel32GOTLoadRelaxable: {
-          Kind = x86_64::RequestGOTAndTransformToPCRel32GOTLoadRelaxable;
-          Addend = 0;
-          break;
-        }
-        case PCRel64GOT: {
-          Kind = x86_64::RequestGOTAndTransformToDelta64;
-          break;
-        }
-        case GOT64: {
-          Kind = x86_64::RequestGOTAndTransformToDelta64FromGOT;
-          break;
-        }
-        case GOTOFF64: {
-          Kind = x86_64::Delta64FromGOT;
-          break;
-        }
-        case Branch32: {
-          Kind = x86_64::BranchPCRel32;
-          Addend = 0;
-          break;
-        }
-        }
+    int64_t Addend = Rel.r_addend;
 
-        LLVM_DEBUG({
-          Edge GE(Kind, FixupAddress - BlockToFix->getAddress(), *TargetSymbol,
-                  Addend);
-          printEdge(dbgs(), *BlockToFix, GE, getELFX86RelocationKindName(Kind));
-          dbgs() << "\n";
-        });
-        BlockToFix->addEdge(Kind, FixupAddress - BlockToFix->getAddress(),
-                            *TargetSymbol, Addend);
-      }
-    }
+    Block *BlockToFix = *(GraphSection.blocks().begin());
+    JITTargetAddress FixupAddress = FixupSection.sh_addr + Rel.r_offset;
+    Edge::OffsetT Offset = FixupAddress - BlockToFix->getAddress();
+    Edge GE(Kind, Offset, *GraphSymbol, Addend);
+      dbgs() << "    ";
+
+    BlockToFix->addEdge(std::move(GE));
     return Error::success();
   }
 
 public:
   ELFLinkGraphBuilder_x86_64(StringRef FileName,
                              const object::ELFFile<object::ELF64LE> &Obj)
       : ELFLinkGraphBuilder(Obj, Triple("x86_64-unknown-linux"), FileName,
                             x86_64::getEdgeKindName) {}