[llvm-readobj] - Improve error reporting for hash tables.

[grimar]

Summary

This improves the next points for broken hash tables:

1. Use reportUniqueWarning to prevent duplication when --hash-table and --elf-hash-histogram are used together.

2. Dump nbuckets and nchain fields. It is often possible to dump them even when the table itself goes past the EOF etc.

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/hash-table.test

233

Nit: I'd indent this line one more space (i.e. to be two spaces more indented than the line above).

llvm/tools/llvm-readobj/ELFDumper.cpp

2640

Up to you, but you could avoid needing to pass in the Obj as well, by using Dumper.getElfObject()->getELFFile(), I believe.

2667–2668

What happens if the HashTable ends before the nbucket/nchain fields can be read?

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

2667–2668

Seems nothing good.. We have a test (--docnum=3), but I think it just a luck that it "worked" for me.
The problem is probably not about where HashTable end, but where file buffer ends: it would be probably OK to
read past the end of the section, but not past the end of the file. That what I missed here and in D80215.
I think this change is better to be just reverted.

[grimar]

• Updated implementation.
• Addressed review comments.

llvm/tools/llvm-readobj/ELFDumper.cpp

2640

Thanks for this hint, I've used it somewhere else. The Dumper argument is gone.

2667–2668

I've updated the implementation to be similar to one for the GNU hash table.

[jhenderson]

LGTM, with nit.

llvm/tools/llvm-readobj/ELFDumper.cpp

2650

Nit: I'd rename this MakeError, because I read BuildError and think there's something wrong with the build, rather than that this is for building an error.

llvm/test/tools/llvm-readobj/ELF/hash-table.test

 ## Each SHT_HASH section starts with two 32-bit fields: nbucket and nchain.
 ## Check we report an error when a DT_HASH value points to data that has size less than 8 bytes.
 
 # RUN: yaml2obj --docnum=3 %s -o %t4.o
 # RUN: llvm-readelf --hash-table %t4.o 2>&1 | FileCheck %s --check-prefix=ERR1 -DFILE=%t4.o
 # RUN: llvm-readobj --hash-table %t4.o 2>&1 | FileCheck %s --check-prefix=ERR1 -DFILE=%t4.o
 
 # ERR1:      HashTable {
+# ERR1-NEXT:  Num Buckets: 0
+# ERR1-NEXT:  Num Chains: 0
 # ERR1-NEXT:  warning: '[[FILE]]': the hash table at offset 0x2b1 goes past the end of the file (0x2b8){{$}}
 # ERR1-NEXT: }
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN

 ##           field that has a value larger than the number of buckets.
 # RUN: yaml2obj --docnum=4 %s -o %t5.2.o -DNBUCKET=0x5e -DNCHAIN=0x1
 # RUN: llvm-readelf --hash-table %t5.2.o 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=ERR2 -DFILE=%t5.2.o --implicit-check-not="warning:"
 # RUN: llvm-readobj --hash-table %t5.2.o 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=ERR2 -DFILE=%t5.2.o --implicit-check-not="warning:"
 
 # ERR2:      HashTable {
+# ERR2:       Num Buckets: 94
+# ERR2:       Num Chains: 1
 # ERR2-NEXT:  warning: '[[FILE]]': the hash table at offset 0x54 goes past the end of the file (0x1d4), nbucket = 94, nchain = 1{{$}}
 # ERR2-NEXT: }
 
 ## Case B.1: the hash table ends right before the EOF. We have a broken nchain
 ##           field that has a value larger than the number of chains.
 # RUN: yaml2obj --docnum=4 %s -o %t5.3.o -DNBUCKET=0x1 -DNCHAIN=0x5d
 # RUN: llvm-readelf --hash-table %t5.3.o 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=NOERR2 -DFILE=%t5.3.o --implicit-check-not="warning:"

 # RUN: yaml2obj --docnum=4 %s -o %t5.4.o -DNBUCKET=0x1 -DNCHAIN=0x5e
 # RUN: llvm-readelf --hash-table %t5.4.o 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=ERR3 -DFILE=%t5.4.o --implicit-check-not="warning:"
 # RUN: llvm-readobj --hash-table %t5.4.o 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=ERR3 -DFILE=%t5.4.o --implicit-check-not="warning:"
 
 # ERR3:      warning: '[[FILE]]': hash table nchain (94) differs from symbol count derived from SHT_DYNSYM section header (1)
 # ERR3:      HashTable {
+# ERR3-NEXT:  Num Buckets: 1
+# ERR3-NEXT:  Num Chains: 94
 # ERR3-NEXT:  warning: '[[FILE]]': the hash table at offset 0x54 goes past the end of the file (0x1d4), nbucket = 1, nchain = 94{{$}}
 # ERR3-NEXT: }
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS32
   Data:    ELFDATA2LSB
   Type:    ET_DYN
Entries:
       - Tag:   DT_NULL
         Value: 0x0
 DynamicSymbols: []
 ProgramHeaders:
   - Type: PT_LOAD
     Sections:
       - Section: .hash
       - Section: .dynamic
+
+## Show we do not duplicate warnings when printing both the hash table and the hash histogram.
+## Note that --elf-hash-histogram is only implemented for llvm-readelf currently.
+# RUN: yaml2obj --docnum=3 %s -o %t4.o
+# RUN: llvm-readelf --hash-table --elf-hash-histogram %t4.o 2>&1 \
+# RUN:  | FileCheck %s --check-prefix=SINGLE-WARN -DFILE=%t4.o --implicit-check-not="warning:"
+
+# SINGLE-WARN:      warning: '[[FILE]]': hash table nchain (0) differs from symbol count derived from SHT_DYNSYM section header (1)
+# SINGLE-WARN-NEXT: HashTable {
+# SINGLE-WARN-NEXT:  Num Buckets: 0
+# SINGLE-WARN-NEXT:  Num Chains: 0
+# SINGLE-WARN-NEXT: warning: '[[FILE]]': the hash table at offset 0x2b1 goes past the end of the file (0x2b8)
+# SINGLE-WARN-NEXT: }

llvm/tools/llvm-readobj/ELFDumper.cpp

template <class ELFT> void ELFDumper<ELFT>::printNeededLibraries() {
 
   llvm::sort(Libs);
 
   for (StringRef L : Libs)
     W.startLine() << L << "\n";
 }
 
 template <class ELFT>
-static bool checkHashTable(const ELFFile<ELFT> *Obj,
-                           const typename ELFT::Hash *H, StringRef FileName) {
+static bool checkHashTable(const ELFDumper<ELFT> &Dumper,
+                           const ELFFile<ELFT> *Obj,
+                           const typename ELFT::Hash *H) {
   auto WarnAndReturn = [&](uint64_t Off, const Twine &Msg = "") {
-    reportWarning(createError("the hash table at offset 0x" +
-                              Twine::utohexstr(Off) +
-                              " goes past the end of the file (0x" +
-                              Twine::utohexstr(Obj->getBufSize()) + ")" + Msg),
-                  FileName);
+    Dumper.reportUniqueWarning(
+        createError("the hash table at offset 0x" + Twine::utohexstr(Off) +
+                    Twine::utohexstr(Obj->getBufSize()) + ")" + Msg));
     return false;
   };
 
   // Each SHT_HASH section starts from two 32-bit fields: nbucket and nchain.
   const unsigned HeaderSize = 2 * sizeof(typename ELFT::Word);
   const uint64_t SecOffset = (const uint8_t *)H - Obj->base();
   if (Obj->getBufSize() - SecOffset < HeaderSize)
     return WarnAndReturn(SecOffset);
 
   if (Obj->getBufSize() - SecOffset - HeaderSize <
       ((uint64_t)H->nbucket + H->nchain) * sizeof(typename ELFT::Word))
     return WarnAndReturn(SecOffset, ", nbucket = " + Twine(H->nbucket) +
                                         ", nchain = " + Twine(H->nchain));
   return true;
 }
 
 template <typename ELFT> void ELFDumper<ELFT>::printHashTable() {
   DictScope D(W, "HashTable");
-  if (!HashTable ||
-      !checkHashTable(ObjF->getELFFile(), HashTable, ObjF->getFileName()))
+  if (!HashTable)
     return;
   W.printNumber("Num Buckets", HashTable->nbucket);
   W.printNumber("Num Chains", HashTable->nchain);
+  if (!checkHashTable(*this, ObjF->getELFFile(), HashTable))
+    return;
+
   W.printList("Buckets", HashTable->buckets());
   W.printList("Chains", HashTable->chains());
 }
 
 template <typename ELFT>
 void ELFDumper<ELFT>::printGnuHashTable(const object::ObjectFile *Obj) {
   DictScope D(W, "GnuHashTable");
   if (!GnuHashTable)
for (uint32_t Buc = 0; Buc < SysVHash->nbucket; Buc++) {
         printHashedSymbol(Obj, &DynSyms[0], Ch, StringTable, Buc);
         Visited[Ch] = true;
       }
     }
   };
 
   if (const Elf_Hash *SysVHash = this->dumper()->getHashTable()) {
     OS << "\n Symbol table of .hash for image:\n";
-    if (checkHashTable(Obj, SysVHash, this->FileName))
+    if (checkHashTable(*this->dumper(), Obj, SysVHash))
       PrintHashTable(SysVHash);
   }
 
   // Try printing .gnu.hash
   if (auto GnuHash = this->dumper()->getGnuHashTable()) {
     OS << "\n Symbol table of .gnu.hash for image:\n";
     if (ELFT::Is64Bits)
       OS << "  Num Buc:    Value          Size   Type   Bind Vis      Ndx Name";

 // 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.
 template <class ELFT>
 void GNUStyle<ELFT>::printHashHistogram(const ELFFile<ELFT> *Obj) {
   // Print histogram for .hash section
   if (const Elf_Hash *HashTable = this->dumper()->getHashTable()) {
-    if (!checkHashTable(Obj, HashTable, this->FileName))
+    if (!checkHashTable(*this->dumper(), Obj, HashTable))
       return;
 
     size_t NBucket = HashTable->nbucket;
     size_t NChain = HashTable->nchain;
     ArrayRef<Elf_Word> Buckets = HashTable->buckets();
     ArrayRef<Elf_Word> Chains = HashTable->chains();
     size_t TotalSyms = 0;
     // If hash table is correct, we have at least chains with 0 length