[llvm-readobj] - Don't get the name of the symbol table in ELFDumper<…

…ELFT>::printSymbolsHelper. 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. Differential revision: https://reviews.llvm.org/D84231
llvm · Jul 22, 2020 · e2529e2 · e2529e2
1 parent f78fe57
commit e2529e2
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 46 deletions.
diff --git a/llvm/test/Object/invalid.test b/llvm/test/Object/invalid.test
@@ -276,13 +276,22 @@ Sections:
     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:
@@ -294,7 +303,10 @@ 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).

diff --git a/llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test b/llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
@@ -86,11 +86,13 @@ ProgramHeaders:
 ## 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
@@ -111,6 +113,8 @@ ProgramHeaders:
 # 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
@@ -137,9 +141,10 @@ DynamicSymbols:
 ##           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:

diff --git a/llvm/test/tools/llvm-readobj/ELF/symbols.test b/llvm/test/tools/llvm-readobj/ELF/symbols.test
@@ -195,12 +195,11 @@ Symbols:
 ##         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
@@ -222,7 +221,7 @@ Symbols:
 # 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

diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -657,30 +657,16 @@ ELFDumper<ELFT>::getVersionDependencies(const Elf_Shdr *Sec) const {
 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;
@@ -695,17 +681,6 @@ void ELFDumper<ELFT>::printSymbolsHelper(bool IsDynamic) const {
       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())
@@ -717,7 +692,7 @@ void ELFDumper<ELFT>::printSymbolsHelper(bool IsDynamic) const {
                                  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,
@@ -748,8 +723,9 @@ template <typename ELFT> class DumpStyle {
   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,
@@ -822,8 +798,8 @@ template <typename ELFT> class GNUStyle : public DumpStyle<ELFT> {
   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,
@@ -3967,9 +3943,21 @@ void GNUStyle<ELFT>::printSectionHeaders(const ELFO *Obj) {
 }
 
 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