[llvm-readobj] - Do not crash when GnuHashTable->symndx is greater than the dynamic symbols count.

[grimar]

Summary

Elf_GnuHash_Impl has the following method:

ArrayRef<Elf_Word> values(unsigned DynamicSymCount) const {
  return ArrayRef<Elf_Word>(buckets().end(), DynamicSymCount - symndx);
}

When DynamicSymCount is less than symndx we return an array with the huge broken size.
This patch fixes the issue ans adds an assert. This assert helped to fix an issue
in one of the test cases.

The printGnuHashHistogram method also needs to be fixed. I am planning to fix it
independently after we land the D81928, which will allow to reuse the code that validates
the GNU hash table before the dumping.

Depends on: D81928

[grimar]

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

258

I had to fix this test case because this place was wrong and the code adds an assert:
assert(DynamicSymCount >= symndx); which is triggered if this test case is not fixed.

We still want to make this assert never happen. I've mentioned in the description that it
is related to printGnuHashHistogram and probably should be fixed independently.

[MaskRay]

Looks great!

[jhenderson]

LGTM, with nit.

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

214–215

whilst you're modifying this line: "described with" -> "described by"

llvm/include/llvm/Object/ELFTypes.h

struct Elf_GnuHash_Impl {
   }
 
   ArrayRef<Elf_Word> buckets() const {
     return ArrayRef<Elf_Word>(
         reinterpret_cast<const Elf_Word *>(filter().end()), nbuckets);
   }
 
   ArrayRef<Elf_Word> values(unsigned DynamicSymCount) const {
+    assert(DynamicSymCount >= symndx);
     return ArrayRef<Elf_Word>(buckets().end(), DynamicSymCount - symndx);
   }
 };
 
 // Compressed section headers.
 // http://www.sco.com/developers/gabi/latest/ch4.sheader.html#compression_header
 template <endianness TargetEndianness>
 struct Elf_Chdr_Impl<ELFType<TargetEndianness, false>> {

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

 ## Such sections normally have a single zero entry in the bloom
 ## filter, a single zero entry in the hash bucket and no values.
 ##
 ## The index of the first symbol in the dynamic symbol table
 ## included in the hash table can be set to the number of dynamic symbols,
 ## which is one larger than the index of the last dynamic symbol.
 ## For empty tables however, this value is unimportant and can be ignored.
 
-# RUN: yaml2obj --docnum=5 %s -o %t.empty
-# RUN: llvm-readobj --gnu-hash-table %t.empty 2>&1 \
-# RUN:   | FileCheck %s -DFILE=%t.empty --check-prefix=EMPTY --implicit-check-not="warning:"
-# RUN: llvm-readelf --gnu-hash-table %t.empty 2>&1 \
-# RUN:   | FileCheck %s -DFILE=%t.empty --check-prefix=EMPTY --implicit-check-not="warning:"
+## Case A: set the index of the first symbol in the dynamic symbol table to
+##         the number of dynamic symbols.
+# RUN: yaml2obj --docnum=5 -DSYMNDX=0x1 %s -o %t.empty.1
+# RUN: llvm-readobj --gnu-hash-table %t.empty.1 2>&1 \
+# RUN:   | FileCheck %s -DFILE=%t.empty.1 -DSYMNDX=1 --check-prefix=EMPTY --implicit-check-not="warning:"
+# RUN: llvm-readelf --gnu-hash-table %t.empty.1 2>&1 \
+# RUN:   | FileCheck %s -DFILE=%t.empty.1 -DSYMNDX=1 --check-prefix=EMPTY --implicit-check-not="warning:"
+
+## Case B: set the index of the first symbol in the dynamic symbol table to
+##         an arbitrary value that is larger than the number of dynamic symbols.
+# RUN: yaml2obj --docnum=5 -DSYMNDX=0x2 %s -o %t.empty.2
+# RUN: llvm-readobj --gnu-hash-table %t.empty.2 2>&1 \
+# RUN:   | FileCheck %s -DFILE=%t.empty.2 -DSYMNDX=2 --check-prefix=EMPTY --implicit-check-not="warning:"
+# RUN: llvm-readelf --gnu-hash-table %t.empty.2 2>&1 \
+# RUN:   | FileCheck %s -DFILE=%t.empty.2 -DSYMNDX=2 --check-prefix=EMPTY --implicit-check-not="warning:"
 
 # EMPTY:      GnuHashTable {
 # EMPTY-NEXT:   Num Buckets: 1
-# EMPTY-NEXT:   First Hashed Symbol Index: 1
+# EMPTY-NEXT:   First Hashed Symbol Index: [[SYMNDX]]
 # EMPTY-NEXT:   Num Mask Words: 1
 # EMPTY-NEXT:   Shift Count: 0
 # EMPTY-NEXT:   Bloom Filter: [0x0]
 # EMPTY-NEXT:   Buckets: [0]
 # EMPTY-NEXT:   Values: []
 # EMPTY-NEXT: }
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:
   - Name:  .gnu.hash
     Type:  SHT_GNU_HASH
     Flags: [ SHF_ALLOC ]
     Header:
-      SymNdx: 0x1
+      SymNdx: [[SYMNDX]]
       Shift2: 0x0
     BloomFilter: [ 0x0 ]
     HashBuckets: [ 0x0 ]
     HashValues:  [ ]
   - Name:  .dynamic
     Type:  SHT_DYNAMIC
     Flags: [ SHF_ALLOC ]
     Link:  0

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

ProgramHeaders:
   - Type: PT_LOAD
     Sections:
       - Section: .hash
       - Section: .dynamic
 
 ## Check we dump a histogram for the .gnu.hash table even when the .hash table is skipped.
 
 ## Case A: the .hash table has no data to build histogram and it is skipped.
-##         (NBUCKET == 0x1 is a no-op: it does not change the number of buckets described with the "Bucket" key).
-# RUN: yaml2obj --docnum=5 -DNBUCKET=0x1 %s -o %t5.o
+##         (NBUCKET == 0x2 is a no-op: it does not change the number of buckets described by the "Bucket" key).
+# RUN: yaml2obj --docnum=5 -DNBUCKET=0x2 %s -o %t5.o
 # RUN: llvm-readelf --elf-hash-histogram %t5.o 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=GNU-HASH --implicit-check-not="Histogram"
 
 ## Case B: the .hash table has a broken nbucket field. We report a warning
 ##         and skip dumping of the .hash table.
 # RUN: yaml2obj --docnum=5 -DNBUCKET=0xffffffff %s -o %t6.o
 # RUN: llvm-readelf --elf-hash-histogram %t6.o 2>&1 | \
 # RUN:   FileCheck %s -DFILE=%t6.o --check-prefixes=WARN,GNU-HASH
 
-# WARN:     warning: '[[FILE]]': the hash table at offset 0x78 goes past the end of the file (0x350), nbucket = 4294967295, nchain = 1
-# GNU-HASH: Histogram for `.gnu.hash' bucket list length (total of 1 buckets)
+# WARN:     warning: '[[FILE]]': the hash table at offset 0x78 goes past the end of the file (0x358), nbucket = 4294967295, nchain = 2
+# GNU-HASH: Histogram for `.gnu.hash' bucket list length (total of 3 buckets)
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:
   - Name:    .hash
     Type:    SHT_HASH
     Flags:   [ SHF_ALLOC ]
     Bucket:  [ 0 ]
     NBucket: [[NBUCKET]]
-    Chain:   [ 0 ]
+    Chain:   [ 0, 0 ]
   - Name:  .gnu.hash
     Type:  SHT_GNU_HASH
     Flags: [ SHF_ALLOC ]
     Header:
       SymNdx: 0x1
       Shift2: 0x0
     BloomFilter: [ 0x0 ]
     HashBuckets: [ 0x00000001, 0x00000004, 0x00000000 ]
     HashValues:  [ 0x0B887388 ]
   - Name:  .dynamic
     Type:  SHT_DYNAMIC
     Flags: [ SHF_WRITE, SHF_ALLOC ]
     Entries:
       - Tag:   DT_HASH
         Value: 0x0
       - Tag:   DT_GNU_HASH
-## sizeof(.hash) == 0x28.
-        Value: 0x28
+## sizeof(.hash) == 0x14.
+        Value: 0x14
       - Tag:   DT_NULL
         Value: 0x0
 DynamicSymbols:
   - Name: foo
 ProgramHeaders:
   - Type:  PT_LOAD
     Sections:
       - Section: .hash

llvm/tools/llvm-readobj/ELFDumper.cpp

return createError("unable to dump 'Values' for the SHT_GNU_HASH "
                        "section: the dynamic symbol table is empty");
 
   if (GnuHashTable->symndx < NumSyms)
     return GnuHashTable->values(NumSyms);
 
   // A normal empty GNU hash table section produced by linker might have
   // symndx set to the number of dynamic symbols + 1 (for the zero symbol)
   // and have dummy null values in the Bloom filter and in the buckets
-  // vector. It happens because the value of symndx is not important for
-  // dynamic loaders when the GNU hash table is empty. They just skip the
-  // whole object during symbol lookup. In such cases, the symndx value is
-  // irrelevant and we should not report a warning.
+  // vector (or no values at all). It happens because the value of symndx is not
+  // important for dynamic loaders when the GNU hash table is empty. They just
+  // skip the whole object during symbol lookup. In such cases, the symndx value
+  // is irrelevant and we should not report a warning.
   ArrayRef<typename ELFT::Word> Buckets = GnuHashTable->buckets();
   if (!llvm::all_of(Buckets, [](typename ELFT::Word V) { return V == 0; }))
     return createError("the first hashed symbol index (" +
                        Twine(GnuHashTable->symndx) +
                        ") is larger than the number of dynamic symbols (" +
                        Twine(NumSyms) + ")");
-
-  return GnuHashTable->values(NumSyms);
+  // There is no way to represent an array of (dynamic symbols count - symndx)
+  // length.
+  return ArrayRef<typename ELFT::Word>();
 }
 
 template <typename ELFT>
 void ELFDumper<ELFT>::printGnuHashTable(const object::ObjectFile *Obj) {
   DictScope D(W, "GnuHashTable");
   if (!GnuHashTable)
     return;