[llvm-readobj] - Refactor the code that dumps relocations.

[grimar]

Summary

The current code has following issues:

1. It has a duplicated logic part.
2. This logic relies on unwrapOrError calls, but if we want to convert them to warnings, we will need to change all of them what is hard to do because of the duplication.

In this patch I've created a new method that returns Expected<> what allows
now to catch all errors in a single place and remove the code duplication.

Note: this change is itself a refactor NFC. It does not change the current logic
anyhow. It prepares the code for the follow-up(s).

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

5737

I've realized that Sym variable is not used for LLVMStyle.
(And a warning was not reported because of std::tie it seems).
Will update diff after finishing with the follow-up patch for this one.

[grimar]

• Addressed my previous comment.

[jhenderson]

LGTM.

[MaskRay]

llvm/tools/llvm-readobj/ELFDumper.cpp

1068

Add a comment about what this if block does.

The st_name field of a STT_SECTION is usually 0 (empty string). This code block returns the section name.

llvm/tools/llvm-readobj/ELFDumper.cpp

public:
 
   Expected<ArrayRef<Elf_Versym>> getVersionTable(const Elf_Shdr *Sec,
                                                  ArrayRef<Elf_Sym> *SymTab,
                                                  StringRef *StrTab) const;
   Expected<std::vector<VerDef>>
   getVersionDefinitions(const Elf_Shdr *Sec) const;
   Expected<std::vector<VerNeed>>
   getVersionDependencies(const Elf_Shdr *Sec) const;
+
+  Expected<std::pair<const Elf_Sym *, std::string>>
+  getRelocationTarget(const Elf_Shdr *SymTab, const Elf_Rela &R) const;
 };
 
 template <class ELFT>
 static Expected<StringRef> getLinkAsStrtab(const ELFFile<ELFT> *Obj,
                                            const typename ELFT::Shdr *Sec,
                                            unsigned SecNdx) {
   Expected<const typename ELFT::Shdr *> StrTabSecOrErr =
       Obj->getSection(Sec->sh_link);
Expected<StringRef> ELFDumper<ELFT>::getSymbolVersion(const Elf_Sym *Sym,
 
   // Get the corresponding version index entry.
   const Elf_Versym *Versym = unwrapOrError(
       ObjF->getFileName(), ObjF->getELFFile()->template getEntry<Elf_Versym>(
                                SymbolVersionSection, EntryIndex));
   return this->getSymbolVersionByIndex(Versym->vs_index, IsDefault);
 }
 
+template <typename ELFT>
+Expected<std::pair<const typename ELFT::Sym *, std::string>>
+ELFDumper<ELFT>::getRelocationTarget(const Elf_Shdr *SymTab,
+                                     const Elf_Rela &R) const {
+  const ELFFile<ELFT> *Obj = ObjF->getELFFile();
+  Expected<const Elf_Sym *> SymOrErr = Obj->getRelocationSymbol(&R, SymTab);
+  if (!SymOrErr)
+    return SymOrErr.takeError();
+  const Elf_Sym *Sym = *SymOrErr;
+  if (!Sym)
+    return std::make_pair(nullptr, "");
+
+  if (Sym->getType() == ELF::STT_SECTION) {
+    Expected<const Elf_Shdr *> SecOrErr =
+        Obj->getSection(Sym, SymTab, ShndxTable);
+    if (!SecOrErr)
+      return SecOrErr.takeError();
+
+    Expected<StringRef> NameOrErr = Obj->getSectionName(*SecOrErr);
+    if (!NameOrErr)
+      return NameOrErr.takeError();
+    return std::make_pair(Sym, NameOrErr->str());
+  }
+
+  Expected<StringRef> StrTableOrErr = Obj->getStringTableForSymtab(*SymTab);
+  if (!StrTableOrErr)
+    return StrTableOrErr.takeError();
+
+  std::string SymbolName =
+      getFullSymbolName(Sym, *StrTableOrErr, SymTab->sh_type == SHT_DYNSYM);
+  return std::make_pair(Sym, SymbolName);
+}
+
 static std::string maybeDemangle(StringRef Name) {
   return opts::Demangle ? demangle(std::string(Name)) : Name.str();
 }
 
 template <typename ELFT>
 Expected<std::string>
 ELFDumper<ELFT>::getStaticSymbolName(uint32_t Index) const {
   const ELFFile<ELFT> *Obj = ObjF->getELFFile();
template <class ELFT> void GNUStyle<ELFT>::printGroupSections(const ELFO *Obj) {
 
   if (V.empty())
     OS << "There are no section groups in this file.\n";
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printRelocation(const ELFO *Obj, const Elf_Shdr *SymTab,
                                      const Elf_Rela &R, bool IsRela) {
-  const Elf_Sym *Sym =
-      unwrapOrError(this->FileName, Obj->getRelocationSymbol(&R, SymTab));
-  std::string TargetName;
-  if (Sym && Sym->getType() == ELF::STT_SECTION) {
-    const Elf_Shdr *Sec = unwrapOrError(
-        this->FileName,
-        Obj->getSection(Sym, SymTab, this->dumper()->getShndxTable()));
-    TargetName =
-        std::string(unwrapOrError(this->FileName, Obj->getSectionName(Sec)));
-  } else if (Sym) {
-    StringRef StrTable =
-        unwrapOrError(this->FileName, Obj->getStringTableForSymtab(*SymTab));
-    TargetName = this->dumper()->getFullSymbolName(
-        Sym, StrTable, SymTab->sh_type == SHT_DYNSYM /* IsDynamic */);
-  }
-  printRelocation(Obj, Sym, TargetName, R, IsRela);
+  const typename ELFT::Sym *Sym;
+  std::string Name;
+  std::tie(Sym, Name) = unwrapOrError(
+      this->FileName, this->dumper()->getRelocationTarget(SymTab, R));
+  printRelocation(Obj, Sym, Name, R, IsRela);
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printRelocation(const ELFO *Obj, const Elf_Sym *Sym,
                                      StringRef SymbolName, const Elf_Rela &R,
                                      bool IsRela) {
   // First two fields are bit width dependent. The rest of them are fixed width.
   unsigned Bias = ELFT::Is64Bits ? 8 : 0;
for (const Elf_Rela &R :
       printRelocation(Obj, R, SymTab);
     break;
   }
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printRelocation(const ELFO *Obj, Elf_Rela Rel,
                                       const Elf_Shdr *SymTab) {
+  std::string TargetName =
+      unwrapOrError(this->FileName,
+                    this->dumper()->getRelocationTarget(SymTab, Rel))
+          .second;
+
   SmallString<32> RelocName;
   Obj->getRelocationTypeName(Rel.getType(Obj->isMips64EL()), RelocName);
-  std::string TargetName;
-  const Elf_Sym *Sym =
-      unwrapOrError(this->FileName, Obj->getRelocationSymbol(&Rel, SymTab));
-  if (Sym && Sym->getType() == ELF::STT_SECTION) {
-    const Elf_Shdr *Sec = unwrapOrError(
-        this->FileName,
-        Obj->getSection(Sym, SymTab, this->dumper()->getShndxTable()));
-    TargetName =
-        std::string(unwrapOrError(this->FileName, Obj->getSectionName(Sec)));
-  } else if (Sym) {
-    StringRef StrTable =
-        unwrapOrError(this->FileName, Obj->getStringTableForSymtab(*SymTab));
-    TargetName = this->dumper()->getFullSymbolName(
-        Sym, StrTable, SymTab->sh_type == SHT_DYNSYM /* IsDynamic */);
-  }
 
   if (opts::ExpandRelocs) {
     DictScope Group(W, "Relocation");
     W.printHex("Offset", Rel.r_offset);
     W.printNumber("Type", RelocName, (int)Rel.getType(Obj->isMips64EL()));
     W.printNumber("Symbol", !TargetName.empty() ? TargetName : "-",
                   Rel.getSymbol(Obj->isMips64EL()));
     W.printHex("Addend", Rel.r_addend);