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

[Georgii Rymar via llvm-commits]

Author: Georgii Rymar
Date: 2020-06-01T12:36:23+03:00
New Revision: e75efcc3c1af068a145ea83deb0435b195a44162

URL: https://github.com/llvm/llvm-project/commit/e75efcc3c1af068a145ea83deb0435b195a44162
DIFF: https://github.com/llvm/llvm-project/commit/e75efcc3c1af068a145ea83deb0435b195a44162.diff

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

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.

Differential revision: https://reviews.llvm.org/D80373

Added: 
    

Modified: 
    llvm/test/tools/llvm-readobj/ELF/hash-table.test
    llvm/tools/llvm-readobj/ELFDumper.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-readobj/ELF/hash-table.test b/llvm/test/tools/llvm-readobj/ELF/hash-table.test
index a1018a9daefe..b247c597eeb1 100644
--- a/llvm/test/tools/llvm-readobj/ELF/hash-table.test
+++ b/llvm/test/tools/llvm-readobj/ELF/hash-table.test
@@ -157,6 +157,8 @@ ProgramHeaders:
 # 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: }
 
@@ -188,6 +190,8 @@ ProgramHeaders:
 
 # ERR3:      warning: '[[FILE]]': hash table nchain (94) 
diff ers 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: }
 
@@ -219,3 +223,14 @@ ProgramHeaders:
     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) 
diff ers 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: }

diff  --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index dc080c8dd49c..4b87067ce557 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -2642,28 +2642,30 @@ template <class ELFT> void ELFDumper<ELFT>::printNeededLibraries() {
 }
 
 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) +
+                       " goes past the end of the file (0x" +
+                       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) +
+                                    ", nchain = " + Twine(H->nchain));
+  return Error::success();
 }
 
 template <class ELFT>
@@ -2690,11 +2692,21 @@ static Error checkGNUHashTable(const ELFFile<ELFT> *Obj,
 
 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.printNumber("Num Buckets", HashTable->nbucket);
-  W.printNumber("Num Chains", HashTable->nchain);
+  }
+
   W.printList("Buckets", HashTable->buckets());
   W.printList("Chains", HashTable->chains());
 }
@@ -4026,7 +4038,9 @@ template <class ELFT> void GNUStyle<ELFT>::printHashSymbols(const ELFO *Obj) {
 
   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);
   }
 
@@ -4689,7 +4703,9 @@ 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.