[llvm-readobj/elf] - Generalize the code for printing dynamic relocations. NFCI.

[grimar]

Summary

Currently we have 2 large printDynamicRelocations methods that
have a very similar code for GNU/LLVM styles.

This patch removes the duplication and renames them to printDynamicReloc
for consistency.

Depends on D87141

[grimar]

• Fix minor nit.

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

5495

I think if you move this function to immediately after the GNUStyle<ELFT>::printDynamicRelocations function, it will make the diff much easier to follow. As it is, there doesn't seem to be a consistent ordering of DumpStyle and GNUStyle methods, so keeping the diff/blame history simpler seems beneficial.

[grimar]

• Addressed review comment.

llvm/tools/llvm-readobj/ELFDumper.cpp

5495

Done.

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

4454–4455

I'm not a fan of having to go through the printRelaReloc, just to get to the printDynamicRelocation method which was being called before. I take it this is because printDynamicRelocation is templated and can't be virtual? I'm beginning to wonder if using the template solution to the Elf_Rel/Elf_Rela problem was a mistake - it's making the code harder to follow, due to there being unnecessary layers of functions (and in this case, parameters). Maybe we should use a class that can represent either an Elf_Rel or Elf_Rela, ideally with an implicit conversion constructor that is used instead of the two of them. That way, the template can be avoided, and the printDynamicRelocation could become a part of the interface. To avoid confusion, the class would not be an Elf_Rela (so that we aren't misrepresenting Elf_Rel instances internally). What do you think?

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

4454–4455

Yes, I think it makes sense. I'll am going to try to introduce such class and experiment with the code a bit.

[grimar]

• Updated in according to discussion. Now depends on D87141.

[jhenderson]

LGTM. I think this is a much cleaner version now.

llvm/tools/llvm-readobj/ELFDumper.cpp

void printDependentLibsHelper(
       function_ref<void(StringRef, uint64_t)> OnSectionEntry);
 
   virtual void printRelReloc(const Elf_Rel &R, unsigned RelIndex,
                              const Elf_Shdr *Sec, const Elf_Shdr *SymTab) = 0;
   virtual void printRelaReloc(const Elf_Rela &R, unsigned RelIndex,
                               const Elf_Shdr *Sec, const Elf_Shdr *SymTab) = 0;
   virtual void printRelrReloc(const Elf_Relr &R) = 0;
   void printRelocationsHelper(const Elf_Shdr &Sec);
+  void printDynamicRelocationsHelper();
+  virtual void printDynamicRelocHeader(unsigned Type, StringRef Name,
+                                       const DynRegionInfo &Reg){};
 
   StringRef getPrintableSectionName(const Elf_Shdr &Sec) const;
 
   void reportUniqueWarning(Error Err) const;
 
   StringRef FileName;
   const ELFFile<ELFT> &Obj;
 
private:
   void printRelRelaReloc(const RelTy &R, unsigned RelIndex, const Elf_Shdr &Sec,
                          const Elf_Shdr *SymTab);
   template <class RelTy>
   void printRelRelaReloc(const Elf_Sym *Sym, StringRef SymbolName,
                          const RelTy &R);
   void printSymbol(const Elf_Sym *Symbol, const Elf_Sym *First,
                    Optional<StringRef> StrTable, bool IsDynamic,
                    bool NonVisibilityBitsUsed) override;
+  void printDynamicRelocHeader(unsigned Type, StringRef Name,
+                               const DynRegionInfo &Reg) override;
   std::string getSymbolSectionNdx(const Elf_Sym *Symbol,
                                   const Elf_Sym *FirstSym);
   template <class RelTy> void printDynamicRelocation(const RelTy &R);
   void printProgramHeaders();
   void printSectionMapping();
   void printGNUVersionSectionProlog(const typename ELFT::Shdr *Sec,
                                     const Twine &Label, unsigned EntriesNum);
 };
template <class ELFT> void GNUStyle<ELFT>::printGroupSections() {
   if (V.empty())
     OS << "There are no section groups in this file.\n";
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printRelReloc(const Elf_Rel &R, unsigned RelIndex,
                                    const Elf_Shdr *Sec,
                                    const Elf_Shdr *SymTab) {
+  if (Sec)
     printRelRelaReloc(R, RelIndex, *Sec, SymTab);
+  else
+    printDynamicRelocation(R);
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printRelaReloc(const Elf_Rela &R, unsigned RelIndex,
                                     const Elf_Shdr *Sec,
                                     const Elf_Shdr *SymTab) {
+  if (Sec)
     printRelRelaReloc(R, RelIndex, *Sec, SymTab);
+  else
+    printDynamicRelocation(R);
 }
 
 template <class ELFT> void GNUStyle<ELFT>::printRelrReloc(const Elf_Relr &R) {
   OS << to_string(format_hex_no_prefix(R, ELFT::Is64Bits ? 16 : 8)) << "\n";
 }
 
 template <class ELFT>
 template <class RelTy>
static void printRelocHeaderFields(formatted_raw_ostream &OS, unsigned SType) {
   else
     OS << "     Info    Type                Sym. Value  Symbol's Name";
   if (IsRela)
     OS << " + Addend";
   OS << "\n";
 }
 
 template <class ELFT>
-static void printDynamicRelocHeader(const ELFFile<ELFT> &Obj,
-                                    formatted_raw_ostream &OS, unsigned Type,
-                                    StringRef Name, const DynRegionInfo &Reg) {
-  uint64_t Offset = Reg.Addr - Obj.base();
+void GNUStyle<ELFT>::printDynamicRelocHeader(unsigned Type, StringRef Name,
+                                             const DynRegionInfo &Reg) {
+  uint64_t Offset = Reg.Addr - this->Obj.base();
   OS << "\n'" << Name.str().c_str() << "' relocation section at offset 0x"
      << to_hexString(Offset, false) << " contains " << Reg.Size << " bytes:\n";
   printRelocHeaderFields<ELFT>(OS, Type);
 }
 
 template <class ELFT>
 static bool isRelocationSec(const typename ELFT::Shdr &Sec) {
   return Sec.sh_type == ELF::SHT_REL || Sec.sh_type == ELF::SHT_RELA ||
for (auto Entry : Table) {
     std::string Type =
         std::string("(") + this->Obj.getDynamicTagAsString(Tag).c_str() + ")";
     std::string Value = this->dumper()->getDynamicEntry(Tag, Entry.getVal());
     OS << "  " << format_hex(Tag, ELFT::Is64Bits ? 18 : 10)
        << format(ValueFmt.c_str(), Type.c_str()) << Value << "\n";
   }
 }
 
 template <class ELFT> void GNUStyle<ELFT>::printDynamicRelocations() {
-  const DynRegionInfo &DynRelRegion = this->dumper()->getDynRelRegion();
-  const DynRegionInfo &DynRelaRegion = this->dumper()->getDynRelaRegion();
-  const DynRegionInfo &DynRelrRegion = this->dumper()->getDynRelrRegion();
-  const DynRegionInfo &DynPLTRelRegion = this->dumper()->getDynPLTRelRegion();
-  if (DynRelaRegion.Size > 0) {
-    printDynamicRelocHeader(this->Obj, OS, ELF::SHT_RELA, "RELA",
-                            DynRelaRegion);
-    for (const Elf_Rela &Rela : this->dumper()->dyn_relas())
-      printDynamicRelocation(Rela);
-  }
-  if (DynRelRegion.Size > 0) {
-    printDynamicRelocHeader(this->Obj, OS, ELF::SHT_REL, "REL", DynRelRegion);
-    for (const Elf_Rel &Rel : this->dumper()->dyn_rels())
-      printDynamicRelocation(Rel);
-  }
-  if (DynRelrRegion.Size > 0) {
-    printDynamicRelocHeader(this->Obj, OS, ELF::SHT_REL, "RELR", DynRelrRegion);
-    Elf_Relr_Range Relrs = this->dumper()->dyn_relrs();
-    for (const Elf_Rel &R : this->Obj.decode_relrs(Relrs))
-      printDynamicRelocation(R);
-  }
-  if (DynPLTRelRegion.Size) {
-    if (DynPLTRelRegion.EntSize == sizeof(Elf_Rela)) {
-      printDynamicRelocHeader(this->Obj, OS, ELF::SHT_RELA, "PLT",
-                              DynPLTRelRegion);
-      for (const Elf_Rela &Rela : DynPLTRelRegion.getAsArrayRef<Elf_Rela>())
-        printDynamicRelocation(Rela);
-    } else {
-      printDynamicRelocHeader(this->Obj, OS, ELF::SHT_REL, "PLT",
-                              DynPLTRelRegion);
-      for (const Elf_Rel &Rel : DynPLTRelRegion.getAsArrayRef<Elf_Rel>())
-        printDynamicRelocation(Rel);
-    }
-  }
+  this->printDynamicRelocationsHelper();
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printGNUVersionSectionProlog(
     const typename ELFT::Shdr *Sec, const Twine &Label, unsigned EntriesNum) {
   StringRef SecName =
       unwrapOrError(this->FileName, this->Obj.getSectionName(Sec));
   OS << Label << " section '" << SecName << "' "
if (Expected<std::vector<Elf_Rela>> RelasOrErr = Obj.android_relas(&Sec)) {
         printRelaReloc(R, ++RelNdx, &Sec, SymTab);
     } else {
       Warn(RelasOrErr.takeError());
     }
     break;
   }
 }
 
+template <class ELFT> void DumpStyle<ELFT>::printDynamicRelocationsHelper() {
+  const DynRegionInfo &DynRelaRegion = this->dumper()->getDynRelaRegion();
+  if (DynRelaRegion.Size > 0) {
+    printDynamicRelocHeader(ELF::SHT_RELA, "RELA", DynRelaRegion);
+    for (const Elf_Rela &Rela : this->dumper()->dyn_relas())
+      printRelaReloc(Rela, /*RelNdx=*/0, /*Sec=*/nullptr, /*Symtab=*/nullptr);
+  }
+
+  const DynRegionInfo &DynRelRegion = this->dumper()->getDynRelRegion();
+  if (DynRelRegion.Size > 0) {
+    printDynamicRelocHeader(ELF::SHT_REL, "REL", DynRelRegion);
+    for (const Elf_Rel &Rel : this->dumper()->dyn_rels())
+      printRelReloc(Rel, /*RelNdx=*/0, /*Sec=*/nullptr, /*Symtab=*/nullptr);
+  }
+
+  const DynRegionInfo &DynRelrRegion = this->dumper()->getDynRelrRegion();
+  if (DynRelrRegion.Size > 0) {
+    printDynamicRelocHeader(ELF::SHT_REL, "RELR", DynRelrRegion);
+    Elf_Relr_Range Relrs = this->dumper()->dyn_relrs();
+    for (const Elf_Rel &R : Obj.decode_relrs(Relrs))
+      printRelReloc(R, /*RelNdx=*/0, /*Sec=*/nullptr, /*Symtab=*/nullptr);
+  }
+
+  const DynRegionInfo &DynPLTRelRegion = this->dumper()->getDynPLTRelRegion();
+  if (DynPLTRelRegion.Size) {
+    if (DynPLTRelRegion.EntSize == sizeof(Elf_Rela)) {
+      printDynamicRelocHeader(ELF::SHT_RELA, "PLT", DynPLTRelRegion);
+      for (const Elf_Rela &Rela : DynPLTRelRegion.getAsArrayRef<Elf_Rela>())
+        printRelaReloc(Rela, /*RelNdx=*/0, /*Sec=*/nullptr, /*Symtab=*/nullptr);
+    } else {
+      printDynamicRelocHeader(ELF::SHT_REL, "PLT", DynPLTRelRegion);
+      for (const Elf_Rel &Rel : DynPLTRelRegion.getAsArrayRef<Elf_Rel>())
+        printRelReloc(Rel, /*RelNdx=*/0, /*Sec=*/nullptr, /*Symtab=*/nullptr);
+    }
+  }
+}
+
 template <class ELFT>
 StringRef DumpStyle<ELFT>::getPrintableSectionName(const Elf_Shdr &Sec) const {
   StringRef Name = "<?>";
   if (Expected<StringRef> SecNameOrErr =
           Obj.getSectionName(&Sec, this->dumper()->WarningHandler))
     Name = *SecNameOrErr;
   else
     this->reportUniqueWarning(createError("unable to get the name of " +
for (const Elf_Shdr &Sec : cantFail(this->Obj.sections())) {
     W.startLine() << "}\n";
   }
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printRelReloc(const Elf_Rel &R, unsigned RelIndex,
                                     const Elf_Shdr *Sec,
                                     const Elf_Shdr *SymTab) {
+  if (Sec)
     printRelRelaReloc(R, RelIndex, *Sec, SymTab);
+  else
+    printDynamicRelocation(R);
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printRelaReloc(const Elf_Rela &R, unsigned RelIndex,
                                      const Elf_Shdr *Sec,
                                      const Elf_Shdr *SymTab) {
+  if (Sec)
     printRelRelaReloc(R, RelIndex, *Sec, SymTab);
+  else
+    printDynamicRelocation(R);
 }
 
 template <class ELFT> void LLVMStyle<ELFT>::printRelrReloc(const Elf_Relr &R) {
   W.startLine() << W.hex(R) << "\n";
 }
 
 template <class ELFT>
 template <class RelTy>
 void LLVMStyle<ELFT>::printRelRelaReloc(const RelTy &Rel, unsigned RelIndex,
                                         const Elf_Shdr &Sec,
                                         const Elf_Shdr *SymTab) {
   Expected<std::pair<const typename ELFT::Sym *, std::string>> Target =
       this->dumper()->getRelocationTarget(SymTab, Rel);
   if (!Target) {
-    this->reportUniqueWarning(
-        createError("unable to print relocation " + Twine(RelIndex) + " in " +
-                    describe(Obj, Sec) + ": " + toString(Target.takeError())));
+    this->reportUniqueWarning(createError(
+        "unable to print relocation " + Twine(RelIndex) + " in " +
+        describe(this->Obj, Sec) + ": " + toString(Target.takeError())));
     return;
   }
 
   std::string TargetName = Target->second;
   SmallString<32> RelocName;
   this->Obj.getRelocationTypeName(Rel.getType(this->Obj.isMips64EL()),
                                   RelocName);
 
W.startLine() << "  " << format_hex(Tag, ELFT::Is64Bits ? 18 : 10, true)
                   << format(ValueFmt.c_str(),
                             this->Obj.getDynamicTagAsString(Tag).c_str())
                   << Value << "\n";
   }
   W.startLine() << "]\n";
 }
 
 template <class ELFT> void LLVMStyle<ELFT>::printDynamicRelocations() {
-  const DynRegionInfo &DynRelRegion = this->dumper()->getDynRelRegion();
-  const DynRegionInfo &DynRelaRegion = this->dumper()->getDynRelaRegion();
-  const DynRegionInfo &DynRelrRegion = this->dumper()->getDynRelrRegion();
-  const DynRegionInfo &DynPLTRelRegion = this->dumper()->getDynPLTRelRegion();
-
   W.startLine() << "Dynamic Relocations {\n";
   W.indent();
-  if (DynRelaRegion.Size > 0) {
-    for (const Elf_Rela &Rela : this->dumper()->dyn_relas())
-      printDynamicRelocation(Rela);
-  }
-  if (DynRelRegion.Size > 0) {
-    for (const Elf_Rel &Rel : this->dumper()->dyn_rels())
-      printDynamicRelocation(Rel);
-  }
-
-  if (DynRelrRegion.Size > 0) {
-    Elf_Relr_Range Relrs = this->dumper()->dyn_relrs();
-    for (const Elf_Rel &R : this->Obj.decode_relrs(Relrs))
-      printDynamicRelocation(R);
-  }
-  if (DynPLTRelRegion.EntSize == sizeof(Elf_Rela))
-    for (const Elf_Rela &Rela : DynPLTRelRegion.getAsArrayRef<Elf_Rela>())
-      printDynamicRelocation(Rela);
-  else
-    for (const Elf_Rel &Rel : DynPLTRelRegion.getAsArrayRef<Elf_Rel>())
-      printDynamicRelocation(Rel);
-
+  this->printDynamicRelocationsHelper();
   W.unindent();
   W.startLine() << "}\n";
 }
 
 template <class ELFT>
 template <class RelTy>
 void LLVMStyle<ELFT>::printDynamicRelocation(const RelTy &Rel) {
   SmallString<32> RelocName;