[llvm] 25e437e - [llvm-readobj/elf] - Ignore the hash table when on EM_S390/EM_ALPHA platforms.

Georgii Rymar via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 12 02:13:41 PDT 2020


Author: Georgii Rymar
Date: 2020-10-12T12:13:01+03:00
New Revision: 25e437ec1e5b96028a319d3f2aca4129cdd3d85d

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

LOG: [llvm-readobj/elf] - Ignore the hash table when on EM_S390/EM_ALPHA platforms.

Specification for `SHT_HASH` table says (https://refspecs.linuxbase.org/elf/gabi4+/ch5.dynamic.html#hash)
that it contains `Elf32_Word` entries for both `32/64` bit objects.

But there is a problem with `EM_S390` and `ELF::EM_ALPHA` platforms: they use 8-bytes entries.
(see the issue reported: https://bugs.llvm.org/show_bug.cgi?id=47681).

Currently we might infer the size of the dynamic symbols table from hash table,
but because of the issue mentioned, the calculation is wrong. And also we don't dump the hash table
properly.

I am not sure if we want to support 8-bytes entries as they violates specification and also the
`.hash` table is kind of deprecated by itself (the `.gnu.hash` table is used nowadays).
So, the solution this patch suggests is to ban using of the hash table on `EM_S390/EM_ALPHA` platforms.

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

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 b8d44e3cdf71..cdb7581b1174 100644
--- a/llvm/test/tools/llvm-readobj/ELF/hash-table.test
+++ b/llvm/test/tools/llvm-readobj/ELF/hash-table.test
@@ -18,16 +18,17 @@
 
 --- !ELF
 FileHeader:
-  Class:   ELFCLASS[[BITS]]
+  Class:   ELFCLASS[[BITS=64]]
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: [[MACHINE]]
 Sections:
-  - Name:   .hash
-    Type:   SHT_HASH
-    Flags:  [ SHF_ALLOC ]
-    Bucket: [ 1, 2 ]
-    Chain:  [ 3, 4, 5 ]
+  - Name:    .hash
+    Type:    SHT_HASH
+    Flags:   [ SHF_ALLOC ]
+    Bucket:  [ 1, 2 ]
+    Chain:   [ 3, 4, 5 ]
+    EntSize: [[ENTSIZE=4]]
   - Name:  .dynamic
     Type:  SHT_DYNAMIC
     Flags: [ SHF_ALLOC ]
@@ -42,6 +43,43 @@ ProgramHeaders:
       - Section: .hash
       - Section: .dynamic
 
+## Document that we ignore the sh_entsize value when dumping the hash section.
+## Implementation assumes that the size of entries is 4, matching the ELF specification.
+
+# RUN: yaml2obj --docnum=1 -DENTSIZE=8 -DBITS=64 -DMACHINE=EM_X86_64 %s -o %t.x64.ent8
+# RUN: yaml2obj --docnum=1 -DENTSIZE=8 -DBITS=32 -DMACHINE=EM_386 %s -o %t.x32.ent8
+
+# RUN: llvm-readobj --hash-table %t.x64.ent8 | FileCheck %s --check-prefix=HASH
+# RUN: llvm-readelf --hash-table %t.x64.ent8 | FileCheck %s --check-prefix=HASH
+# RUN: llvm-readobj --hash-table %t.x32.ent8 | FileCheck %s --check-prefix=HASH
+# RUN: llvm-readelf --hash-table %t.x32.ent8 | FileCheck %s --check-prefix=HASH
+
+## We don't support dumping hash tables on EM_S390 and EM_ALPHA platforms and report a warning.
+## On these platforms the size of entries is 8, which violates the ELF specification, which says that the size
+## of hash entries in the hash table must be 4.
+
+# RUN: yaml2obj --docnum=1 -DMACHINE=EM_S390 %s -o %t.s390
+# RUN: llvm-readobj --hash-table %t.s390 2>&1 | FileCheck %s -DFILE=%t.s390 --check-prefixes=WARN-HASH -DNAME="IBM S/390"
+# RUN: llvm-readelf --hash-table %t.s390 2>&1 | FileCheck %s -DFILE=%t.s390 --check-prefixes=WARN-HASH -DNAME="IBM S/390"
+
+# WARN-HASH:      HashTable {
+# WARN-HASH-NEXT: warning: '[[FILE]]': the hash table at 0x78 is not supported: it contains non-standard 8 byte entries on [[NAME]] platform
+# WARN-HASH-NEXT: }
+
+# RUN: yaml2obj --docnum=1 -DMACHINE=EM_ALPHA %s -o %t.alpha
+# RUN: llvm-readobj --hash-table %t.alpha 2>&1 | FileCheck %s -DFILE=%t.alpha --check-prefixes=WARN-HASH -DNAME="EM_ALPHA"
+# RUN: llvm-readelf --hash-table %t.alpha 2>&1 | FileCheck %s -DFILE=%t.alpha --check-prefixes=WARN-HASH -DNAME="EM_ALPHA"
+
+## We don't report warnings about the unsupported hash table on EM_S390 and EM_ALPHA platforms
+## when --hash-table is not requested.
+
+# RUN: llvm-readobj %t.s390 2>&1 | FileCheck %s -DFILE=%t.s390 --implicit-check-not="warning:"  --check-prefix=NOWARN
+# RUN: llvm-readelf %t.s390 2>&1 | FileCheck %s -DFILE=%t.s390 --implicit-check-not="warning:"  --check-prefix=NOWARN
+# RUN: llvm-readobj %t.alpha 2>&1 | FileCheck %s -DFILE=%t.alpha --implicit-check-not="warning:" --check-prefix=NOWARN
+# RUN: llvm-readelf %t.alpha 2>&1 | FileCheck %s -DFILE=%t.alpha --implicit-check-not="warning:" --check-prefix=NOWARN
+
+# NOWARN: warning: '[[FILE]]': string table was not found
+
 ## Check we can dump the SHT_HASH section even when an object
 ## does not have the section header table.
 

diff  --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 88437000a88f..e7ee0793b903 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -328,6 +328,15 @@ template <typename ELFT> class ELFDumper : public ObjDumper {
   std::string describe(const Elf_Shdr &Sec) const;
 
 public:
+  unsigned getHashTableEntSize() const {
+    // EM_S390 and ELF::EM_ALPHA platforms use 8-bytes entries in SHT_HASH
+    // sections. This violates the ELF specification.
+    if (Obj.getHeader().e_machine == ELF::EM_S390 ||
+        Obj.getHeader().e_machine == ELF::EM_ALPHA)
+      return 8;
+    return 4;
+  }
+
   Elf_Dyn_Range dynamic_table() const {
     // A valid .dynamic section contains an array of entries terminated
     // with a DT_NULL entry. However, sometimes the section content may
@@ -2176,6 +2185,7 @@ void ELFDumper<ELFT>::parseDynamicTable() {
 
   SOName = getDynamicString(SONameOffset);
 
+  const bool IsHashTableSupported = getHashTableEntSize() == 4;
   if (DynSymRegion) {
     // Often we find the information about the dynamic symbol table
     // location in the SHT_DYNSYM section header. However, the value in
@@ -2191,7 +2201,7 @@ void ELFDumper<ELFT>::parseDynamicTable() {
     // equal nchain". Check to see if the DT_HASH hash table nchain value
     // conflicts with the number of symbols in the dynamic symbol table
     // according to the section header.
-    if (HashTable) {
+    if (HashTable && IsHashTableSupported) {
       if (DynSymRegion->EntSize == 0)
         reportUniqueWarning(
             createError("SHT_DYNSYM section has sh_entsize == 0"));
@@ -2219,7 +2229,7 @@ void ELFDumper<ELFT>::parseDynamicTable() {
 
   // Derive the dynamic symbol table size from the DT_HASH hash table, if
   // present.
-  if (HashTable && DynSymRegion) {
+  if (HashTable && IsHashTableSupported && DynSymRegion) {
     const uint64_t FileSize = Obj.getBufSize();
     const uint64_t DerivedSize =
         (uint64_t)HashTable->nchain * DynSymRegion->EntSize;
@@ -2653,29 +2663,43 @@ template <class ELFT> void ELFDumper<ELFT>::printNeededLibraries() {
 }
 
 template <class ELFT>
-static Error checkHashTable(const ELFFile<ELFT> &Obj,
+static Error checkHashTable(const ELFDumper<ELFT> &Dumper,
                             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) +
+  const ELFFile<ELFT> &Obj = *Dumper.getElfObject().getELFFile();
+  const uint64_t SecOffset = (const uint8_t *)H - Obj.base();
+  if (Dumper.getHashTableEntSize() == 8) {
+    auto It = llvm::find_if(ElfMachineType, [&](const EnumEntry<unsigned> &E) {
+      return E.Value == Obj.getHeader().e_machine;
+    });
+    if (IsHeaderValid)
+      *IsHeaderValid = false;
+    return createError("the hash table at 0x" + Twine::utohexstr(SecOffset) +
+                       " is not supported: it contains non-standard 8 "
+                       "byte entries on " +
+                       It->AltName + " platform");
+  }
+
+  auto MakeError = [&](const Twine &Msg = "") {
+    return createError("the hash table at offset 0x" +
+                       Twine::utohexstr(SecOffset) +
                        " 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 MakeError(SecOffset);
+    return MakeError();
 
   if (Obj.getBufSize() - SecOffset - HeaderSize <
       ((uint64_t)H->nbucket + H->nchain) * sizeof(typename ELFT::Word))
-    return MakeError(SecOffset, ", nbucket = " + Twine(H->nbucket) +
-                                    ", nchain = " + Twine(H->nchain));
+    return MakeError(", nbucket = " + Twine(H->nbucket) +
+                     ", nchain = " + Twine(H->nchain));
   return Error::success();
 }
 
@@ -2706,7 +2730,7 @@ template <typename ELFT> void ELFDumper<ELFT>::printHashTable() {
     return;
 
   bool IsHeaderValid;
-  Error Err = checkHashTable(Obj, HashTable, &IsHeaderValid);
+  Error Err = checkHashTable(*this, HashTable, &IsHeaderValid);
   if (IsHeaderValid) {
     W.printNumber("Num Buckets", HashTable->nbucket);
     W.printNumber("Num Chains", HashTable->nchain);
@@ -4112,7 +4136,7 @@ void GNUStyle<ELFT>::printGnuHashTableSymbols(const Elf_GnuHash &GnuHash) {
 template <class ELFT> void GNUStyle<ELFT>::printHashSymbols() {
   if (const Elf_Hash *SysVHash = this->dumper().getHashTable()) {
     OS << "\n Symbol table of .hash for image:\n";
-    if (Error E = checkHashTable<ELFT>(this->Obj, SysVHash))
+    if (Error E = checkHashTable<ELFT>(this->dumper(), SysVHash))
       this->reportUniqueWarning(std::move(E));
     else
       printHashTableSymbols(*SysVHash);
@@ -4742,7 +4766,7 @@ void GNUStyle<ELFT>::printGnuHashHistogram(const Elf_GnuHash &GnuHashTable) {
 template <class ELFT> void GNUStyle<ELFT>::printHashHistograms() {
   // Print histogram for the .hash section.
   if (const Elf_Hash *HashTable = this->dumper().getHashTable()) {
-    if (Error E = checkHashTable<ELFT>(this->Obj, HashTable))
+    if (Error E = checkHashTable<ELFT>(this->dumper(), HashTable))
       this->reportUniqueWarning(std::move(E));
     else
       printHashHistogram(*HashTable);


        


More information about the llvm-commits mailing list