[llvm-readobj] Derive dynamic symtab size from hash table

[jhenderson]

Summary

If the section headers have been removed by a tool such as llvm-objcopy or llvm-strip, previously llvm-readobj/llvm-readelf would not dump the dynamic symbols when --dyn-symbols was specified. However, the nchain value of the DT_HASH data specifies the number of dynamic symbols, so if it is present, we can use that. This patch implements this behaviour.

Fixes https://bugs.llvm.org/show_bug.cgi?id=45089.

[jhenderson]

Fix unchecked Expected.

[grimar]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

455 ↗

(On Diff #251029)

dervied -> derived

462 ↗

(On Diff #251029)

Why there is no -DBITS=32 case like above?

572 ↗

(On Diff #251029)

Could you expand the comment to mention we have 4 dynamic symbols (with null) in the object,
but we set the size of .dynsym so that it says we have 3 (it is not really obvious I think).

575 ↗

(On Diff #251029)

LLVM-HASHB -> LLVM-HASHB2? (a bit more consistent with LLVM-HASHB4)
HASHB-WARN -> WARN? (shorter)

581 ↗

(On Diff #251029)

Should we test -DCHAIN="[1, 2, 3]" to verify there is no warning?

715 ↗

(On Diff #251029)

--docnum=15

Should we move all the test you've added to a new test case? e.g. dyn-symbols-no-section-table.test?

725 ↗

(On Diff #251029)

Perhaps you can reuse the --docnum-13? e.g. if you reorder tags:

Entries:
  - Tag:   DT_STRTAB
    Value: 0x800
  - Tag:   DT_STRSZ
    Value: 9
  - Tag:   [[TAG1]]
    Value: [[ADDR1]]
  - Tag:   [[TAG2]]
    Value: [[ADDR2]]
  - Tag:   DT_NULL
    Value: 0

You should probably be able to -DTAG1=DT_SYMTAB -DTAG2=DT_NULL instead of -DTAG1=DT_SYMTAB -DTAG2=DT_HASH`

llvm/tools/llvm-readobj/ELFDumper.cpp

2064

I'd rename somehow to remove Temp prefix.

2177

Should we just use reportUniqueWarning everywhere for the new code, even if it is not really needed?
That might help to avoid copy-paste issues in future probably.

2182

"conflicts with the number of symbols ... " I think, not with the size of the dynamic symbol table.

I'd also add something like "In according to the ELF specification, the number of symbol table entries should equal nchain"
because I think it was never said in comments explicitly.

2208

You do not need curly bracers around a single code line.

3726

Looks like an independent improvement?

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

5925

Perhaps, the following piece looks slightly better.

StringRef Name= "<?>";
if (!this->dumper()->getElfObject()->sections().empty()) {
  if (Expected<StringRef> SectionName =
      this->dumper()->getSymbolSectionName(Symbol, *SectionIndex))
  Name = *SectionName;
else
  this->reportUniqueWarning(SectionName.takeError());
}

W.printHex("Section", Name, *SectionIndex);

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

462 ↗

(On Diff #251029)

I didn't think it was necessary. We've confirmed with the first 32/64 bit case that we can read the nchain value. After that, there's no difference in behaviour beyond what is already tested in the generic test cases.

575 ↗

(On Diff #251029)

I'm reluctant to do either of these.

LLVM-HASHB is used by both the 2 and 4 case. Having the 4 case use one called "HASHB2" would be potentially confusing.

HASHB-WARN gives context to this warning check, and prevents confusion if somebody wanted to add a check for a different warning in this test.

715 ↗

(On Diff #251029)

I originally considered that, but the b) cases have a section table, so it didn't make sense to call it that. I guess dyn-symbols-size-from-hash.test might work?

725 ↗

(On Diff #251029)

Good idea, thanks.

llvm/tools/llvm-readobj/ELFDumper.cpp

2177

This warning has just been moved from above. I'm happy to change it as I move it though, if you still want?

3726

The "Symbol table for image" part is almost dead code prior to my patch. This function is only called in one place, and there the only time this can be hit is if either the SHT_SYMTAB/SHT_DYNSYM section name is empty. If you attempted to dump the dynamic symbols for a stripped object prior to my change, the early out in the calling function stops this code being executed.

If you still want me to, I can go ahead and move it into a separate patch, but I'm not sure there's much benefit to it (I'd either have to write a test for a symbol table with no name, or modify the new test cases I'm adding in the next patch).

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

572 ↗

(On Diff #251029)

I've put the comment by the YAML, which is what describes the section header.

575 ↗

(On Diff #251029)

The WARN rename makes sense after splitting out the new tests.

I've redone the LLVM-HASHB case, since I've added the third test case for this situation now.

llvm/tools/llvm-readobj/ELFDumper.cpp

5925

This won't work because getSymbolSectionName also handles the special section indexes (SHN_UNDEF, SHN_ABS etc).

[grimar]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

462 ↗

(On Diff #251029)

OK. Perhaps mention this in comment for other readers?

575 ↗

(On Diff #251029)

OK

715 ↗

(On Diff #251029)

Sound good.

llvm/tools/llvm-readobj/ELFDumper.cpp

2177

I think it is OK to change while you are here anyways.

3726

Lets keep it.
I think I slightly misread this code, I've remembered this piece as the no-op cleanup, but today I see it is not (sorry).

[jhenderson]

Address review comments.

[MaskRay]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols-size-from-hash-table.test

314

Not aligned?

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

444 ↗

(On Diff #251045)

--implicit-check-not=warning:

(It is very unlikely that %t10a1-64 may contain warning:)

462 ↗

(On Diff #251029)

Why there is no -DBITS=32 case like above?

Already covered. No need to add another -DBITS=32 case...

llvm/tools/llvm-readobj/ELFDumper.cpp

2175

The two if can be guarded by an outer if (DynSymRegion) { ... }

We may add a DT_GNU_HASH check in the future.

[jhenderson]

Address review comments from @MaskRay.

[jhenderson]

Add colons to --implicit-check-not=warning (missed because they didn't appear in the diff, as they were in the old test).

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols-size-from-hash-table.test

314

Thanks: copy and paste error.

[MaskRay]

from hash table

Using DT_HASH may resolve ambiguity here, because there at least 2 flavors of hash tables (I used "at least" because GNU OSABI has DT_MIPS_XHASH. I started a thread about dropping it. dalias thinks this is doable https://sourceware.org/pipermail/binutils/2020-January/109362.html I hope we won't add it.)

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

Sections:
       - Section: .dynamic
   - Type: PT_GNU_EH_FRAME
     Sections:
       - Section: .eh_frame_hdr
   - Type: PT_NOTE
     Sections:
       - Section: .note.gnu.build-id
 Symbols: []
-DynamicSymbols: []
+DynamicSymbols:
+  - Name: foo

llvm/test/tools/llvm-readobj/ELF/dyn-symbols-size-from-hash-table.test

+## This test shows how llvm-readobj uses the hash table section to derive the
+## size of a dynamic symbol table. This allows dumping of the dynamic symbol
+## table in the event of an object without section headers.
+
+## Case 1a) Table size is derived from hash table, with DT_SYMTAB before DT_HASH.
+# RUN: yaml2obj --docnum=1 %s -o %t1a-64 -DBITS=64 \
+# RUN:          -DTAG1=DT_SYMTAB -DTAG2=DT_HASH -DVAL1=0x400 -DVAL2=0x600
+# RUN: llvm-strip --strip-sections %t1a-64
+# RUN: llvm-readobj --dyn-symbols %t1a-64 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=LLVM1,STRIP --implicit-check-not=warning
+# RUN: llvm-readelf --dyn-symbols %t1a-64 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=GNU1,GNU1-STRIP --implicit-check-not=warning
+# RUN: yaml2obj --docnum=1 %s -o %t1a-32 -DBITS=32 \
+# RUN:          -DTAG1=DT_SYMTAB -DTAG2=DT_HASH -DVAL1=0x400 -DVAL2=0x600
+# RUN: llvm-strip --strip-sections %t1a-32
+# RUN: llvm-readobj --dyn-symbols %t1a-32 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=LLVM1,STRIP --implicit-check-not=warning
+# RUN: llvm-readelf --dyn-symbols %t1a-32 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=GNU1,GNU1-STRIP --implicit-check-not=warning
+
+## 1b) Table size is derived from hash table, with DT_HASH before DT_SYMTAB.
+## We don't bother testing 32 and 64-bit here. The above cases show that reading
+## the nchain value is correct for all formats, and other tests show the basic
+## printing behaviour.
+# RUN: yaml2obj --docnum=1 %s -o %t1b-64 -DBITS=64 \
+# RUN:          -DTAG1=DT_HASH -DTAG2=DT_SYMTAB -DVAL1=0x600 -DVAL2=0x400
+# RUN: llvm-strip --strip-sections %t1b-64
+# RUN: llvm-readobj --dyn-symbols %t1b-64 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=LLVM1,STRIP --implicit-check-not=warning
+# RUN: llvm-readelf --dyn-symbols %t1b-64 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=GNU1,GNU1-STRIP --implicit-check-not=warning
+
+# LLVM1:      DynamicSymbols [
+# LLVM1-NEXT:   Symbol {
+# LLVM1-NEXT:     Name:  (0)
+# LLVM1-NEXT:     Value: 0x0
+# LLVM1-NEXT:     Size: 0
+# LLVM1-NEXT:     Binding: Local (0x0)
+# LLVM1-NEXT:     Type: None (0x0)
+# LLVM1-NEXT:     Other: 0
+# LLVM1-NEXT:     Section: Undefined (0x0)
+# LLVM1-NEXT:   }
+# LLVM1-NEXT:   Symbol {
+# LLVM1-NEXT:     Name: foo (5)
+# LLVM1-NEXT:     Value: 0x100
+# LLVM1-NEXT:     Size: 0
+# LLVM1-NEXT:     Binding: Local (0x0)
+# LLVM1-NEXT:     Type: Function (0x2)
+# LLVM1-NEXT:     Other: 0
+# STRIP-NEXT:     Section: <?> (0x1)
+# NOSTRIP-NEXT:   Section: .text (0x1)
+# LLVM1-NEXT:   }
+# LLVM1-NEXT:   Symbol {
+# LLVM1-NEXT:     Name: bar (1)
+# LLVM1-NEXT:     Value: 0x200
+# LLVM1-NEXT:     Size: 0
+# LLVM1-NEXT:     Binding: Local (0x0)
+# LLVM1-NEXT:     Type: Object (0x1)
+# LLVM1-NEXT:     Other: 0
+# STRIP-NEXT:     Section: <?> (0x2)
+# NOSTRIP-NEXT:   Section: .data (0x2)
+# LLVM1-NEXT:   }
+# LLVM1-NEXT: ]
+
+# GNU1-STRIP:   Symbol table for image contains 3 entries:
+# GNU1-NOSTRIP: Symbol table '.dynsym' contains 3 entries:
+# GNU1-NEXT:      Num:    Value        Size Type    Bind   Vis       Ndx Name
+# GNU1-NEXT:        0: {{0*}}00000000     0 NOTYPE  LOCAL  DEFAULT   UND
+# GNU1-NEXT:        1: {{0*}}00000100     0 FUNC    LOCAL  DEFAULT     1 foo
+# GNU1-NEXT:        2: {{0*}}00000200     0 OBJECT  LOCAL  DEFAULT     2 bar
+# GNU1-EMPTY:
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS[[BITS]]
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:         .text
+    Type:         SHT_PROGBITS
+  - Name:         .data
+    Type:         SHT_PROGBITS
+  - Name:         .dynsym
+    Type:         SHT_DYNSYM
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x400
+    AddressAlign: 0x400
+  - Name:         .hash
+    Type:         SHT_HASH
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x600
+    AddressAlign: 0x200
+    Bucket:       [ 1 ]
+    Chain:        [ 1, 2, 3 ]
+  - Name:         .dynstr
+    Type:         SHT_STRTAB
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x800
+    AddressAlign: 0x200
+  - Name:         .dynamic
+    Type:         SHT_DYNAMIC
+    Flags:        [ SHF_ALLOC ]
+    Address:      0xA00
+    AddressAlign: 0x200
+    Entries:
+      - Tag:   DT_STRTAB
+        Value: 0x800
+      - Tag:   DT_STRSZ
+        Value: 9
+      - Tag:   [[TAG1]]
+        Value: [[VAL1]]
+      - Tag:   [[TAG2]]
+        Value: [[VAL2]]
+      - Tag:   DT_NULL
+        Value: 0
+DynamicSymbols:
+  - Name:    foo
+    Type:    STT_FUNC
+    Section: .text
+    Value:   0x100
+  - Name:    bar
+    Type:    STT_OBJECT
+    Section: .data
+    Value:   0x200
+ProgramHeaders:
+  - Type:  PT_LOAD
+    VAddr: 0
+    Sections:
+      - Section: .text
+      - Section: .data
+  - Type:  PT_LOAD
+    VAddr: 0x400
+    Sections:
+      - Section: .dynsym
+      - Section: .hash
+      - Section: .dynstr
+      - Section: .dynamic
+  - Type:  PT_DYNAMIC
+    VAddr: 0xA00
+    Sections:
+      - Section: .dynamic
+
+## Case 2: Table size from DT_HASH does not match size from section header.
+# RUN: yaml2obj --docnum=2 %s -o %t2-smaller -DCHAIN="[1, 2]"
+# RUN: llvm-readobj --dyn-symbols %t2-smaller 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=LLVM2,WARN \
+# RUN:              --implicit-check-not=warning -DNCHAIN=2
+# RUN: llvm-readelf --dyn-symbols %t2-smaller 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=GNU2,WARN \
+# RUN:                --implicit-check-not=warning -DNCHAIN=2
+
+# RUN: yaml2obj --docnum=2 %s -o %t2-larger -DCHAIN="[1, 2, 3, 4]"
+# RUN: llvm-readobj --dyn-symbols %t2-larger 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=LLVM2,LLVM2-MORE,LLVM2-ALL,WARN \
+# RUN:                --implicit-check-not=warning -DNCHAIN=4
+# RUN: llvm-readelf --dyn-symbols %t2-larger 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=GNU2,GNU2-MORE,GNU2-ALL,WARN \
+# RUN:                --implicit-check-not=warning -DNCHAIN=4
+
+## Show there's no warning if the sizes match
+# RUN: yaml2obj --docnum=2 %s -o %t2-same -DCHAIN="[1, 2, 3]"
+# RUN: llvm-readobj --dyn-symbols %t2-same 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=LLVM2,LLVM2-MORE --implicit-check-not=warning
+# RUN: llvm-readelf --dyn-symbols %t2-same 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=GNU2,GNU2-MORE \
+# RUN:                --implicit-check-not=warning -DNCHAIN=3
+
+# WARN: warning: '{{.*}}2-{{.*}}': hash table nchain ([[NCHAIN]]) differs from symbol count derived from SHT_DYNSYM section header (3)
+
+# LLVM2:           DynamicSymbols [
+# LLVM2-NEXT:        Symbol {
+# LLVM2-NEXT:          Name:  (0)
+# LLVM2-NEXT:          Value: 0x0
+# LLVM2-NEXT:          Size: 0
+# LLVM2-NEXT:          Binding: Local (0x0)
+# LLVM2-NEXT:          Type: None (0x0)
+# LLVM2-NEXT:          Other: 0
+# LLVM2-NEXT:          Section: Undefined (0x0)
+# LLVM2-NEXT:        }
+# LLVM2-NEXT:        Symbol {
+# LLVM2-NEXT:          Name: foo (9)
+# LLVM2-NEXT:          Value: 0x100
+# LLVM2-NEXT:          Size: 0
+# LLVM2-NEXT:          Binding: Local (0x0)
+# LLVM2-NEXT:          Type: Function (0x2)
+# LLVM2-NEXT:          Other: 0
+# LLVM2-NEXT:          Section: .text (0x1)
+# LLVM2-NEXT:        }
+# LLVM2-MORE-NEXT:   Symbol {
+# LLVM2-MORE-NEXT:     Name: bar (5)
+# LLVM2-MORE-NEXT:     Value: 0x200
+# LLVM2-MORE-NEXT:     Size: 0
+# LLVM2-MORE-NEXT:     Binding: Local (0x0)
+# LLVM2-MORE-NEXT:     Type: Object (0x1)
+# LLVM2-MORE-NEXT:     Other: 0
+# LLVM2-MORE-NEXT:     Section: .data (0x2)
+# LLVM2-MORE-NEXT:   }
+# LLVM2-ALL-NEXT:    Symbol {
+# LLVM2-ALL-NEXT:      Name: baz (1)
+# LLVM2-ALL-NEXT:      Value: 0x300
+# LLVM2-ALL-NEXT:      Size: 0
+# LLVM2-ALL-NEXT:      Binding: Local (0x0)
+# LLVM2-ALL-NEXT:      Type: Object (0x1)
+# LLVM2-ALL-NEXT:      Other: 0
+# LLVM2-ALL-NEXT:      Section: .data (0x2)
+# LLVM2-ALL-NEXT:    }
+# LLVM2-NEXT:      ]
+
+# GNU2:           Symbol table '.dynsym' contains [[NCHAIN]] entries:
+# GNU2-NEXT:        Num:    Value        Size Type    Bind   Vis       Ndx Name
+# GNU2-NEXT:          0: {{0*}}00000000     0 NOTYPE  LOCAL  DEFAULT   UND
+# GNU2-NEXT:          1: {{0*}}00000100     0 FUNC    LOCAL  DEFAULT     1 foo
+# GNU2-MORE-NEXT:     2: {{0*}}00000200     0 OBJECT  LOCAL  DEFAULT     2 bar
+# GNU2-ALL-NEXT:      3: {{0*}}00000300     0 OBJECT  LOCAL  DEFAULT     2 baz
+# GNU2-EMPTY:
+
+## In this YAML, we define 4 dynamic symbols (including the null symbol), but
+## constrain the .dynsym section header to say there are only 3. This means that
+## when a size of 4 is derived from the hash table, we still have a valid symbol
+## to dump.
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:         .text
+    Type:         SHT_PROGBITS
+  - Name:         .data
+    Type:         SHT_PROGBITS
+  - Name:         .dynsym
+    Type:         SHT_DYNSYM
+    Flags:        [ SHF_ALLOC ]
+    ShSize:       0x48
+    Address:      0x400
+    AddressAlign: 0x400
+  - Name:         .hash
+    Type:         SHT_HASH
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x600
+    AddressAlign: 0x200
+    Bucket:       [ 1 ]
+    Chain:        [[CHAIN]]
+  - Name:         .dynstr
+    Type:         SHT_STRTAB
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x800
+    AddressAlign: 0x200
+  - Name:         .dynamic
+    Type:         SHT_DYNAMIC
+    Flags:        [ SHF_ALLOC ]
+    Address:      0xA00
+    AddressAlign: 0x200
+    Entries:
+      - Tag:   DT_SYMTAB
+        Value: 0x400
+      - Tag:   DT_HASH
+        Value: 0x600
+      - Tag:   DT_STRTAB
+        Value: 0x800
+      - Tag:   DT_STRSZ
+        Value: 13
+      - Tag:   DT_NULL
+        Value: 0
+DynamicSymbols:
+  - Name:    foo
+    Type:    STT_FUNC
+    Section: .text
+    Value:   0x100
+  - Name:    bar
+    Type:    STT_OBJECT
+    Section: .data
+    Value:   0x200
+  - Name:    baz
+    Type:    STT_OBJECT
+    Section: .data
+    Value:   0x300
+ProgramHeaders:
+  - Type:  PT_LOAD
+    VAddr: 0
+    Sections:
+      - Section: .text
+      - Section: .data
+  - Type:  PT_LOAD
+    VAddr: 0x400
+    Sections:
+      - Section: .dynsym
+      - Section: .hash
+      - Section: .dynstr
+      - Section: .dynamic
+  - Type:  PT_DYNAMIC
+    VAddr: 0xA00
+    Sections:
+      - Section: .dynamic
+
+## Case 3: DT_HASH is missing.
+## Show that no warning occurs if there are section headers.
+# RUN: yaml2obj --docnum=1 %s -o %t3 -DTAG1=DT_SYMTAB -DVAL1=0x400 -DTAG2=DT_NULL -DVAL2=0 -DBITS=64
+# RUN: llvm-readobj --dyn-symbols %t3 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=LLVM1,NOSTRIP --implicit-check-not=warning
+# RUN: llvm-readelf --dyn-symbols %t3 2>&1 | \
+# RUN:   FileCheck %s --check-prefixes=GNU1,GNU1-NOSTRIP --implicit-check-not=warning
+
+## Show that size is treated as zero, if no section headers are present.
+# RUN: llvm-strip --strip-sections %t3
+# RUN: llvm-readobj --dyn-symbols %t3 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=LLVM3 --implicit-check-not=warning
+# RUN: llvm-readelf --dyn-symbols %t3 2>&1 | \
+# RUN:   FileCheck %s --implicit-check-not={{.}} --allow-empty
+
+# LLVM3: DynamicSymbols [
+# LLVM3: ]

llvm/tools/llvm-readobj/ELFDumper.cpp

Expected<StringRef> getSymbolVersion(const Elf_Sym *symb,
                                        bool &IsDefault) const;
   Error LoadVersionMap() const;
 
   const object::ELFObjectFile<ELFT> *ObjF;
   DynRegionInfo DynRelRegion;
   DynRegionInfo DynRelaRegion;
   DynRegionInfo DynRelrRegion;
   DynRegionInfo DynPLTRelRegion;
-  DynRegionInfo DynSymRegion;
+  Optional<DynRegionInfo> DynSymRegion;
   DynRegionInfo DynamicTable;
   StringRef DynamicStringTable;
   std::string SOName = "<Not found>";
   const Elf_Hash *HashTable = nullptr;
   const Elf_GnuHash *GnuHashTable = nullptr;
   const Elf_Shdr *DotSymtabSec = nullptr;
   const Elf_Shdr *DotCGProfileSec = nullptr;
   const Elf_Shdr *DotAddrsigSec = nullptr;
Elf_Dyn_Range dynamic_table() const {
     while (Size < Table.size())
       if (Table[Size++].getTag() == DT_NULL)
         break;
 
     return Table.slice(0, Size);
   }
 
   Elf_Sym_Range dynamic_symbols() const {
-    return DynSymRegion.getAsArrayRef<Elf_Sym>();
+    if (!DynSymRegion)
+      return Elf_Sym_Range();
+    return DynSymRegion->getAsArrayRef<Elf_Sym>();
   }
 
   Elf_Rel_Range dyn_rels() const;
   Elf_Rela_Range dyn_relas() const;
   Elf_Relr_Range dyn_relrs() const;
   std::string getFullSymbolName(const Elf_Sym *Symbol, StringRef StrTable,
                                 bool IsDynamic) const;
   Expected<unsigned> getSymbolSectionIndex(const Elf_Sym *Symbol,
void ELFDumper<ELFT>::printSymbolsHelper(bool IsDynamic) const {
   StringRef StrTable, SymtabName;
   size_t Entries = 0;
   Elf_Sym_Range Syms(nullptr, nullptr);
   const ELFFile<ELFT> *Obj = ObjF->getELFFile();
   if (IsDynamic) {
     StrTable = DynamicStringTable;
     Syms = dynamic_symbols();
     SymtabName = DynSymtabName;
-    if (DynSymRegion.Addr)
-      Entries = DynSymRegion.Size / DynSymRegion.EntSize;
+    if (DynSymRegion)
+      Entries = DynSymRegion->Size / DynSymRegion->EntSize;
   } else {
     if (!DotSymtabSec)
       return;
     StrTable = unwrapOrError(ObjF->getFileName(),
                              Obj->getStringTableForSymtab(*DotSymtabSec));
     Syms = unwrapOrError(ObjF->getFileName(), Obj->symbols(DotSymtabSec));
     SymtabName =
         unwrapOrError(ObjF->getFileName(), Obj->getSectionName(DotSymtabSec));
std::error_code createELFDumper(const object::ObjectFile *Obj,
 
   return readobj_error::unsupported_obj_file_format;
 }
 
 } // end namespace llvm
 
 template <class ELFT> Error ELFDumper<ELFT>::LoadVersionMap() const {
   // If there is no dynamic symtab or version table, there is nothing to do.
-  if (!DynSymRegion.Addr || !SymbolVersionSection)
+  if (!DynSymRegion || !SymbolVersionSection)
     return Error::success();
 
   // Has the VersionMap already been loaded?
   if (!VersionMap.empty())
     return Error::success();
 
   // The first two version indexes are reserved.
   // Index 0 is LOCAL, index 1 is GLOBAL.
Expected<StringRef> ELFDumper<ELFT>::getSymbolVersion(const Elf_Sym *Sym,
                                                       bool &IsDefault) const {
   // This is a dynamic symbol. Look in the GNU symbol version table.
   if (!SymbolVersionSection) {
     // No version table.
     IsDefault = false;
     return "";
   }
 
+  assert(DynSymRegion && "DynSymRegion has not been initialised");
   // Determine the position in the symbol table of this entry.
   size_t EntryIndex = (reinterpret_cast<uintptr_t>(Sym) -
-                        reinterpret_cast<uintptr_t>(DynSymRegion.Addr)) /
-                       sizeof(Elf_Sym);
+                       reinterpret_cast<uintptr_t>(DynSymRegion->Addr)) /
 
   // Get the corresponding version index entry.
   const Elf_Versym *Versym = unwrapOrError(
       ObjF->getFileName(), ObjF->getELFFile()->template getEntry<Elf_Versym>(
                                SymbolVersionSection, EntryIndex));
   return this->getSymbolVersionByIndex(Versym->vs_index, IsDefault);
 }
 
void ELFDumper<ELFT>::loadDynamicTable(const ELFFile<ELFT> *Obj) {
   parseDynamicTable(Obj);
 }
 
 template <typename ELFT>
 ELFDumper<ELFT>::ELFDumper(const object::ELFObjectFile<ELFT> *ObjF,
                            ScopedPrinter &Writer)
     : ObjDumper(Writer), ObjF(ObjF), DynRelRegion(ObjF->getFileName()),
       DynRelaRegion(ObjF->getFileName()), DynRelrRegion(ObjF->getFileName()),
-      DynPLTRelRegion(ObjF->getFileName()), DynSymRegion(ObjF->getFileName()),
-      DynamicTable(ObjF->getFileName()) {
+      DynPLTRelRegion(ObjF->getFileName()), DynamicTable(ObjF->getFileName()) {
+  if (opts::Output == opts::GNU)
+    ELFDumperStyle.reset(new GNUStyle<ELFT>(Writer, this));
+  else
+    ELFDumperStyle.reset(new LLVMStyle<ELFT>(Writer, this));
+
   const ELFFile<ELFT> *Obj = ObjF->getELFFile();
   typename ELFT::ShdrRange Sections =
       unwrapOrError(ObjF->getFileName(), Obj->sections());
   for (const Elf_Shdr &Sec : Sections) {
     switch (Sec.sh_type) {
     case ELF::SHT_SYMTAB:
       if (!DotSymtabSec)
         DotSymtabSec = &Sec;
       break;
     case ELF::SHT_DYNSYM:
-      if (!DynSymRegion.Size) {
+      if (!DynSymRegion) {
         DynSymRegion = createDRIFrom(&Sec);
-        DynSymRegion.Context =
+        DynSymRegion->Context =
             ("section with index " + Twine(&Sec - &Sections.front())).str();
         // This is only used (if Elf_Shdr present)for naming section in GNU
         // style
         DynSymtabName =
             unwrapOrError(ObjF->getFileName(), Obj->getSectionName(&Sec));
 
         if (Expected<StringRef> E = Obj->getStringTableForSymtab(Sec))
           DynamicStringTable = *E;
for (const Elf_Shdr &Sec : Sections) {
     case ELF::SHT_LLVM_ADDRSIG:
       if (!DotAddrsigSec)
         DotAddrsigSec = &Sec;
       break;
     }
   }
 
   loadDynamicTable(Obj);
-
-  if (opts::Output == opts::GNU)
-    ELFDumperStyle.reset(new GNUStyle<ELFT>(Writer, this));
-  else
-    ELFDumperStyle.reset(new LLVMStyle<ELFT>(Writer, this));
 }
 
 template <typename ELFT>
 void ELFDumper<ELFT>::parseDynamicTable(const ELFFile<ELFT> *Obj) {
   auto toMappedAddr = [&](uint64_t Tag, uint64_t VAddr) -> const uint8_t * {
     auto MappedAddrOrError = ObjF->getELFFile()->toMappedAddr(VAddr);
     if (!MappedAddrOrError) {
       Error Err =
           createError("Unable to parse DT_" + Obj->getDynamicTagAsString(Tag) +
                       ": " + llvm::toString(MappedAddrOrError.takeError()));
 
       reportWarning(std::move(Err), ObjF->getFileName());
       return nullptr;
     }
     return MappedAddrOrError.get();
   };
 
   uint64_t SONameOffset = 0;
   const char *StringTableBegin = nullptr;
   uint64_t StringTableSize = 0;
+  Optional<DynRegionInfo> DynSymFromTable;
   for (const Elf_Dyn &Dyn : dynamic_table()) {
     switch (Dyn.d_tag) {
     case ELF::DT_HASH:
       HashTable = reinterpret_cast<const Elf_Hash *>(
           toMappedAddr(Dyn.getTag(), Dyn.getPtr()));
       break;
     case ELF::DT_GNU_HASH:
       GnuHashTable = reinterpret_cast<const Elf_GnuHash *>(
           toMappedAddr(Dyn.getTag(), Dyn.getPtr()));
       break;
     case ELF::DT_STRTAB:
       StringTableBegin = reinterpret_cast<const char *>(
           toMappedAddr(Dyn.getTag(), Dyn.getPtr()));
       break;
     case ELF::DT_STRSZ:
       StringTableSize = Dyn.getVal();
       break;
     case ELF::DT_SYMTAB: {
-      // Often we find the information about the dynamic symbol table
-      // location in the SHT_DYNSYM section header. However, the value in
-      // DT_SYMTAB has priority, because it is used by dynamic loaders to
-      // locate .dynsym at runtime. The location we find in the section header
-      // and the location we find here should match. If we can't map the
-      // DT_SYMTAB value to an address (e.g. when there are no program headers), we
-      // ignore its value.
+      // If we can't map the DT_SYMTAB value to an address (e.g. when there are
+      // no program headers), we ignore its value.
       if (const uint8_t *VA = toMappedAddr(Dyn.getTag(), Dyn.getPtr())) {
-        // EntSize is non-zero if the dynamic symbol table has been found via a
-        // section header.
-        if (DynSymRegion.EntSize && VA != DynSymRegion.Addr)
-          reportWarning(
-              createError(
-                  "SHT_DYNSYM section header and DT_SYMTAB disagree about "
-                  "the location of the dynamic symbol table"),
-              ObjF->getFileName());
-
-        DynSymRegion.Addr = VA;
-        DynSymRegion.EntSize = sizeof(Elf_Sym);
-        DynSymRegion.EntSizePrintName = "";
+        DynSymFromTable.emplace(ObjF->getFileName());
+        DynSymFromTable->Addr = VA;
+        DynSymFromTable->EntSize = sizeof(Elf_Sym);
+        DynSymFromTable->EntSizePrintName = "";
       }
       break;
     }
     case ELF::DT_SYMENT: {
       uint64_t Val = Dyn.getVal();
       if (Val != sizeof(Elf_Sym))
         reportWarning(createError("DT_SYMENT value of 0x" +
                                   Twine::utohexstr(Val) +
case ELF::DT_PLTRELSZ:
       DynPLTRelRegion.Size = Dyn.getVal();
       DynPLTRelRegion.SizePrintName = "DT_PLTRELSZ value";
       break;
     }
   }
   if (StringTableBegin)
     DynamicStringTable = StringRef(StringTableBegin, StringTableSize);
   SOName = getDynamicString(SONameOffset);
+
+  if (DynSymRegion) {
+    // Often we find the information about the dynamic symbol table
+    // location in the SHT_DYNSYM section header. However, the value in
+    // DT_SYMTAB has priority, because it is used by dynamic loaders to
+    // locate .dynsym at runtime. The location we find in the section header
+    // and the location we find here should match.
+    if (DynSymFromTable && DynSymFromTable->Addr != DynSymRegion->Addr)
+      ELFDumperStyle->reportUniqueWarning(
+          createError("SHT_DYNSYM section header and DT_SYMTAB disagree about "
+                      "the location of the dynamic symbol table"));
+
+    // According to the ELF gABI: "The number of symbol table entries should
+    // equal nchain". Check to see if the hash table nchain value conflicts with
+    // the number of symbols in the dynamic symbol table according to the
+    // section header.
+    if (HashTable &&
+        HashTable->nchain != DynSymRegion->Size / DynSymRegion->EntSize)
+      ELFDumperStyle->reportUniqueWarning(createError(
+          "hash table nchain (" + Twine(HashTable->nchain) +
+          ") differs from symbol count derived from SHT_DYNSYM section "
+          "header (" +
+          Twine(DynSymRegion->Size / DynSymRegion->EntSize) + ")"));
+  }
+
+  // Delay the creation of the actual dynamic symbol table until now, so that
+  // checks can always be made against the section header-based properties,
+  // without worrying about tag order.
+  if (DynSymFromTable) {
+    if (!DynSymRegion) {
+      DynSymRegion = DynSymFromTable;
+    } else {
+      DynSymRegion->Addr = DynSymFromTable->Addr;
+      DynSymRegion->EntSize = DynSymFromTable->EntSize;
+      DynSymRegion->EntSizePrintName = DynSymFromTable->EntSizePrintName;
+    }
+  }
+
+  // Derive the dynamic symbol table size from the hash table, if present.
+  if (HashTable && DynSymRegion)
+    DynSymRegion->Size = HashTable->nchain * DynSymRegion->EntSize;
 }
 
 template <typename ELFT>
 typename ELFDumper<ELFT>::Elf_Rel_Range ELFDumper<ELFT>::dyn_rels() const {
   return DynRelRegion.getAsArrayRef<Elf_Rel>();
 }
 
 template <typename ELFT>
template <typename ELFT> void ELFDumper<ELFT>::printGnuHashTable() {
   W.printNumber("Shift Count", GnuHashTable->shift2);
 
   ArrayRef<typename ELFT::Off> BloomFilter = GnuHashTable->filter();
   W.printHexList("Bloom Filter", BloomFilter);
 
   ArrayRef<Elf_Word> Buckets = GnuHashTable->buckets();
   W.printList("Buckets", Buckets);
 
-  if (!DynSymRegion.Addr) {
+  if (!DynSymRegion) {
     reportWarning(createError("unable to dump 'Values' for the SHT_GNU_HASH "
                               "section: no dynamic symbol table found"),
                   ObjF->getFileName());
     return;
   }
 
   size_t NumSyms = dynamic_symbols().size();
   if (!NumSyms) {
void GNUStyle<ELFT>::printSectionHeaders(const ELFO *Obj) {
   printSectionDescription(OS, Obj->getHeader()->e_machine);
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printSymtabMessage(const ELFO *Obj, StringRef Name,
                                         size_t Entries,
                                         bool NonVisibilityBitsUsed) {
   if (!Name.empty())
-    OS << "\nSymbol table '" << Name << "' contains " << Entries
-       << " entries:\n";
+    OS << "\nSymbol table '" << Name << "'";
   else
-    OS << "\n Symbol table for image:\n";
+    OS << "\nSymbol table for image";
+  OS << " contains " << Entries << " entries:\n";
 
   if (ELFT::Is64Bits)
     OS << "   Num:    Value          Size Type    Bind   Vis";
   else
     OS << "   Num:    Value  Size Type    Bind   Vis";
 
   if (NonVisibilityBitsUsed)
     OS << "             ";
if (!SectionIndex) {
     this->reportUniqueWarning(SectionIndex.takeError());
     W.printHex("Section", "Reserved", SHN_XINDEX);
     return;
   }
 
   Expected<StringRef> SectionName =
       this->dumper()->getSymbolSectionName(Symbol, *SectionIndex);
   if (!SectionName) {
+    // Don't report an invalid section name if the section headers are missing.
+    // In such situations, all sections will be "invalid".
+    if (!this->dumper()->getElfObject()->sections().empty())
       this->reportUniqueWarning(SectionName.takeError());
+    else
+      consumeError(SectionName.takeError());
     W.printHex("Section", "<?>", *SectionIndex);
   } else {
     W.printHex("Section", *SectionName, *SectionIndex);
   }
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printSymbol(const ELFO *Obj, const Elf_Sym *Symbol,
                                   const Elf_Sym *First, StringRef StrTable,
                                   bool IsDynamic,
                                   bool /*NonVisibilityBitsUsed*/) {
   std::string FullSymbolName =
       this->dumper()->getFullSymbolName(Symbol, StrTable, IsDynamic);