[llvm-readobj] - Don't get the name of the symbol table in ELFDumper<ELFT>::printSymbolsHelper.

[grimar]

Summary

It was requested in D84173 thread to not do it, because otherwise we extract and
check the name of the symbol table in LLVM style, but do not use it and
might report a warning which perhaps might be confusing.

[MaskRay]

llvm/tools/llvm-readobj/ELFDumper.cpp

3956

The original messages carry more information as a symbol table can represent either SHT_DYNSYM or SHT_SYMTAB... I am fine combing them, though

[jhenderson]

llvm/test/Object/invalid.test

292

This seems to be missing context that was useful before? In other words, what went wrong here?

llvm/tools/llvm-readobj/ELFDumper.cpp

3956

Could you use the new describe function from D84240 to resolve that?

[grimar]

• Addressed review comments.

llvm/test/Object/invalid.test

292

The llvm-readobj doesn't try to read a name for a symbol table in ELFDumper<ELFT>::printSymbolsHelper
for no reason anymore.

The original YAML produces no warning with this patch. I had to change it slightly
to force the llvm-readobj code to try to read the .shstrtab section name again.
So the different message is shown, but the reason (an invalid sh_offset, which is tested here) remains the same,

llvm/tools/llvm-readobj/ELFDumper.cpp

3956

Could you use the new describe function from D84240 to resolve that?

Yes. Done.

[jhenderson]

LGTM.

llvm/test/Object/invalid.test

FileHeader:
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 Sections:
   - Name:     .rela
     Type:     SHT_RELA
     ShOffset: 0x10000
 
-## Check that llvm-objdump reports a warning when we try to print symbols and
+## Check that llvm-readobj reports a warning when we try to print section symbols and
 ## .shstrtab has a broken sh_offset so large that sh_offset + sh_size overflows the platform address size type.
 
 # RUN: yaml2obj %s --docnum=14 -o %t14
 # RUN: llvm-readobj --symbols %t14 2>&1 | FileCheck -DFILE=%t14 --check-prefix=INVALID-SECTION-SIZE2 %s
 
-# INVALID-SECTION-SIZE2: warning: '[[FILE]]': unable to get the name of the SHT_SYMTAB section: section [index 1] has a sh_offset (0xffffffff) + sh_size (0x1b) that cannot be represented
+# INVALID-SECTION-SIZE2:      Symbol {
+# INVALID-SECTION-SIZE2:       Name: symbol (1)
+# INVALID-SECTION-SIZE2-NEXT:  Value: 0x123
+# INVALID-SECTION-SIZE2-NEXT:  Size: 0
+# INVALID-SECTION-SIZE2-NEXT:  Binding: Local (0x0)
+# INVALID-SECTION-SIZE2-NEXT:  Type: None (0x0)
+# INVALID-SECTION-SIZE2-NEXT:  Other: 0
+# INVALID-SECTION-SIZE2-NEXT: warning: '[[FILE]]': section [index 1] has a sh_offset (0xffffffff) + sh_size (0x1b) that cannot be represented
+# INVALID-SECTION-SIZE2-NEXT:  Section: <?> (0x1)
+# INVALID-SECTION-SIZE2-NEXT: }
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS32
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_386
 Sections:
   - Name:     .shstrtab
     Type:     SHT_STRTAB
     ShOffset: 0xFFFFFFFF
-Symbols: []
+Symbols:
+  - Name:    symbol
+    Section: .shstrtab
+    Value:   0x123
 
 ## Check that llvm-readobj reports an error when trying to dump sections
 ## when the e_shnum field is broken (is greater than the actual number of sections).
 
 # RUN: yaml2obj %s --docnum=15 -o %t15
 # RUN: not llvm-readobj -S %t15 2>&1 | FileCheck --check-prefix=INVALID-SECTION-NUM %s
 
 # INVALID-SECTION-NUM: error: {{.*}}: section table goes past the end of file

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

 ## c) Check --dyn-syms equals --dyn-symbols for llvm-readelf.
 # RUN: llvm-readelf --dyn-syms %t1.so > %t.readelf-dyn-syms
 # RUN: llvm-readelf --dyn-symbols %t1.so > %t.readelf-dyn-symbols
 # RUN: cmp %t.readelf-dyn-symbols %t.readelf-dyn-syms
 
 ## Case 3.1: Check that we are able to dump the dynamic symbol table even when we have no program headers.
 ## In this case we find the table by it's type (SHT_DYNSYM) and ignore the DT_SYMTAB value.
 # RUN: yaml2obj --docnum=2 %s -o %t2.so
-# RUN: llvm-readobj %t2.so --dyn-symbols | FileCheck %s --check-prefix=NOPHDRS-LLVM
-# RUN: llvm-readelf %t2.so --dyn-symbols | FileCheck %s -DNAME=.dynsym --check-prefix=NOPHDRS-GNU
+# RUN: llvm-readobj %t2.so --dyn-symbols 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t2.so --implicit-check-not=warning: --check-prefix=NOPHDRS-LLVM
+# RUN: llvm-readelf %t2.so --dyn-symbols 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t2.so --implicit-check-not=warning: -DNAME=.dynsym --check-prefix=NOPHDRS-GNU
 
+# NOPHDRS-LLVM: warning: '[[FILE]]': Unable to parse DT_SYMTAB: virtual address is not in any segment: 0xffff1234
 # NOPHDRS-LLVM:      DynamicSymbols [
-# NOPHDRS-WARN:      warning: '[[FILE]]': unable to get the name of the SHT_DYNSYM section: a section [index 2] has an invalid sh_name (0xffffffff) offset which goes past the end of the section name string table
 # NOPHDRS-LLVM-NEXT:   Symbol {
 # NOPHDRS-LLVM-NEXT:     Name:  (0)
 # NOPHDRS-LLVM-NEXT:     Value: 0x0
 # NOPHDRS-LLVM-NEXT:     Size: 0
 # NOPHDRS-LLVM-NEXT:     Binding: Local (0x0)
 # NOPHDRS-LLVM-NEXT:     Type: None (0x0)
 # NOPHDRS-LLVM-NEXT:     Other: 0
 # NOPHDRS-LLVM-NEXT:     Section: Undefined (0x0)
 # NOPHDRS-LLVM-NEXT:   }
 # NOPHDRS-LLVM-NEXT:   Symbol {
 # NOPHDRS-LLVM-NEXT:     Name: foo (1)
 # NOPHDRS-LLVM-NEXT:     Value: 0x0
 # NOPHDRS-LLVM-NEXT:     Size: 0
 # NOPHDRS-LLVM-NEXT:     Binding: Local (0x0)
 # NOPHDRS-LLVM-NEXT:     Type: None (0x0)
 # NOPHDRS-LLVM-NEXT:     Other: 0
 # NOPHDRS-LLVM-NEXT:     Section: Undefined (0x0)
 # NOPHDRS-LLVM-NEXT:   }
 # NOPHDRS-LLVM-NEXT: ]
 
+# NOPHDRS-GNU:      warning: '[[FILE]]': Unable to parse DT_SYMTAB: virtual address is not in any segment: 0xffff1234
+# NOPHDRS-NAMEWARN: warning: '[[FILE]]': unable to get the name of SHT_DYNSYM section with index 2: a section [index 2] has an invalid sh_name (0xffffffff) offset which goes past the end of the section name string table
 # NOPHDRS-GNU:      Symbol table '[[NAME]]' contains 2 entries:
 # NOPHDRS-GNU-NEXT:  Num:    Value          Size Type    Bind   Vis       Ndx Name
 # NOPHDRS-GNU-NEXT:    0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND
 # NOPHDRS-GNU-NEXT:    1: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND foo
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
Entries:
         Value: 0
 DynamicSymbols:
   - Name: foo
 
 ## Case 3.2: the same as 3.1, but the sh_name field of the SHT_DYNSYM section is invalid.
 ##           Check we are still able to dump symbols.
 # RUN: yaml2obj --docnum=3 %s -o %t2.broken.name
 # RUN: llvm-readobj %t2.broken.name --dyn-symbols 2>&1 | \
-# RUN:   FileCheck %s -DFILE=%t2.broken.name --check-prefixes=NOPHDRS-LLVM,NOPHDRS-WARN
+# RUN:   FileCheck %s -DFILE=%t2.broken.name --check-prefix=NOPHDRS-LLVM --implicit-check-not=warning:
 # RUN: llvm-readelf %t2.broken.name --dyn-symbols 2>&1 | \
-# RUN:   FileCheck %s -DFILE=%t2.broken.name -DNAME="<?>" --check-prefixes=NOPHDRS-GNU,NOPHDRS-WARN
+# RUN:   FileCheck %s -DFILE=%t2.broken.name -DNAME="<?>" \
+# RUN:     --check-prefixes=NOPHDRS-GNU,NOPHDRS-NAMEWARN --implicit-check-not=warning:
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:

llvm/test/tools/llvm-readobj/ELF/symbols.test

 
 # SYMTAB-ENTSIZE-ERR-GNU:     warning: '[[FILE]]': unable to read symbols from the SHT_SYMTAB section: section [index 1] has an invalid sh_entsize: 255
 # SYMTAB-ENTSIZE-ERR-GNU-NOT: {{.}}
 
 ## Case 3: check we report a warning, but continue dumping, when we are unable to read the name of the SHT_SYMTAB section.
 ##         In this case we set the e_shstrndx field to an invalid index so that the .shstrtab section can't be located.
 # RUN: yaml2obj --docnum=2 -DSHSTRTAB=0xff %s -o %t64.err3
 # RUN: llvm-readobj --symbols %t64.err3 2>&1 | \
-# RUN:  FileCheck %s -DFILE=%t64.err3 --check-prefix=SYMTAB-SHSTRTAB-ERR-LLVM
+# RUN:  FileCheck %s -DFILE=%t64.err3 --check-prefix=SYMTAB-SHSTRTAB-ERR-LLVM --implicit-check-not=warning:
 # RUN: llvm-readelf --symbols %t64.err3 2>&1 | \
-# RUN:  FileCheck %s -DFILE=%t64.err3 --check-prefix=SYMTAB-SHSTRTAB-ERR-GNU
+# RUN:  FileCheck %s -DFILE=%t64.err3 --check-prefix=SYMTAB-SHSTRTAB-ERR-GNU --implicit-check-not=warning:
 
 # SYMTAB-SHSTRTAB-ERR-LLVM:      Symbols [
-# SYMTAB-SHSTRTAB-ERR-LLVM-NEXT: warning: '[[FILE]]': unable to get the name of the SHT_SYMTAB section: section header string table index 255 does not exist
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:   Symbol {
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Name:  (0)
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Value: 0x0
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Size: 0
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Binding: Local (0x0)
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Type: None (0x0)
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Other: 0
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Section: Undefined (0x0)
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:   }
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:   Symbol {
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Name: foo (1)
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Value: 0x1
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Size: 0
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Binding: Global (0x1)
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Type: None (0x0)
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Other: 0
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT: warning: '[[FILE]]': section header string table index 255 does not exist
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:     Section: <?> (0x1)
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT:   }
 # SYMTAB-SHSTRTAB-ERR-LLVM-NEXT: ]
 
-# SYMTAB-SHSTRTAB-ERR-GNU:      warning: '[[FILE]]': unable to get the name of the SHT_SYMTAB section: section header string table index 255 does not exist
+# SYMTAB-SHSTRTAB-ERR-GNU:      warning: '[[FILE]]': unable to get the name of SHT_SYMTAB section with index 1: section header string table index 255 does not exist
 # SYMTAB-SHSTRTAB-ERR-GNU:      Symbol table '<?>' contains 2 entries:
 # SYMTAB-SHSTRTAB-ERR-GNU-NEXT:    Num:    Value          Size Type    Bind   Vis       Ndx Name
 # SYMTAB-SHSTRTAB-ERR-GNU-NEXT:      0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND
 # SYMTAB-SHSTRTAB-ERR-GNU-NEXT:      1: 0000000000000001     0 NOTYPE  GLOBAL DEFAULT     1 foo

llvm/tools/llvm-readobj/ELFDumper.cpp

for (unsigned I = 1; I <= /*VerneedNum=*/Sec->sh_info; ++I) {
     VerneedBuf += Verneed->vn_next;
   }
   return Ret;
 }
 
 template <class ELFT>
 void ELFDumper<ELFT>::printSymbolsHelper(bool IsDynamic) const {
   Optional<StringRef> StrTable;
-  StringRef SymtabName;
   size_t Entries = 0;
   Elf_Sym_Range Syms(nullptr, nullptr);
   const ELFFile<ELFT> *Obj = ObjF->getELFFile();
+  const Elf_Shdr *SymtabSec = IsDynamic ? DotDynsymSec : DotSymtabSec;
+
   if (IsDynamic) {
     StrTable = DynamicStringTable;
     Syms = dynamic_symbols();
-
-    if (DotDynsymSec) {
-      if (Expected<StringRef> NameOrErr = Obj->getSectionName(DotDynsymSec)) {
-        SymtabName = *NameOrErr;
-      } else {
-        reportUniqueWarning(
-            createError("unable to get the name of the SHT_DYNSYM section: " +
-                        toString(NameOrErr.takeError())));
-        SymtabName = "<?>";
-      }
-    }
-
     Entries = Syms.size();
-  } else {
-    if (!DotSymtabSec)
-      return;
-
+  } else if (DotSymtabSec) {
     if (Expected<StringRef> StrTableOrErr =
             Obj->getStringTableForSymtab(*DotSymtabSec))
       StrTable = *StrTableOrErr;
     else
       reportUniqueWarning(createError(
           "unable to get the string table for the SHT_SYMTAB section: " +
           toString(StrTableOrErr.takeError())));
 
     if (Expected<Elf_Sym_Range> SymsOrErr = Obj->symbols(DotSymtabSec))
       Syms = *SymsOrErr;
     else
       reportUniqueWarning(
           createError("unable to read symbols from the SHT_SYMTAB section: " +
                       toString(SymsOrErr.takeError())));
-
-    if (Expected<StringRef> SymtabNameOrErr =
-            Obj->getSectionName(DotSymtabSec)) {
-      SymtabName = *SymtabNameOrErr;
-    } else {
-      reportUniqueWarning(
-          createError("unable to get the name of the SHT_SYMTAB section: " +
-                      toString(SymtabNameOrErr.takeError())));
-      SymtabName = "<?>";
-    }
-
     Entries = DotSymtabSec->getEntityCount();
   }
   if (Syms.begin() == Syms.end())
     return;
 
   // The st_other field has 2 logical parts. The first two bits hold the symbol
   // visibility (STV_*) and the remainder hold other platform-specific values.
   bool NonVisibilityBitsUsed = llvm::find_if(Syms, [](const Elf_Sym &S) {
                                  return S.st_other & ~0x3;
                                }) != Syms.end();
 
-  ELFDumperStyle->printSymtabMessage(Obj, SymtabName, Entries,
+  ELFDumperStyle->printSymtabMessage(Obj, SymtabSec, Entries,
                                      NonVisibilityBitsUsed);
   for (const auto &Sym : Syms)
     ELFDumperStyle->printSymbol(Obj, &Sym, Syms.begin(), StrTable, IsDynamic,
                                 NonVisibilityBitsUsed);
 }
 
 template <class ELFT> class MipsGOTParser;
 
public:
   virtual void printRelocations(const ELFFile<ELFT> *Obj) = 0;
   virtual void printSectionHeaders(const ELFFile<ELFT> *Obj) = 0;
   virtual void printSymbols(const ELFFile<ELFT> *Obj, bool PrintSymbols,
                             bool PrintDynamicSymbols) = 0;
   virtual void printHashSymbols(const ELFFile<ELFT> *Obj) {}
   virtual void printDependentLibs(const ELFFile<ELFT> *Obj) = 0;
   virtual void printDynamic(const ELFFile<ELFT> *Obj) {}
   virtual void printDynamicRelocations(const ELFFile<ELFT> *Obj) = 0;
-  virtual void printSymtabMessage(const ELFFile<ELFT> *Obj, StringRef Name,
-                                  size_t Offset, bool NonVisibilityBitsUsed) {}
+  virtual void printSymtabMessage(const ELFFile<ELFT> *Obj,
+                                  const Elf_Shdr *Symtab, size_t Offset,
+                                  bool NonVisibilityBitsUsed) {}
   virtual void printSymbol(const ELFFile<ELFT> *Obj, const Elf_Sym *Symbol,
                            const Elf_Sym *FirstSym,
                            Optional<StringRef> StrTable, bool IsDynamic,
                            bool NonVisibilityBitsUsed) = 0;
   virtual void printProgramHeaders(const ELFFile<ELFT> *Obj,
                                    bool PrintProgramHeaders,
                                    cl::boolOrDefault PrintSectionMapping) = 0;
   virtual void printVersionSymbolSection(const ELFFile<ELFT> *Obj,
public:
   void printRelocations(const ELFO *Obj) override;
   void printSectionHeaders(const ELFO *Obj) override;
   void printSymbols(const ELFO *Obj, bool PrintSymbols,
                     bool PrintDynamicSymbols) override;
   void printHashSymbols(const ELFO *Obj) override;
   void printDependentLibs(const ELFFile<ELFT> *Obj) override;
   void printDynamic(const ELFFile<ELFT> *Obj) override;
   void printDynamicRelocations(const ELFO *Obj) override;
-  void printSymtabMessage(const ELFO *Obj, StringRef Name, size_t Offset,
-                          bool NonVisibilityBitsUsed) override;
+  void printSymtabMessage(const ELFO *Obj, const Elf_Shdr *Symtab,
+                          size_t Offset, bool NonVisibilityBitsUsed) 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;
for (const Elf_Shdr &Sec : Sections) {
     OS << right_justify(Fields[10].Str, 2);
     OS << "\n";
     ++SectionIndex;
   }
   printSectionDescription(OS, Obj->getHeader()->e_machine);
 }
 
 template <class ELFT>
-void GNUStyle<ELFT>::printSymtabMessage(const ELFO *Obj, StringRef Name,
+void GNUStyle<ELFT>::printSymtabMessage(const ELFO *Obj, const Elf_Shdr *Symtab,
                                         size_t Entries,
                                         bool NonVisibilityBitsUsed) {
+  StringRef Name;
+  if (Symtab) {
+    if (Expected<StringRef> NameOrErr = Obj->getSectionName(Symtab)) {
+      Name = *NameOrErr;
+    } else {
+      this->reportUniqueWarning(createError("unable to get the name of " +
+                                            describe(Obj, *Symtab) + ": " +
+                                            toString(NameOrErr.takeError())));
+      Name = "<?>";
+    }
+  }
+
   if (!Name.empty())
     OS << "\nSymbol table '" << Name << "'";
   else
     OS << "\nSymbol table for image";
   OS << " contains " << Entries << " entries:\n";
 
   if (ELFT::Is64Bits)
     OS << "   Num:    Value          Size Type    Bind   Vis";