[llvm-readobj] Add helper functions `printVersionSymbol()`, `printVersionDefinition()` and `printVersionDependency()`

[Higuoxing]

Summary

Since LLVM Style dumpers and GNU style dumpers share some same logic, we could simplify some logic inside printing functions.

[Higuoxing]

Fix one small nit.

[Higuoxing]

Move VersymBuf out of printVersionSymbol().

[Higuoxing]

llvm/tools/llvm-readobj/ELFDumper.cpp

4578–4579

Here, I changed a behavior of this tool. In this patch, if no versioning section appeared, we won't print anything. Before this patch, we will print:

Version symbols {
}
SHT_GNU_verdef {
}
SHT_GNU_verneed {
}

I don't know if this could be accepted

[Higuoxing]

Rebase.

[Higuoxing]

This patch breaks some tests, I will modify this later

[Higuoxing]

Sorry for not responding on this patch for a long time.

I have tried several methods to simplify some shared logic of LLVMStyle and GNUStyle.

In this patch, I add another 3 functions to place some shared logic or variables. I met a problem which is, in LLVMStyle, we should print empty closure for empty versym, verneed, verdef sections. But DictScope cannot be used to forward-declare. For example:

DictScope D; // to extend D's lifetime.
if (opts::Output == opts::LLVM)
  D = DictScope(W, "XXX");
if (!Sec)
  return;

So, I have to write empty closure by hand. Like:

if (!Sec) {
  if (opts::Output == opts::LLVM)
    W.startLine() << "ScopeName {\n}\n";
  return;
}

In lld, some tests detect the empty closures, so we have to keep this feature.

[jhenderson]

As I'll be away next week, I've added a couple of others to help with reviewing this too.

I'm not certain that the approach you've taken is quite right. There should be no need to query the opts::Output member from within the dumper. That's the purpose of the individual dump styles. There's barely any common code you've actually pulled out. What is the purpose of what you have done?

llvm/test/tools/llvm-readobj/elf-empty-version-info.test

2

What about a test for the llvm-readelf behaviour?

19–21

Can you get rid of the Sections: block?

llvm/tools/llvm-readobj/ELFDumper.cpp

354

This should be plural (i.e. printVersionSymbols) since it will print more than one symbol. The same goes for the other functions below (Definitions and Dependencies).

[Higuoxing]

In D60321#1468450, @jhenderson wrote:

As I'll be away next week, I've added a couple of others to help with reviewing this too.

I'm not certain that the approach you've taken is quite right. There should be no need to query the opts::Output member from within the dumper. That's the purpose of the individual dump styles. There's barely any common code you've actually pulled out. What is the purpose of what you have done?

Sure, I will try to get rid of it.

In D60321#1468450, @jhenderson wrote:

As I'll be away next week, I've added a couple of others to help with reviewing this too.

Thanks a lot. I will discuss with them.

llvm/test/tools/llvm-readobj/elf-empty-version-info.test

+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj -V %t | FileCheck %s
+
+# CHECK:      Version symbols {
+# CHECK-NEXT: }
+
+# CHECK:      SHT_GNU_verdef {
+# CHECK-NEXT: }
+
+# CHECK:      SHT_GNU_verneed {
+# CHECK-NEXT: }
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name: .text
+    Type: SHT_PROGBITS

llvm/tools/llvm-readobj/ELFDumper.cpp

std::string getFullSymbolName(const Elf_Sym *Symbol, StringRef StrTable,
                                 bool IsDynamic) const;
   void getSectionNameIndex(const Elf_Sym *Symbol, const Elf_Sym *FirstSym,
                            StringRef &SectionName,
                            unsigned &SectionIndex) const;
   std::string getStaticSymbolName(uint32_t Index) const;
   StringRef getSymbolVersionByIndex(StringRef StrTab,
                                     uint32_t VersionSymbolIndex,
                                     bool &IsDefault) const;
-
   void printSymbolsHelper(bool IsDynamic) const;
+  void printVersionSymbolSection(const ELFFile<ELFT> *Obj,
+                                 const Elf_Shdr *Sec) const;
+  void printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
+                                     const Elf_Shdr *Sec) const;
+  void printVersionDependencySection(const ELFFile<ELFT> *Obj,
+                                     const Elf_Shdr *Sec) const;
   const Elf_Shdr *getDotSymtabSec() const { return DotSymtabSec; }
   const Elf_Shdr *getDotCGProfileSec() const { return DotCGProfileSec; }
   const Elf_Shdr *getDotAddrsigSec() const { return DotAddrsigSec; }
   ArrayRef<Elf_Word> getShndxTable() const { return ShndxTable; }
   StringRef getDynamicStringTable() const { return DynamicStringTable; }
   const DynRegionInfo &getDynRelRegion() const { return DynRelRegion; }
   const DynRegionInfo &getDynRelaRegion() const { return DynRelaRegion; }
   const DynRegionInfo &getDynRelrRegion() const { return DynRelrRegion; }
public:
   virtual void printSymtabMessage(const ELFFile<ELFT> *Obj, StringRef Name,
                                   size_t Offset) {}
   virtual void printSymbol(const ELFFile<ELFT> *Obj, const Elf_Sym *Symbol,
                            const Elf_Sym *FirstSym, StringRef StrTable,
                            bool IsDynamic) = 0;
   virtual void printProgramHeaders(const ELFFile<ELFT> *Obj,
                                    bool PrintProgramHeaders,
                                    cl::boolOrDefault PrintSectionMapping) = 0;
-  virtual void printVersionSymbolSection(const ELFFile<ELFT> *Obj,
-                                         const Elf_Shdr *Sec) = 0;
-  virtual void printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
-                                             const Elf_Shdr *Sec) = 0;
-  virtual void printVersionDependencySection(const ELFFile<ELFT> *Obj,
-                                             const Elf_Shdr *Sec) = 0;
+  virtual void printVersionSymbol(const ELFFile<ELFT> *Obj, const Elf_Shdr *Sec,
+                                  StringRef SecName, StringRef StrTab,
+                                  const uint8_t *VersymBuf) = 0;
+  virtual void printVersionDefinition(const ELFFile<ELFT> *Obj,
+  virtual void printVersionDependency(const ELFFile<ELFT> *Obj,
   virtual void printHashHistogram(const ELFFile<ELFT> *Obj) = 0;
   virtual void printCGProfile(const ELFFile<ELFT> *Obj) = 0;
   virtual void printAddrsig(const ELFFile<ELFT> *Obj) = 0;
   virtual void printNotes(const ELFFile<ELFT> *Obj) = 0;
   virtual void printELFLinkerOptions(const ELFFile<ELFT> *Obj) = 0;
   virtual void printMipsGOT(const MipsGOTParser<ELFT> &Parser) = 0;
   virtual void printMipsPLT(const MipsGOTParser<ELFT> &Parser) = 0;
   const ELFDumper<ELFT> *dumper() const { return Dumper; }
public:
   void printSymbols(const ELFO *Obj, bool PrintSymbols,
                     bool PrintDynamicSymbols) override;
   void printHashSymbols(const ELFO *Obj) override;
   void printDynamicRelocations(const ELFO *Obj) override;
   void printSymtabMessage(const ELFO *Obj, StringRef Name,
                           size_t Offset) override;
   void printProgramHeaders(const ELFO *Obj, bool PrintProgramHeaders,
                            cl::boolOrDefault PrintSectionMapping) override;
-  void printVersionSymbolSection(const ELFFile<ELFT> *Obj,
-                                 const Elf_Shdr *Sec) override;
-  void printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
-                                     const Elf_Shdr *Sec) override;
-  void printVersionDependencySection(const ELFFile<ELFT> *Obj,
-                                     const Elf_Shdr *Sec) override;
+  void printVersionSymbol(const ELFFile<ELFT> *Obj, const Elf_Shdr *Sec,
+                          StringRef SecName, StringRef StrTab,
+                          const uint8_t *VersymBuf) override;
+  void printVersionDefinition(const ELFFile<ELFT> *Obj,
+  void printVersionDependency(const ELFFile<ELFT> *Obj,
   void printHashHistogram(const ELFFile<ELFT> *Obj) override;
   void printCGProfile(const ELFFile<ELFT> *Obj) override;
   void printAddrsig(const ELFFile<ELFT> *Obj) override;
   void printNotes(const ELFFile<ELFT> *Obj) override;
   void printELFLinkerOptions(const ELFFile<ELFT> *Obj) override;
   void printMipsGOT(const MipsGOTParser<ELFT> &Parser) override;
   void printMipsPLT(const MipsGOTParser<ELFT> &Parser) override;
 
public:
   void printRelocations(const ELFO *Obj) override;
   void printRelocations(const Elf_Shdr *Sec, const ELFO *Obj);
   void printSectionHeaders(const ELFO *Obj) override;
   void printSymbols(const ELFO *Obj, bool PrintSymbols,
                     bool PrintDynamicSymbols) override;
   void printDynamicRelocations(const ELFO *Obj) override;
   void printProgramHeaders(const ELFO *Obj, bool PrintProgramHeaders,
                            cl::boolOrDefault PrintSectionMapping) override;
-  void printVersionSymbolSection(const ELFFile<ELFT> *Obj,
-                                 const Elf_Shdr *Sec) override;
-  void printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
-                                     const Elf_Shdr *Sec) override;
-  void printVersionDependencySection(const ELFFile<ELFT> *Obj,
-                                     const Elf_Shdr *Sec) override;
+  void printVersionSymbol(const ELFFile<ELFT> *Obj, const Elf_Shdr *Sec,
+                          StringRef SecName, StringRef StrTab,
+                          const uint8_t *VersymBuf) override;
+  void printVersionDefinition(const ELFFile<ELFT> *Obj,
+  void printVersionDependency(const ELFFile<ELFT> *Obj,
   void printHashHistogram(const ELFFile<ELFT> *Obj) override;
   void printCGProfile(const ELFFile<ELFT> *Obj) override;
   void printAddrsig(const ELFFile<ELFT> *Obj) override;
   void printNotes(const ELFFile<ELFT> *Obj) override;
   void printELFLinkerOptions(const ELFFile<ELFT> *Obj) override;
   void printMipsGOT(const MipsGOTParser<ELFT> &Parser) override;
   void printMipsPLT(const MipsGOTParser<ELFT> &Parser) override;
 
StringRef ELFDumper<ELFT>::getSymbolVersion(StringRef StrTab,
 
   // Get the corresponding version index entry.
   const Elf_Versym *Versym =
       unwrapOrError(ObjF->getELFFile()->template getEntry<Elf_Versym>(
           SymbolVersionSection, EntryIndex));
   return this->getSymbolVersionByIndex(StrTab, Versym->vs_index, IsDefault);
 }
 
+template <typename ELFT>
+void ELFDumper<ELFT>::printVersionSymbolSection(const ELFFile<ELFT> *Obj,
+                                                const Elf_Shdr *Sec) const {
+  if (!Sec) {
+    if (opts::Output == opts::LLVM)
+      // FIXME: We should use DictScope() to print closure.
+      W.startLine() << "Version symbols {\n}\n";
+    return;
+  }
+
+  StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
+  StringRef StrTab = getDynamicStringTable();
+  const uint8_t *VersymBuf =
+      reinterpret_cast<const uint8_t *>(Obj->base() + Sec->sh_offset);
+
+  ELFDumperStyle->printVersionSymbol(Obj, Sec, SecName, StrTab, VersymBuf);
+}
+
+template <typename ELFT>
+void ELFDumper<ELFT>::printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
+                                                    const Elf_Shdr *Sec) const {
+  if (!Sec) {
+    if (opts::Output == opts::LLVM)
+      // FIXME: We should use DictScope() to print closure.
+      W.startLine() << "SHT_GNU_verdef {\n}\n";
+    return;
+  }
+
+  ELFDumperStyle->printVersionDefinition(Obj, Sec);
+}
+
+template <typename ELFT>
+void ELFDumper<ELFT>::printVersionDependencySection(const ELFFile<ELFT> *Obj,
+                                                    const Elf_Shdr *Sec) const {
+  if (!Sec) {
+    if (opts::Output == opts::LLVM)
+      // FIXME: We should use DictScope() to print closure.
+      W.startLine() << "SHT_GNU_verneed {\n}\n";
+    return;
+  }
+
+  ELFDumperStyle->printVersionDependency(Obj, Sec);
+}
+
 static std::string maybeDemangle(StringRef Name) {
   return opts::Demangle ? demangle(Name) : Name.str();
 }
 
 template <typename ELFT>
 std::string ELFDumper<ELFT>::getStaticSymbolName(uint32_t Index) const {
   const ELFFile<ELFT> *Obj = ObjF->getELFFile();
   StringRef StrTable = unwrapOrError(Obj->getStringTableForSymtab(*DotSymtabSec));

 void ELFDumper<ELFT>::printProgramHeaders(
     bool PrintProgramHeaders, cl::boolOrDefault PrintSectionMapping) {
   ELFDumperStyle->printProgramHeaders(ObjF->getELFFile(), PrintProgramHeaders,
                                       PrintSectionMapping);
 }
 
 template <typename ELFT> void ELFDumper<ELFT>::printVersionInfo() {
   // Dump version symbol section.
-  ELFDumperStyle->printVersionSymbolSection(ObjF->getELFFile(),
-                                            SymbolVersionSection);
+  printVersionSymbolSection(ObjF->getELFFile(), SymbolVersionSection);
 
   // Dump version definition section.
-  ELFDumperStyle->printVersionDefinitionSection(ObjF->getELFFile(),
-                                                SymbolVersionDefSection);
+  printVersionDefinitionSection(ObjF->getELFFile(), SymbolVersionDefSection);
 
   // Dump version dependency section.
-  ELFDumperStyle->printVersionDependencySection(ObjF->getELFFile(),
-                                                SymbolVersionNeedSection);
+  printVersionDependencySection(ObjF->getELFFile(), SymbolVersionNeedSection);
 }
 
 template <class ELFT> void ELFDumper<ELFT>::printDynamicRelocations() {
   ELFDumperStyle->printDynamicRelocations(ObjF->getELFFile());
 }
 
 template <class ELFT>
 void ELFDumper<ELFT>::printSymbols(bool PrintSymbols,
for (const Elf_Rel &Rel : DynPLTRelRegion.getAsArrayRef<Elf_Rel>()) {
       Rela.r_info = Rel.r_info;
       Rela.r_addend = 0;
       printDynamicRelocation(Obj, Rela, false);
     }
   }
 }
 
 template <class ELFT>
-void GNUStyle<ELFT>::printVersionSymbolSection(const ELFFile<ELFT> *Obj,
-                                               const Elf_Shdr *Sec) {
-  if (!Sec)
-    return;
-
-  StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
+void GNUStyle<ELFT>::printVersionSymbol(const ELFFile<ELFT> *Obj,
+                                        const Elf_Shdr *Sec, StringRef SecName,
+                                        StringRef StrTab,
+                                        const uint8_t *VersymBuf) {
   uint64_t Entries = Sec->sh_size / sizeof(Elf_Versym);
 
   OS << "Version symbols section '" << SecName << "' "
      << "contains " << Entries << " entries:\n";
 
   const Elf_Shdr *SymTab = unwrapOrError(Obj->getSection(Sec->sh_link));
   StringRef SymTabName = unwrapOrError(Obj->getSectionName(SymTab));
+  const ELFDumper<ELFT> *Dumper = this->dumper();
   OS << " Addr: " << format_hex_no_prefix(Sec->sh_addr, 16)
      << "  Offset: " << format_hex(Sec->sh_offset, 8)
      << "  Link: " << Sec->sh_link << " (" << SymTabName << ")\n";
 
-  const uint8_t *VersymBuf =
-      reinterpret_cast<const uint8_t *>(Obj->base() + Sec->sh_offset);
-  const ELFDumper<ELFT> *Dumper = this->dumper();
-  StringRef StrTable = Dumper->getDynamicStringTable();
-
   // readelf prints 4 entries per line.
   for (uint64_t VersymRow = 0; VersymRow < Entries; VersymRow += 4) {
     OS << "  " << format_hex_no_prefix(VersymRow, 3) << ":";
 
     for (uint64_t VersymIndex = 0;
          (VersymIndex < 4) && (VersymIndex + VersymRow) < Entries;
          ++VersymIndex) {
       const Elf_Versym *Versym =
           reinterpret_cast<const Elf_Versym *>(VersymBuf);
       switch (Versym->vs_index) {
       case 0:
         OS << "   0 (*local*)    ";
         break;
       case 1:
         OS << "   1 (*global*)   ";
         break;
       default:
         OS << format("%4x%c", Versym->vs_index & VERSYM_VERSION,
                      Versym->vs_index & VERSYM_HIDDEN ? 'h' : ' ');
 
         bool IsDefault = true;
         std::string VersionName = Dumper->getSymbolVersionByIndex(
-            StrTable, Versym->vs_index, IsDefault);
+            StrTab, Versym->vs_index, IsDefault);
 
         if (!VersionName.empty())
           VersionName = "(" + VersionName + ")";
         else
           VersionName = "(*invalid*)";
         OS << left_justify(VersionName, 13);
       }
       VersymBuf += sizeof(Elf_Versym);
     }
     OS << '\n';
   }
   OS << '\n';
 }
 
 template <class ELFT>
-void GNUStyle<ELFT>::printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
-                                                   const Elf_Shdr *Sec) {
-  if (!Sec)
-    return;
-
+void GNUStyle<ELFT>::printVersionDefinition(const ELFFile<ELFT> *Obj,
   StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
   OS << "Dumper for " << SecName << " is not implemented\n";
 }
 
 template <class ELFT>
-void GNUStyle<ELFT>::printVersionDependencySection(const ELFFile<ELFT> *Obj,
-                                                   const Elf_Shdr *Sec) {
-  if (!Sec)
-    return;
-
+void GNUStyle<ELFT>::printVersionDependency(const ELFFile<ELFT> *Obj,
   StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
   OS << "Dumper for " << SecName << " is not implemented\n";
 }
 
 // Hash histogram shows  statistics of how efficient the hash was for the
 // dynamic symbol table. The table shows number of hash buckets for different
 // lengths of chains as absolute number and percentage of the total buckets.
 // Additionally cumulative coverage of symbols for each set of buckets.
for (const Elf_Phdr &Phdr : unwrapOrError(Obj->program_headers())) {
     W.printNumber("FileSize", Phdr.p_filesz);
     W.printNumber("MemSize", Phdr.p_memsz);
     W.printFlags("Flags", Phdr.p_flags, makeArrayRef(ElfSegmentFlags));
     W.printNumber("Alignment", Phdr.p_align);
   }
 }
 
 template <class ELFT>
-void LLVMStyle<ELFT>::printVersionSymbolSection(const ELFFile<ELFT> *Obj,
-                                                const Elf_Shdr *Sec) {
+void LLVMStyle<ELFT>::printVersionSymbol(const ELFFile<ELFT> *Obj,
+                                         const Elf_Shdr *Sec, StringRef SecName,
+                                         StringRef StrTab,
+                                         const uint8_t *VersymBuf) {
   DictScope SS(W, "Version symbols");
-  if (!Sec)
-    return;
 
-  StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
   W.printNumber("Section Name", SecName, Sec->sh_name);
   W.printHex("Address", Sec->sh_addr);
   W.printHex("Offset", Sec->sh_offset);
   W.printNumber("Link", Sec->sh_link);
 
-  const uint8_t *VersymBuf =
-      reinterpret_cast<const uint8_t *>(Obj->base() + Sec->sh_offset);
   const ELFDumper<ELFT> *Dumper = this->dumper();
-  StringRef StrTable = Dumper->getDynamicStringTable();
 
   // Same number of entries in the dynamic symbol table (DT_SYMTAB).
   ListScope Syms(W, "Symbols");
   for (const Elf_Sym &Sym : Dumper->dynamic_symbols()) {
     DictScope S(W, "Symbol");
     const Elf_Versym *Versym = reinterpret_cast<const Elf_Versym *>(VersymBuf);
     std::string FullSymbolName =
-        Dumper->getFullSymbolName(&Sym, StrTable, true /* IsDynamic */);
+        Dumper->getFullSymbolName(&Sym, StrTab, true /* IsDynamic */);
     W.printNumber("Version", Versym->vs_index & VERSYM_VERSION);
     W.printString("Name", FullSymbolName);
     VersymBuf += sizeof(Elf_Versym);
   }
 }
 
 template <class ELFT>
-void LLVMStyle<ELFT>::printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
-                                                    const Elf_Shdr *Sec) {
+void LLVMStyle<ELFT>::printVersionDefinition(const ELFFile<ELFT> *Obj,
   DictScope SD(W, "SHT_GNU_verdef");
-  if (!Sec)
-    return;
 
   const uint8_t *SecStartAddress =
       reinterpret_cast<const uint8_t *>(Obj->base() + Sec->sh_offset);
   const uint8_t *SecEndAddress = SecStartAddress + Sec->sh_size;
   const uint8_t *VerdefBuf = SecStartAddress;
   const Elf_Shdr *StrTab = unwrapOrError(Obj->getSection(Sec->sh_link));
 
   unsigned VerDefsNum = Sec->sh_info;
if (Verdef->vd_cnt == 2) {
                     StringRef(reinterpret_cast<const char *>(
                         Obj->base() + StrTab->sh_offset + Verdaux->vda_name)));
     }
     VerdefBuf += Verdef->vd_next;
   }
 }
 
 template <class ELFT>
-void LLVMStyle<ELFT>::printVersionDependencySection(const ELFFile<ELFT> *Obj,
-                                                    const Elf_Shdr *Sec) {
+void LLVMStyle<ELFT>::printVersionDependency(const ELFFile<ELFT> *Obj,
   DictScope SD(W, "SHT_GNU_verneed");
-  if (!Sec)
-    return;
 
   const uint8_t *SecData =
       reinterpret_cast<const uint8_t *>(Obj->base() + Sec->sh_offset);
   const Elf_Shdr *StrTab = unwrapOrError(Obj->getSection(Sec->sh_link));
 
   const uint8_t *VerneedBuf = SecData;
   unsigned VerneedNum = Sec->sh_info;
   for (unsigned I = 0; I < VerneedNum; ++I) {