[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

231

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

2645

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

2707–2709

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

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

2707–2709

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

2645

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

2707–2709

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

2648

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

 ##           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: 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) {
-  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);
-    return false;
+static Error checkHashTable(const ELFFile<ELFT> *Obj,
+                            const typename ELFT::Hash *H,
+                            bool *IsHeaderValid = nullptr) {
+  auto MakeError = [&](uint64_t Off, const Twine &Msg = "") {
+    return createError("the hash table at offset 0x" + Twine::utohexstr(Off) +
+                       Twine::utohexstr(Obj->getBufSize()) + ")" + Msg);
   };
 
   // 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 (IsHeaderValid)
+    *IsHeaderValid = Obj->getBufSize() - SecOffset >= HeaderSize;
+
   if (Obj->getBufSize() - SecOffset < HeaderSize)
-    return WarnAndReturn(SecOffset);
+    return MakeError(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;
+    return MakeError(SecOffset, ", nbucket = " + Twine(H->nbucket) +
+  return Error::success();
 }
 
 template <class ELFT>
 static Error checkGNUHashTable(const ELFFile<ELFT> *Obj,
                                const typename ELFT::GnuHash *GnuHashTable,
                                bool *IsHeaderValid = nullptr) {
   const uint8_t *TableData = reinterpret_cast<const uint8_t *>(GnuHashTable);
   assert(TableData >= Obj->base() &&
return createError("unable to dump the SHT_GNU_HASH "
                        "section at 0x" +
                        Twine::utohexstr(TableOffset) +
                        ": it goes past the end of the file");
   return Error::success();
 }
 
 template <typename ELFT> void ELFDumper<ELFT>::printHashTable() {
   DictScope D(W, "HashTable");
-  if (!HashTable ||
-      !checkHashTable(ObjF->getELFFile(), HashTable, ObjF->getFileName()))
+  if (!HashTable)
     return;
+
+  bool IsHeaderValid;
+  Error Err = checkHashTable(ObjF->getELFFile(), HashTable, &IsHeaderValid);
+  if (IsHeaderValid) {
     W.printNumber("Num Buckets", HashTable->nbucket);
     W.printNumber("Num Chains", HashTable->nchain);
+  }
+
+  if (Err) {
+    reportUniqueWarning(std::move(Err));
+    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 (Error E = checkHashTable<ELFT>(Obj, SysVHash))
+      this->reportUniqueWarning(std::move(E));
+    else
       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 the number of hash buckets for
 // different lengths of chains as an absolute number and percentage of the total
 // buckets, and the cumulative coverage of symbols for each set of buckets.
 template <class ELFT>
 void GNUStyle<ELFT>::printHashHistograms(const ELFFile<ELFT> *Obj) {
   // Print histogram for the .hash section.
   if (const Elf_Hash *HashTable = this->dumper()->getHashTable())
-    if (checkHashTable(Obj, HashTable, this->FileName))
+    if (Error E = checkHashTable<ELFT>(Obj, HashTable))
+      this->reportUniqueWarning(std::move(E));
+    else
       printHashHistogram(*HashTable);
 
   // Print histogram for the .gnu.hash section.
   if (const Elf_GnuHash *GnuHashTable = this->dumper()->getGnuHashTable()) {
     if (Error E = checkGNUHashTable<ELFT>(Obj, GnuHashTable))
       this->reportUniqueWarning(std::move(E));
     else
       printGnuHashHistogram(*GnuHashTable);