[llvm-readobj/llvm-readelf] - Reimplement dumping of the SHT_GNU_verdef section.

[grimar]

Summary

Currently we have following issues:

1. We have 2 different implementations with a different behaviors for GNU/LLVM styles.
2. Errors are either not handled at all or we call report_fatal_error with not helpfull messages.
3. There is no test coverage even for those errors that are reported.

This patch reimplements parsing of the SHT_GNU_verdef section entries
in a single place, adds a few error messages and test coverage.

[MaskRay]

llvm/tools/llvm-readobj/ELFDumper.cpp

4035

ContentsOrErr->data()

4051

->

4060

Use for (unsigned I = 1; I <= Sec->sh_info; ++I) to avoid I + 1 below.

[MaskRay]

llvm/test/tools/llvm-readobj/elf-versioninfo.test

292

How about "report an error ... sh_link references a non-existent section"

llvm/tools/llvm-readobj/ELFDumper.cpp

4045

Probably just auto *

4074

Check alignment, probably D->vd_aux % alignof(VerdAux) == 0,
otherwise this may trigger a misaligned exception on some architectures.

[grimar]

• Addressed review comments.

llvm/tools/llvm-readobj/ELFDumper.cpp

4074

Good point!
There was a problem: for me alignof(Elf_Verdef) == 1, but I think the alignment for verdef should be 32
Seems it is because, for example, Elf_Word is a packed<uint32_t> which is packed_endian_specific_integral, which use
an array of chars for representing the storage buffer. And alignof(char) == 1.

I've used hardcoded constants instead. I've added 2 additional errors: one for "unaligned auxiliary entry" and another for
"unaligned version definition entry". Unfortunately I believe there is no way to test the first one with use of yaml2obj.

[MaskRay]

The logic looks good to me now. I've only got a few nits.

llvm/test/tools/llvm-readobj/elf-versioninfo.test

317

Probably ".. the sh_link field of a SHT_GNU_verdef section references a non-string table section"

I think the error/warning messages are a bit unnatural but I am not confident enough to suggest alternative messages. Thankfully we've got native speakers to suggest the wording :)

llvm/tools/llvm-readobj/ELFDumper.cpp

4046

We should probably only use FIXME for real code problems. Here we should "TODO".

Actually I think the alignment check should be done outside the function. Maybe we should check the alignments of both VerdefBuf and D->vd_aux.

4071

I guess this should be called "misaligned", not "unaligned", but I am not a native speaker and not very confident about this...

Not checking "misaligned auxiliary entry" should be fine.

5830

For consistency, shall we simply drop this if statement to print an empty list?

[jhenderson]

llvm/test/tools/llvm-readobj/elf-versioninfo.test

300

also should -> should also

is more natural

323

I'd change this to:

sh_link field of SHT_GNU_verdef section at index N is invalid: the section at index 0 is not a string table: expected SHT_STRTAB but got SHT_NULL

338

This comment doesn't line up with the warning we see below. I'd expect a similar warning to the previous test case.

344

How about "cannot read content of SHT_GNU_verdef section: ..."

I'd ideally put the [index 1] before the colon, i.e. "... of SHT_GNU_verdef section with index 1: the section has a ...", but I'm not sure if that risks losing useful context elsewhere.

353–357

Nit: this lot all need an extra space of padding to line up with ShOffset.

369

invalid SHT_GNU_verdef section with index N: ...

400

invalid SHT_GNU_verdef section with index N: version definition 1 refers ...

425

overruns -> overrun
the string table linked -> the linked string table

Perhaps worth checking that no warning is printed?

475

invalid SHT_GNU_verdef section with index N: found a misaligned definition entry at offset X

llvm/tools/llvm-readobj/ELFDumper.cpp

523–524

I'm not sure getVersionDefinitions belongs in the DumpStyle class, as it isn't a property of the style. I think it should be moved to the ElfDumper class, along with the other get* functions that are common to both dumping styles.

4025

Maybe since we now have it, use reportUniqueWarning?

4046

Is there a reason we can't just test this as part of this change?

4071

"misaligned" sounds good to me and matches the LoadVersionNeeds function.

5818

reportUniqueWarning?

5833

Do we have any tests for multiple predecessors?

[grimar]

• Addressed review comments.

llvm/tools/llvm-readobj/ELFDumper.cpp

4046

We should probably only use FIXME for real code problems. Here we should "TODO".

Done.

Actually I think the alignment check should be done outside the function.

Done.

Maybe we should check the alignments of both VerdefBuf and D->vd_aux.

This is checked already:

1. VerdefBuf is checked to be 4-bytes aligned.
2. I do VerdauxBuf = VerdefBuf + D->vd_aux and check VerdauxBuf. This checks D->vd_aux.
3. I do VerdefBuf += D->vd_next and check VerdefBuf. This checks D->vd_next.

4071

I guess this should be called "misaligned", not "unaligned", but I am not a native speaker and not very confident about this...

Perhaps "misaligned" is correct, since "If something is misaligned, then there's a correct alignment that has not been adhered to" and we have one in mind.
(https://www.reddit.com/r/grammar/comments/6x1sov/difference_between_unaligned_and_misaligned/)

5830

I think yes. I was going to suggest a follow-up for this. I find it strange to print or not print a tag basing on a "if empty" condition.
It's like if we would drop printing "Alignment: 0", just because it is not set. And it can be a problem for machine parsing,
so I believe we just should always print tags..

But I did not do it in this patch, because this introduces more changes in the existent tests, which
are probably unrelated to this patch. (I wasn't happy to change Predecessor -> Predecessors either, but I had to).

[grimar]

• Addressed review comments.

llvm/test/tools/llvm-readobj/elf-versioninfo.test

323

It comes from:

Expected<StringRef> StrTabOrErr = Obj->getStringTable(*StrTabSecOrErr);
  if (!StrTabOrErr)
    return createError(
        "SHT_GNU_verdef section: can't use the string table linked: " +
        toString(StrTabOrErr.takeError()));

i.e. we know that getStringTable failed, but can't do any additional assumptions about details. getStringTable() reports the reason and it's text is
out of the area of this patch. I've changed the prefix part.

344

How about "cannot read content of SHT_GNU_verdef section: ..."

Done.

I'd ideally put the [index 1] before the colon, i.e. "... of SHT_GNU_verdef section with index 1: the section has a ...", but I'm not sure if that risks losing useful context elsewhere.

I can only change the prefix, the rest part of the error comes from elsewhere.

Expected<ArrayRef<uint8_t>> ContentsOrErr = Obj->getSectionContents(Sec);
 if (!ContentsOrErr)
   return createError("cannot read content of SHT_GNU_verdef section: " +
                      toString(ContentsOrErr.takeError()));

llvm/tools/llvm-readobj/ELFDumper.cpp

523–524

Yeah, looks true. Done.

4046

Is there a reason we can't just test this as part of this change?

I believe it is impossible to use yaml2obj here. So the only way to do that is probably to craft and checking a binary. Do you want me to do that?

5833

I've added one.

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

4046

checking -> commit

[MaskRay]

LGTM, but please make sure @jhenderson is happy about the untested part.

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

4046

One of the simple way to avoid binary being committed is to teach yaml2obj to accept raw Content field for VerdefSection sections it seems. But again, this needs a separate yaml2obj change to be done. I am to work on this.

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

4046

"I am to work on this." -> " I am going to work on this."

[jhenderson]

In D70495#1759592, @MaskRay wrote:

LGTM, but please make sure @jhenderson is happy about the untested part.

I'd prefer the yaml2obj change land first if possible. However, if it's a sizeable change (I doubt it is, but just in case), I don't think it's worth blocking this change from landing if there's a particular need for it. I would like the error message improvement discussed inline though.

llvm/test/tools/llvm-readobj/elf-versioninfo.test

323

The "can't get the string table linked" is the bit I'm most concerned about, as it is not good English and is part of this patch. How about this instead: "invalid string table linked to SHT_GNU_verdef secion with index 1: invalid sh_type ..." (or simply "invalid section linked to...")?

Same goes for the similar message above.

338

can't read a content -> we can't read the content

[grimar]

• Addressed review comments.

[jhenderson]

Thanks! LGTM.

llvm/test/tools/llvm-readobj/elf-versioninfo.test

Entries:
           - VERSION1
       - Version:    1
         Flags:      8
         VersionNdx: 3
         Hash:       175630258
         Names:
           - VERSION2
           - VERSION1
+          - VERSION3
   - Name:         .gnu.version_r
     Type:         SHT_GNU_verneed
     Flags:        [ SHF_ALLOC ]
     Link:         .dynstr
     AddressAlign: 0x0000000000000004
     Info:         0x0000000000000002
     Dependencies:
       - Version: 1

 # LLVM-NEXT:   }
 # LLVM-NEXT:   Definition {
 # LLVM-NEXT:     Version: 1
 # LLVM-NEXT:     Flags [ (0x8)
 # LLVM-NEXT:     ]
 # LLVM-NEXT:     Index: 3
 # LLVM-NEXT:     Hash: 175630258
 # LLVM-NEXT:     Name: VERSION2
-# LLVM-NEXT:     Predecessor: VERSION1
+# LLVM-NEXT:     Predecessors: [VERSION1, VERSION3]
 # LLVM-NEXT:   }
 # LLVM-NEXT: ]
 # LLVM-NEXT: VersionRequirements [
 # LLVM-NEXT:   Dependency {
 # LLVM-NEXT:     Version: 1
 # LLVM-NEXT:     Count: 5
 # LLVM-NEXT:     FileName: verneed1.so.0
 # LLVM-NEXT:     Entries [

 # GNU-EMPTY:
 # GNU-NEXT:  Version definition section '.gnu.version_d' contains 6 entries:
 # GNU-NEXT:   Addr: 0000000000000000  Offset: 0x00004c  Link: 7 (.dynstr)
 # GNU-NEXT:    0x0000: Rev: 1  Flags: none  Index: 0  Cnt: 1  Name: VERSION1
 # GNU-NEXT:    0x001c: Rev: 1  Flags: BASE  Index: 0  Cnt: 1  Name: VERSION1
 # GNU-NEXT:    0x0038: Rev: 1  Flags: WEAK  Index: 0  Cnt: 1  Name: VERSION1
 # GNU-NEXT:    0x0054: Rev: 1  Flags: INFO  Index: 0  Cnt: 1  Name: VERSION1
 # GNU-NEXT:    0x0070: Rev: 1  Flags: BASE | WEAK | INFO  Index: 2  Cnt: 1  Name: VERSION1
-# GNU-NEXT:    0x008c: Rev: 1  Flags: <unknown>  Index: 3  Cnt: 2  Name: VERSION2
-# GNU-NEXT:    0x00a8: Parent 1: VERSION1
+# GNU-NEXT:    0x008c: Rev: 1  Flags: <unknown>  Index: 3  Cnt: 3  Name: VERSION2
+# GNU-NEXT:    0x00b0: Parent 1: VERSION1
+# GNU-NEXT:    0x00b0: Parent 2: VERSION3
 # GNU-EMPTY:
 # GNU-NEXT:  Version needs section '.gnu.version_r' contains 2 entries:
-# GNU-NEXT:   Addr: 0000000000000000  Offset: 0x0000fc  Link: 7 (.dynstr)
+# GNU-NEXT:   Addr: 0000000000000000  Offset: 0x000104  Link: 7 (.dynstr)
 # GNU-NEXT:    0x0000: Version: 1  File: verneed1.so.0  Cnt: 5
 # GNU-NEXT:    0x0010:   Name: v1  Flags: BASE  Version: 0
 # GNU-NEXT:    0x0020:   Name: v1  Flags: WEAK  Version: 0
 # GNU-NEXT:    0x0030:   Name: v1  Flags: INFO  Version: 0
 # GNU-NEXT:    0x0040:   Name: v1  Flags: BASE | WEAK | INFO  Version: 4
 # GNU-NEXT:    0x0050:   Name: v2  Flags: <unknown>  Version: 5
 # GNU-NEXT:    0x0060: Version: 1  File: verneed2.so.0  Cnt: 1
 # GNU-NEXT:    0x0070:   Name: v3  Flags: none  Version: 6
+
+## Check that we report a warning when sh_link references a non-existent section.
+
+# RUN: yaml2obj %s --docnum=2 -o %t2
+# RUN: llvm-readobj -V %t2 2>&1 | FileCheck %s --check-prefix=INVALID-LINK-LLVM -DFILE=%t2
+# RUN: not llvm-readelf -V %t2 2>&1 | FileCheck %s --check-prefix=INVALID-LINK-GNU -DFILE=%t2
+
+# INVALID-LINK-LLVM: warning: '[[FILE]]': invalid section linked to SHT_GNU_verdef section with index 1: invalid section index: 255
+
+## TODO: llvm-readelf should also report a meaningful warning instead of an error.
+# INVALID-LINK-GNU: Version definition
+# INVALID-LINK-GNU: error: '[[FILE]]': invalid section index: 255
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .gnu.version_d
+    Type:    SHT_GNU_verdef
+    Link:    0xFF
+    Info:    0x0
+    Entries: []
+
+## Check that we report a warning when the sh_link field of a SHT_GNU_verdef section references a non-string table section.
+
+# RUN: yaml2obj %s --docnum=3 -o %t3
+# RUN: llvm-readobj -V %t3 2>&1 | FileCheck %s --check-prefix=INVALID-STRING-TABLE -DFILE=%t3
+# RUN: llvm-readelf -V %t3 2>&1 | FileCheck %s --check-prefix=INVALID-STRING-TABLE -DFILE=%t3
+
+# INVALID-STRING-TABLE: warning: '[[FILE]]': invalid string table linked to SHT_GNU_verdef section with index 1: invalid sh_type for string table section [index 0]: expected SHT_STRTAB, but got SHT_NULL
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .gnu.version_d
+    Type:    SHT_GNU_verdef
+    Link:    0x0
+    Info:    0x0
+    Entries: []
+
+## Check that we report a warning when we can't read the content of the SHT_GNU_verdef section.
+
+# RUN: yaml2obj %s --docnum=4 -o %t4
+# RUN: llvm-readobj -V %t4 2>&1 | FileCheck %s --check-prefix=INVALID-DATA -DFILE=%t4
+# RUN: llvm-readelf -V %t4 2>&1 | FileCheck %s --check-prefix=INVALID-DATA -DFILE=%t4
+
+# INVALID-DATA: warning: '[[FILE]]': cannot read content of SHT_GNU_verdef section with index 1: section [index 1] has a sh_offset (0xffffffff) + sh_size (0x0) that cannot be represented
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:     .gnu.version_d
+    Type:     SHT_GNU_verdef
+    Link:     .dynstr
+    Info:     0x0
+    Entries:  []
+    ShOffset: 0xFFFFFFFF
+DynamicSymbols:
+  - Name: foo
+
+## Check that we report a warning when a SHT_GNU_verdef section contains a version definition
+## that goes past the end of the section.
+
+# RUN: yaml2obj %s --docnum=5 -o %t5
+# RUN: llvm-readobj -V %t5 2>&1 | FileCheck %s --check-prefix=DEF-PAST-END -DFILE=%t5
+# RUN: llvm-readelf -V %t5 2>&1 | FileCheck %s --check-prefix=DEF-PAST-END -DFILE=%t5
+
+# DEF-PAST-END: warning: '[[FILE]]': invalid SHT_GNU_verdef section with index 1: version definition 1 goes past the end of the section
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .gnu.version_d
+    Type:    SHT_GNU_verdef
+    Link:    .dynstr
+    Info:    0x1
+    Entries:
+      - Version:    0
+        Flags:      0
+        VersionNdx: 0
+        Hash:       0
+        Names:
+          - FOO
+    ShSize: 1
+DynamicSymbols:
+  - Name: foo
+
+## Check that we report a warning when a SHT_GNU_verdef section contains a version definition
+## that refers to an auxiliary entry that goes past the end of the section.
+
+# RUN: yaml2obj %s --docnum=6 -o %t6
+# RUN: llvm-readobj -V %t6 2>&1 | FileCheck %s --check-prefix=AUX-PAST-END -DFILE=%t6
+# RUN: llvm-readelf -V %t6 2>&1 | FileCheck %s --check-prefix=AUX-PAST-END -DFILE=%t6
+
+# AUX-PAST-END: warning: '[[FILE]]': invalid SHT_GNU_verdef section with index 1: version definition 1 refers to an auxiliary entry that goes past the end of the section
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .gnu.version_d
+    Type:    SHT_GNU_verdef
+    Link:    .dynstr
+    Info:    0x1
+    Entries:
+      - Version:    0
+        Flags:      0
+        VersionNdx: 0
+        Hash:       0
+        Names:
+          - FOO
+    ShSize: 21
+DynamicSymbols:
+  - Name: foo
+
+## Check that we can dump a SHT_GNU_verdef section properly even if it contains version names strings
+## that overrun the linked string table.
+
+# RUN: yaml2obj %s --docnum=7 -o %t7
+# RUN: llvm-readobj -V %t7 2>&1 | FileCheck %s --check-prefix=PAST-STRTAB-END-LLVM --implicit-check-not="warning:" -DFILE=%t7
+# RUN: llvm-readelf -V %t7 2>&1 | FileCheck %s --check-prefix=PAST-STRTAB-END-GNU --implicit-check-not="warning:" -DFILE=%t7
+
+# PAST-STRTAB-END-LLVM:      VersionDefinitions [
+# PAST-STRTAB-END-LLVM-NEXT:   Definition {
+# PAST-STRTAB-END-LLVM-NEXT:     Version: 0
+# PAST-STRTAB-END-LLVM-NEXT:     Flags [ (0x0)
+# PAST-STRTAB-END-LLVM-NEXT:     ]
+# PAST-STRTAB-END-LLVM-NEXT:     Index: 0
+# PAST-STRTAB-END-LLVM-NEXT:     Hash: 0
+# PAST-STRTAB-END-LLVM-NEXT:     Name: <invalid vda_name: 5>
+# PAST-STRTAB-END-LLVM-NEXT:   }
+# PAST-STRTAB-END-LLVM-NEXT: ]
+
+# PAST-STRTAB-END-GNU:      Version definition section '.gnu.version_d' contains 1 entries:
+# PAST-STRTAB-END-GNU-NEXT:  Addr: 0000000000000000  Offset: 0x000040  Link: 2 (.strtab)
+# PAST-STRTAB-END-GNU-NEXT:   0x0000: Rev: 0  Flags: none  Index: 0  Cnt: 1  Name: <invalid vda_name: 5>
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .gnu.version_d
+    Type:    SHT_GNU_verdef
+    Link:    .strtab
+    Info:    0x1
+    Entries:
+      - Version:    0
+        Flags:      0
+        VersionNdx: 0
+        Hash:       0
+        Names:
+          - FOO
+  - Name: .strtab
+    Type: SHT_STRTAB
+DynamicSymbols:
+  - Name: BAR
+
+## Check we report a warning when a version definition is not correctly aligned in memory.
+
+# RUN: yaml2obj %s --docnum=8 -o %t8
+# RUN: llvm-readobj -V %t8 2>&1 | FileCheck %s --check-prefix=MISALIGNED-DEF -DFILE=%t8
+# RUN: llvm-readelf -V %t8 2>&1 | FileCheck %s --check-prefix=MISALIGNED-DEF -DFILE=%t8
+
+# MISALIGNED-DEF: warning: '[[FILE]]': invalid SHT_GNU_verdef section with index 1: found a misaligned version definition entry at offset 0x0
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Type: Fill
+    Size: 0x1
+  - Name: .gnu.version_d
+    Type: SHT_GNU_verdef
+    Link: .dynstr
+    Info: 0x1
+    Entries:
+      - Version:    0
+        Flags:      0
+        VersionNdx: 0
+        Hash:       0
+        Names:
+          - FOO
+DynamicSymbols:
+  - Name: foo
+
+## Check we report a warning when an auxiliary entry is not correctly aligned in memory.
+
+# RUN: yaml2obj %s --docnum=9 -o %t9
+# RUN: llvm-readobj -V %t9 2>&1 | FileCheck %s --check-prefix=MISALIGNED-AUX -DFILE=%t9
+# RUN: llvm-readelf -V %t9 2>&1 | FileCheck %s --check-prefix=MISALIGNED-AUX -DFILE=%t9
+
+# MISALIGNED-AUX: warning: '[[FILE]]': invalid SHT_GNU_verdef section with index 1: found a misaligned auxiliary entry at offset 0x13
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:  .gnu.version_d
+    Type:  SHT_GNU_verdef
+    Flags: [ SHF_ALLOC ]
+    Link:  .dynstr
+    Info:  0x1
+## The byte offset to the auxiliary entry is 0x13, i.e. it is not correctly aligned in memory.
+    Content: "0000000000000100000000001300000000000000"
+DynamicSymbols:
+  - Name:    foo
+    Binding: STB_GLOBAL

llvm/test/tools/yaml2obj/ELF/verdef-section.yaml

 # CHECK-NEXT:     Version: 1
 # CHECK-NEXT:     Flags [ (0x3)
 # CHECK-NEXT:       Base (0x1)
 # CHECK-NEXT:       Weak (0x2)
 # CHECK-NEXT:     ]
 # CHECK-NEXT:     Index: 3
 # CHECK-NEXT:     Hash: 108387922
 # CHECK-NEXT:     Name: VERSION_2
-# CHECK-NEXT:     Predecessor: VERSION_3
+# CHECK-NEXT:     Predecessors: [VERSION_3]
 # CHECK-NEXT:   }
 # CHECK-NEXT: ]
 
 --- !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_DYN

llvm/tools/llvm-readobj/ELFDumper.cpp

if (EntSize != sizeof(Type) || Size % EntSize) {
                                 ") or entity size (" + Twine(EntSize) + ")"),
                     FileName);
       return {Start, Start};
     }
     return {Start, Start + (Size / EntSize)};
   }
 };
 
+namespace {
+struct VerdAux {
+  unsigned Offset;
+  std::string Name;
+};
+
+struct VerDef {
+  unsigned Offset;
+  unsigned Version;
+  unsigned Flags;
+  unsigned Ndx;
+  unsigned Cnt;
+  unsigned Hash;
+  std::string Name;
+  std::vector<VerdAux> AuxV;
+};
+} // namespace
+
 template <typename ELFT> class ELFDumper : public ObjDumper {
 public:
   ELFDumper(const object::ELFObjectFile<ELFT> *ObjF, ScopedPrinter &Writer);
 
   void printFileHeaders() override;
   void printSectionHeaders() override;
   void printRelocations() override;
   void printDynamicRelocations() override;
public:
   StringRef getDynamicStringTable() const { return DynamicStringTable; }
   const DynRegionInfo &getDynRelRegion() const { return DynRelRegion; }
   const DynRegionInfo &getDynRelaRegion() const { return DynRelaRegion; }
   const DynRegionInfo &getDynRelrRegion() const { return DynRelrRegion; }
   const DynRegionInfo &getDynPLTRelRegion() const { return DynPLTRelRegion; }
   const DynRegionInfo &getDynamicTableRegion() const { return DynamicTable; }
   const Elf_Hash *getHashTable() const { return HashTable; }
   const Elf_GnuHash *getGnuHashTable() const { return GnuHashTable; }
+
+  Expected<std::vector<VerDef>>
+  getVersionDefinitions(const Elf_Shdr *Sec) const;
+};
+
+template <class ELFT>
+Expected<std::vector<VerDef>>
+ELFDumper<ELFT>::getVersionDefinitions(const Elf_Shdr *Sec) const {
+  const ELFFile<ELFT> *Obj = ObjF->getELFFile();
+  unsigned SecNdx = Sec - &cantFail(Obj->sections()).front();
+
+  Expected<const Elf_Shdr *> StrTabSecOrErr = Obj->getSection(Sec->sh_link);
+  if (!StrTabSecOrErr)
+    return createError(
+        "invalid section linked to SHT_GNU_verdef section with index " +
+        Twine(SecNdx) + ": " + toString(StrTabSecOrErr.takeError()));
+
+  Expected<StringRef> StrTabOrErr = Obj->getStringTable(*StrTabSecOrErr);
+  if (!StrTabOrErr)
+    return createError(
+        "invalid string table linked to SHT_GNU_verdef section with index " +
+        Twine(SecNdx) + ": " + toString(StrTabOrErr.takeError()));
+
+  Expected<ArrayRef<uint8_t>> ContentsOrErr = Obj->getSectionContents(Sec);
+  if (!ContentsOrErr)
+    return createError(
+        "cannot read content of SHT_GNU_verdef section with index " +
+        Twine(SecNdx) + ": " + toString(ContentsOrErr.takeError()));
+
+  const uint8_t *Start = ContentsOrErr->data();
+  const uint8_t *End = Start + ContentsOrErr->size();
+
+  auto ExtractNextAux = [&](const uint8_t *&VerdauxBuf,
+                            unsigned VerDefNdx) -> Expected<VerdAux> {
+    if (VerdauxBuf + sizeof(Elf_Verdaux) > End)
+      return createError("invalid SHT_GNU_verdef section with index " +
+                         Twine(SecNdx) + ": version definition " +
+                         Twine(VerDefNdx) +
+                         " refers to an auxiliary entry that goes past the end "
+                         "of the section");
+
+    auto *Verdaux = reinterpret_cast<const Elf_Verdaux *>(VerdauxBuf);
+    VerdauxBuf += Verdaux->vda_next;
+
+    VerdAux Aux;
+    Aux.Offset = VerdauxBuf - Start;
+    if (Verdaux->vda_name <= StrTabOrErr->size())
+      Aux.Name = StrTabOrErr->drop_front(Verdaux->vda_name);
+    else
+      Aux.Name = "<invalid vda_name: " + to_string(Verdaux->vda_name) + ">";
+    return Aux;
   };
 
+  std::vector<VerDef> Ret;
+  const uint8_t *VerdefBuf = Start;
+  for (unsigned I = 1; I <= /*VerDefsNum=*/Sec->sh_info; ++I) {
+    if (VerdefBuf + sizeof(Elf_Verdef) > End)
+      return createError("invalid SHT_GNU_verdef section with index " +
+                         Twine(SecNdx) + ": version definition " + Twine(I) +
+                         " goes past the end of the section");
+
+    if (uintptr_t(VerdefBuf) % sizeof(uint32_t) != 0)
+      return createError(
+          "invalid SHT_GNU_verdef section with index " + Twine(SecNdx) +
+          ": found a misaligned version definition entry at offset 0x" +
+          Twine::utohexstr(VerdefBuf - Start));
+
+    const Elf_Verdef *D = reinterpret_cast<const Elf_Verdef *>(VerdefBuf);
+    VerDef &VD = *Ret.emplace(Ret.end());
+    VD.Offset = VerdefBuf - Start;
+    VD.Version = D->vd_version;
+    VD.Flags = D->vd_flags;
+    VD.Ndx = D->vd_ndx;
+    VD.Cnt = D->vd_cnt;
+    VD.Hash = D->vd_hash;
+
+    const uint8_t *VerdauxBuf = VerdefBuf + D->vd_aux;
+    for (unsigned J = 0; J < D->vd_cnt; ++J) {
+      if (uintptr_t(VerdauxBuf) % sizeof(uint32_t) != 0)
+        return createError("invalid SHT_GNU_verdef section with index " +
+                           Twine(SecNdx) +
+                           ": found a misaligned auxiliary entry at offset 0x" +
+                           Twine::utohexstr(VerdauxBuf - Start));
+
+      Expected<VerdAux> AuxOrErr = ExtractNextAux(VerdauxBuf, I);
+      if (!AuxOrErr)
+        return AuxOrErr.takeError();
+
+      if (J == 0)
+        VD.Name = AuxOrErr->Name;
+      else
+        VD.AuxV.push_back(*AuxOrErr);
+    }
+
+    VerdefBuf += D->vd_next;
+  }
+
+  return Ret;
+}
+
 template <class ELFT>
 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;
virtual void printSymtabMessage(const ELFFile<ELFT> *Obj, StringRef Name,
                                   size_t Offset, bool NonVisibilityBitsUsed) {}
   virtual void printSymbol(const ELFFile<ELFT> *Obj, const Elf_Sym *Symbol,
                            const Elf_Sym *FirstSym, StringRef StrTable,
                            bool IsDynamic, bool NonVisibilityBitsUsed) = 0;
   virtual void printProgramHeaders(const ELFFile<ELFT> *Obj,
                                    bool PrintProgramHeaders,
                                    cl::boolOrDefault PrintSectionMapping) = 0;
   virtual void printVersionSymbolSection(const ELFFile<ELFT> *Obj,
                                          const Elf_Shdr *Sec) = 0;
   virtual void printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
                                              const Elf_Shdr *Sec) = 0;
   virtual void printVersionDependencySection(const ELFFile<ELFT> *Obj,
                                              const Elf_Shdr *Sec) = 0;
   virtual void printHashHistogram(const ELFFile<ELFT> *Obj) = 0;
   virtual void printCGProfile(const ELFFile<ELFT> *Obj) = 0;
   virtual void printAddrsig(const ELFFile<ELFT> *Obj) = 0;
   virtual void printNotes(const ELFFile<ELFT> *Obj) = 0;
   virtual void printELFLinkerOptions(const ELFFile<ELFT> *Obj) = 0;

 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
                                                    const Elf_Shdr *Sec) {
   if (!Sec)
     return;
 
-  unsigned VerDefsNum = Sec->sh_info;
-  printGNUVersionSectionProlog(OS, "Version definition", VerDefsNum, Obj, Sec,
+  printGNUVersionSectionProlog(OS, "Version definition", Sec->sh_info, Obj, Sec,
                                this->FileName);
 
-  const Elf_Shdr *StrTabSec =
-      unwrapOrError(this->FileName, Obj->getSection(Sec->sh_link));
-  StringRef StringTable(
-      reinterpret_cast<const char *>(Obj->base() + StrTabSec->sh_offset),
-      (size_t)StrTabSec->sh_size);
-
-  const uint8_t *VerdefBuf =
-      unwrapOrError(this->FileName, Obj->getSectionContents(Sec)).data();
-  const uint8_t *Begin = VerdefBuf;
-
-  while (VerDefsNum--) {
-    const Elf_Verdef *Verdef = reinterpret_cast<const Elf_Verdef *>(VerdefBuf);
-    OS << format("  0x%04x: Rev: %u  Flags: %s  Index: %u  Cnt: %u",
-                 VerdefBuf - Begin, (unsigned)Verdef->vd_version,
-                 versionFlagToString(Verdef->vd_flags).c_str(),
-                 (unsigned)Verdef->vd_ndx, (unsigned)Verdef->vd_cnt);
-
-    const uint8_t *VerdauxBuf = VerdefBuf + Verdef->vd_aux;
-    const Elf_Verdaux *Verdaux =
-        reinterpret_cast<const Elf_Verdaux *>(VerdauxBuf);
-    OS << format("  Name: %s\n",
-                 StringTable.drop_front(Verdaux->vda_name).data());
-
-    for (unsigned I = 1; I < Verdef->vd_cnt; ++I) {
-      VerdauxBuf += Verdaux->vda_next;
-      Verdaux = reinterpret_cast<const Elf_Verdaux *>(VerdauxBuf);
-      OS << format("  0x%04x: Parent %u: %s\n", VerdauxBuf - Begin, I,
-                   StringTable.drop_front(Verdaux->vda_name).data());
-    }
+  Expected<std::vector<VerDef>> V = this->dumper()->getVersionDefinitions(Sec);
+  if (!V) {
+    this->reportUniqueWarning(V.takeError());
+    return;
 
-    VerdefBuf += Verdef->vd_next;
+  for (const VerDef &Def : *V) {
+    OS << format("  0x%04x: Rev: %u  Flags: %s  Index: %u  Cnt: %u  Name: %s\n",
+                 Def.Offset, Def.Version,
+                 versionFlagToString(Def.Flags).c_str(), Def.Ndx, Def.Cnt,
+                 Def.Name.data());
+    unsigned I = 0;
+    for (const VerdAux &Aux : Def.AuxV)
+      OS << format("  0x%04x: Parent %u: %s\n", Aux.Offset, ++I,
+                   Aux.Name.data());
   }
+
   OS << '\n';
 }
 
 template <class ELFT>
 void GNUStyle<ELFT>::printVersionDependencySection(const ELFFile<ELFT> *Obj,
                                                    const Elf_Shdr *Sec) {
   if (!Sec)
     return;
 
   unsigned VerneedNum = Sec->sh_info;
   printGNUVersionSectionProlog(OS, "Version needs", VerneedNum, Obj, Sec,
                                this->FileName);
 
   ArrayRef<uint8_t> SecData =
       unwrapOrError(this->FileName, Obj->getSectionContents(Sec));
 
   const Elf_Shdr *StrTabSec =
       unwrapOrError(this->FileName, Obj->getSection(Sec->sh_link));
   StringRef StringTable = {
       reinterpret_cast<const char *>(Obj->base() + StrTabSec->sh_offset),
       (size_t)StrTabSec->sh_size};
 
   const uint8_t *VerneedBuf = SecData.data();
   for (unsigned I = 0; I < VerneedNum; ++I) {
     const Elf_Verneed *Verneed =
         reinterpret_cast<const Elf_Verneed *>(VerneedBuf);
 
     StringRef File = StringTable.size() > Verneed->vn_file
                          ? StringTable.drop_front(Verneed->vn_file)
                          : "<invalid>";
 
     OS << format("  0x%04x: Version: %u  File: %s  Cnt: %u\n",
                  reinterpret_cast<const uint8_t *>(Verneed) - SecData.begin(),
                  (unsigned)Verneed->vn_version, File.data(),
                  (unsigned)Verneed->vn_cnt);
 
     const uint8_t *VernauxBuf = VerneedBuf + Verneed->vn_aux;
     for (unsigned J = 0; J < Verneed->vn_cnt; ++J) {
       const Elf_Vernaux *Vernaux =
           reinterpret_cast<const Elf_Vernaux *>(VernauxBuf);
 
       StringRef Name = StringTable.size() > Vernaux->vna_name
                            ? StringTable.drop_front(Vernaux->vna_name)

 
 template <class ELFT>
 void LLVMStyle<ELFT>::printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
                                                     const Elf_Shdr *Sec) {
   ListScope SD(W, "VersionDefinitions");
   if (!Sec)
     return;
 
-  const uint8_t *SecStartAddress =
-      reinterpret_cast<const uint8_t *>(Obj->base() + Sec->sh_offset);
-  const uint8_t *SecEndAddress = SecStartAddress + Sec->sh_size;
-  const uint8_t *VerdefBuf = SecStartAddress;
-  const Elf_Shdr *StrTab =
-      unwrapOrError(this->FileName, Obj->getSection(Sec->sh_link));
-
-  unsigned VerDefsNum = Sec->sh_info;
-  while (VerDefsNum--) {
-    if (VerdefBuf + sizeof(Elf_Verdef) > SecEndAddress)
-      // FIXME: report_fatal_error is not a good way to report error. We should
-      // emit a parsing error here and below.
-      report_fatal_error("invalid offset in the section");
+  Expected<std::vector<VerDef>> V = this->dumper()->getVersionDefinitions(Sec);
+  if (!V) {
+    this->reportUniqueWarning(V.takeError());
+    return;
+  }
 
-    const Elf_Verdef *Verdef = reinterpret_cast<const Elf_Verdef *>(VerdefBuf);
+  for (const VerDef &D : *V) {
     DictScope Def(W, "Definition");
-    W.printNumber("Version", Verdef->vd_version);
-    W.printFlags("Flags", Verdef->vd_flags, makeArrayRef(SymVersionFlags));
-    W.printNumber("Index", Verdef->vd_ndx);
-    W.printNumber("Hash", Verdef->vd_hash);
-    W.printString("Name", StringRef(reinterpret_cast<const char *>(
-                              Obj->base() + StrTab->sh_offset +
-                              Verdef->getAux()->vda_name)));
-    if (!Verdef->vd_cnt)
-      report_fatal_error("at least one definition string must exist");
-    if (Verdef->vd_cnt > 2)
-      report_fatal_error("more than one predecessor is not expected");
-
-    if (Verdef->vd_cnt == 2) {
-      const uint8_t *VerdauxBuf =
-          VerdefBuf + Verdef->vd_aux + Verdef->getAux()->vda_next;
-      const Elf_Verdaux *Verdaux =
-          reinterpret_cast<const Elf_Verdaux *>(VerdauxBuf);
-      W.printString("Predecessor",
-                    StringRef(reinterpret_cast<const char *>(
-                        Obj->base() + StrTab->sh_offset + Verdaux->vda_name)));
-    }
-    VerdefBuf += Verdef->vd_next;
+    W.printNumber("Version", D.Version);
+    W.printFlags("Flags", D.Flags, makeArrayRef(SymVersionFlags));
+    W.printNumber("Index", D.Ndx);
+    W.printNumber("Hash", D.Hash);
+    W.printString("Name", D.Name.c_str());
+
+    if (D.AuxV.empty())
+      continue;
+    W.printList(
+        "Predecessors", D.AuxV,
+        [](raw_ostream &OS, const VerdAux &Aux) { OS << Aux.Name.c_str(); });
   }
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printVersionDependencySection(const ELFFile<ELFT> *Obj,
                                                     const Elf_Shdr *Sec) {
   ListScope SD(W, "VersionRequirements");
   if (!Sec)