[llvm-readobj] Separate `Symbol Version` dumpers into `LLVM style` and `GNU style`

[Higuoxing]

Summary

Currently, llvm-readobj can dump symbol version sections only in LLVM style. In this patch, I would like to separate these dumpers into GNU style and
LLVM style for future implementation.

[Higuoxing]

Format codes.

[Higuoxing]

Updating D59186: [llvm-readobj] Separate Symbol Version dumpers into LLVM style and GNU style

[jhenderson]

Are you definitely going to implement a GNU style after this? Otherwise, you've removed any form of support from the GNU dumper, without replacing it with something.

tools/llvm-readobj/ELFDumper.cpp

3392–3393

I think these unused named parameters might result in warnings on some build systems. Ditto below.

3394

Here, and below, I don't think you should mention the section names, unless you have actually derived the names from the section passed in.

4524

It may be worth factoring out this->dumper() into a local variable, since you use it in a number of places.

4555

I know this was there before, but report_fatal_error is not a good way of reporting errors, since it implies an internal bug in LLVM. It should report a normal parse error or similar in the same way as other errors in llvm-readobj.

Ditto below.

I don't mind if it's in a follow-up patch though.

[Higuoxing]

In D59186#1425803, @jhenderson wrote:

Are you definitely going to implement a GNU style after this? Otherwise, you've removed any form of support from the GNU dumper, without replacing it with something.

Yes, I would like to implement these functions. But I'm afraid these functions make this patch too big. I would like to make these patches in series, so you could review them one by one.

[grimar]

I'll leave this to James. AFAIK he will do a BoF at LLVM 2019. I think
I need to listen to it to understand the desired direction of tools.

(splitting of formats is my concern)

[Higuoxing]

In D59186#1425910, @grimar wrote:

I'll leave this to James. AFAIK he will do a BoF at LLVM 2019.

Cool! I would like to watch it on Youtube ...
Thank you for taking time review this.

[jhenderson]

In D59186#1425920, @Higuoxing wrote:

In D59186#1425910, @grimar wrote:

I'll leave this to James. AFAIK he will do a BoF at LLVM 2019.

Cool! I would like to watch it on Youtube ...
Thank you for taking time review this.

BoFs are not usually recorded, I believe, but there will be a write-up published on llvm-dev hopefully after the event.

[grimar]

In D59186#1425942, @jhenderson wrote:

In D59186#1425920, @Higuoxing wrote:

In D59186#1425910, @grimar wrote:

I'll leave this to James. AFAIK he will do a BoF at LLVM 2019.

Cool! I would like to watch it on Youtube ...
Thank you for taking time review this.

BoFs are not usually recorded

Not this time, James )) Don't blame me when you be on youtube..

[grimar]

I am just kidding. Waiting for BoF..

[Higuoxing]

Addressed @jhenderson 's comments

It may be worth factoring out this->dumper() into a local variable, since you use it in a number of places.

Done

I know this was there before, but report_fatal_error is not a good way of reporting errors, since it implies an internal bug in LLVM. It should report a normal parse error or similar in the same way as other errors in llvm-readobj.

I would like to fix this issue here and that in LLVM style dumper in next patch

---

I implemented 3 dumpers for .gnu.version, .gnu.version_r, .gnu.version_d, with some small adjustments.

• GNU readelf treats 0x0 flags as none, so I add one in SymVersionFlags
• I separate getSymbolVersion into getSymbolVersion and getSymbolVersionByIndex. Because when dumping .gnu.version section, we should print symbol version for symbols that have valid version name.

However, there are some issues.

• Currently, when dumping .gnu.version_d section, we assume a symbol can have only one predecessor. But GNU readelf could dump these predecessors correctly.
• When dumping some fields that contains offset into string table, if the offset is invalid, GNU readelf could give warning, and give the malformed value.

Shall I separate this patch into several small patches?

[jhenderson]

I'm always in favour of splitting up changes into multiple smaller reviews, as long as it doesn't make things more complicated by requiring temporary logic or whatever that is just going to be removed again immediately. It'll hopefully help diffing the changes too.

[Higuoxing]

In D59186#1434629, @jhenderson wrote:

I'm always in favour of splitting up changes into multiple smaller reviews, as long as it doesn't make things more complicated by requiring temporary logic or whatever that is just going to be removed again immediately. It'll hopefully help diffing the changes too.

Ok, thanks :)

[Higuoxing]

Update.

Addressed @jhenderson 's comments.

• Using the actual section name.
• Add FIXME comments near report_fatal_error() (I would like to fix this in the future).

[rupprecht]

test/tools/yaml2obj/verdef-section.yaml

2

These tests should -- temporarily -- run both llvm-readobj+llvm-readelf and verify it's the same (or at least the version sections match)

(i.e. you probably can't cmp the whole thing like other tests do, but the same filecheck should work)

[Higuoxing]

Addressed @rupprecht 's comments.

• Add llvm-readelf to test cases.

I didn't modify the tests in tests/tools/yaml2obj, because I think those tests are for yaml2obj not llvm-readobj. Am I right?

[jhenderson]

In D59186#1435764, @Higuoxing wrote:

I didn't modify the tests in tests/tools/yaml2obj, because I think those tests are for yaml2obj not llvm-readobj. Am I right?

Yes, that's right.

What's the relation between this patch and D59545?

[Higuoxing]

In D59186#1436196, @jhenderson wrote:

What's the relation between this patch and D59545?

I want to implement dumpers for these sections. In .gnu.version, we should print the version name of a versioning symbol, so I need a helper function to find the version name by its vs_index.

[jhenderson]

In D59186#1436228, @Higuoxing wrote:

In D59186#1436196, @jhenderson wrote:

What's the relation between this patch and D59545?

I want to implement dumpers for these sections. In .gnu.version, we should print the version name of a versioning symbol, so I need a helper function to find the version name by its vs_index.

Right, so does this patch depend on D59545 then?

tools/llvm-readobj/ELFDumper.cpp

4524

This hasn't been done?

[Higuoxing]

In D59186#1436234, @jhenderson wrote:

Right, so does this patch depend on D59545 then?

No, this is the first patch, which makes way for future implementation.

[jhenderson]

I'm happy with this, once my suggestions have been fixed.

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

83

Nit: extra blank line.

[Higuoxing]

Addressed comments. Thanks for reviewing!

[jhenderson]

LGTM, with one last comment. Please wait for @rupprecht to confirm that he is happy.

tools/llvm-readobj/ELFDumper.cpp

4524

ELFDumper<ELFT> not auto

[Higuoxing]

Thanks :) I will work on following patches.

[Higuoxing]

Rebase.

[rupprecht]

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

6–118

nit: use LLVM-VERDEF/GNU-VERDEF for consistent hyphens (e.g. LLVM_VERDEF-NEXT should be LLVM-VERDEF-NEXT)

[Higuoxing]

Addressed comments.

[Higuoxing]

Oops, I made a mistake in this patch, so I reverted the change. I'm sorry.

[RKSimon]

In D59186#1439653, @Higuoxing wrote:

Oops, I made a mistake in this patch, so I reverted the change. I'm sorry.

@Higuoxing The reversion doesn't appear to have been committed

[RKSimon]

In D59186#1440082, @RKSimon wrote:

In D59186#1439653, @Higuoxing wrote:

Oops, I made a mistake in this patch, so I reverted the change. I'm sorry.

@Higuoxing The reversion doesn't appear to have been committed

It looks like you only reverted ElfDumper.cpp at rL356777 - @dyung reverted the tests at rL356811

[Higuoxing]

In D59186#1440097, @RKSimon wrote:

In D59186#1440082, @RKSimon wrote:

In D59186#1439653, @Higuoxing wrote:

Oops, I made a mistake in this patch, so I reverted the change. I'm sorry.

@Higuoxing The reversion doesn't appear to have been committed

It looks like you only reverted ElfDumper.cpp at rL356777 - @dyung reverted the tests at rL356811

I am sorry, after reverting the change, I went to sleep last night ... I should have double checked this change.

[Higuoxing]

Hi, @jhenderson, @rupprecht

I am sorry that this patch should be reviewed once again. I made a mistake in previous patch, which I will illustrate below.

[Higuoxing]

tools/llvm-readobj/ELFDumper.cpp

4534

The version of a symbol in .gnu.version should be calculated using Versym->vs_index & VERSYM_VERSION (0x7fff). Before this patch, we use *VersymBuf as its version. I think it's not a right way, though it does the right thing in most time.

[jhenderson]

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

119

Nit: no newline at EOF.

test/tools/yaml2obj/verneed-section.yaml

74

Nit: missing new line at EOF here.

tools/llvm-readobj/ELFDumper.cpp

4534

I wasn't able to find any documentation for this outside of code, but I see that it is already used elsewhere in this file in a similar manner, and I found other examples of it in code elsewhere, so this looks fine to me.

I'm not quite sure what you mean by:

I think it's not a right way, though it does the right thing in most time.

Which way don't you think is right? What conditions does it do the right thin most of the time?

[Higuoxing]

tools/llvm-readobj/ELFDumper.cpp

4534

Which way don't you think is right? What conditions does it do the right thin most of the time?

I think using *VersymBuf is not a good way to indicate the symbol version, and we should use Versym->vs_index & VERSYM_VERSION. Usually, the version of a symbol should be a number that not so big. So, *VersymBuf (1 byte) should be equal to Versym->vs_index & VERSYM_VERSION (2 bytes).

[Higuoxing]

Update diff according to comments.

[jhenderson]

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

11

Maybe you should make one of the symbols in this section have a version > 0x7fff, e.g. 0xffff?

tools/llvm-readobj/ELFDumper.cpp

4534

Right, gotcha. LGTM. In fact, it is wise to not do *VersymBuf, because that won't take account of any Endian issues.

[Higuoxing]

.

[Higuoxing]

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

11

This test file uses a pre-compiled object file, I want (replace this/add one test) using yaml2obj in next patch.

[jhenderson]

LGTM.

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

11

Right, of course.

[Higuoxing]

In D59186#1441183, @jhenderson wrote:

LGTM.

Thanks a lot!

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

 // Test that llvm-readobj dumps version info tags correctly.
 
-RUN: llvm-readobj -dynamic-table -V %p/Inputs/verdef.elf-x86-64 | FileCheck %s
+RUN: llvm-readobj -dynamic-table -V %p/Inputs/verdef.elf-x86-64 | FileCheck %s --check-prefix=LLVM_VERDEF
+RUN: llvm-readelf -dynamic-table -V %p/Inputs/verdef.elf-x86-64 | FileCheck %s --check-prefix=GNU_VERDEF
 
-CHECK: 0x000000006FFFFFF0 VERSYM               0x24C
-CHECK: 0x000000006FFFFFFC VERDEF               0x25C
-CHECK: 0x000000006FFFFFFD VERDEFNUM            3
-
-CHECK: Version symbols {
-CHECK-NEXT:   Section Name: .gnu.version (20)
-CHECK-NEXT:   Address: 0x24C
-CHECK-NEXT:   Offset: 0x24C
-CHECK-NEXT:   Link: 1
-CHECK-NEXT:   Symbols [
-CHECK-NEXT:     Symbol {
-CHECK-NEXT:       Version: 0
-CHECK-NEXT:       Name: {{$}}
-CHECK-NEXT:     }
-CHECK-NEXT:     Symbol {
-CHECK-NEXT:       Version: 1
-CHECK-NEXT:       Name: _end{{$}}
-CHECK-NEXT:     }
-CHECK-NEXT:     Symbol {
-CHECK-NEXT:       Version: 1
-CHECK-NEXT:       Name: _edata{{$}}
-CHECK-NEXT:     }
-CHECK-NEXT:     Symbol {
-CHECK-NEXT:       Version: 3
-CHECK-NEXT:       Name: goo@@VERSION2
-CHECK-NEXT:     }
-CHECK-NEXT:     Symbol {
-CHECK-NEXT:       Version: 1
-CHECK-NEXT:       Name: __bss_start{{$}}
-CHECK-NEXT:     }
-CHECK-NEXT:     Symbol {
-CHECK-NEXT:       Version: 2
-CHECK-NEXT:       Name: foo@@VERSION1
-CHECK-NEXT:     }
-CHECK-NEXT:     Symbol {
-CHECK-NEXT:       Version: 2
-CHECK-NEXT:       Name: VERSION1@@VERSION1
-CHECK-NEXT:     }
-CHECK-NEXT:     Symbol {
-CHECK-NEXT:       Version: 3
-CHECK-NEXT:       Name: VERSION2@@VERSION2
-CHECK-NEXT:     }
-CHECK-NEXT:   ]
-CHECK-NEXT: }
-
-CHECK:      SHT_GNU_verdef {
-CHECK-NEXT:   Definition {
-CHECK-NEXT:     Version: 1
-CHECK-NEXT:     Flags: Base (0x1)
-CHECK-NEXT:     Index: 1
-CHECK-NEXT:     Hash: 430712
-CHECK-NEXT:     Name: blah
-CHECK-NEXT:   }
-CHECK-NEXT:   Definition {
-CHECK-NEXT:     Version: 1
-CHECK-NEXT:     Flags: 0x0
-CHECK-NEXT:     Index: 2
-CHECK-NEXT:     Hash: 175630257
-CHECK-NEXT:     Name: VERSION1
-CHECK-NEXT:   }
-CHECK-NEXT:   Definition {
-CHECK-NEXT:     Version: 1
-CHECK-NEXT:     Flags: 0x0
-CHECK-NEXT:     Index: 3
-CHECK-NEXT:     Hash: 175630258
-CHECK-NEXT:     Name: VERSION2
-CHECK-NEXT:     Predecessor: VERSION1
-CHECK-NEXT:   }
-CHECK-NEXT: }
-
-RUN: llvm-readobj -V %p/Inputs/verneed.elf-x86-64 | FileCheck %s --check-prefix=VERNEED
-
-VERNEED:       SHT_GNU_verneed {
-VERNEED-NEXT:   Dependency {
-VERNEED-NEXT:     Version: 1
-VERNEED-NEXT:     Count: 2
-VERNEED-NEXT:     FileName: verneed1.so.0
-VERNEED-NEXT:     Entry {
-VERNEED-NEXT:       Hash: 1938
-VERNEED-NEXT:       Flags: 0x0
-VERNEED-NEXT:       Index: 3
-VERNEED-NEXT:       Name: v2
-VERNEED-NEXT:     }
-VERNEED-NEXT:     Entry {
-VERNEED-NEXT:       Hash: 1939
-VERNEED-NEXT:       Flags: 0x0
-VERNEED-NEXT:       Index: 2
-VERNEED-NEXT:       Name: v3
-VERNEED-NEXT:     }
-VERNEED-NEXT:   }
-VERNEED-NEXT:   Dependency {
-VERNEED-NEXT:     Version: 1
-VERNEED-NEXT:     Count: 1
-VERNEED-NEXT:     FileName: verneed2.so.0
-VERNEED-NEXT:     Entry {
-VERNEED-NEXT:       Hash: 1937
-VERNEED-NEXT:       Flags: 0x0
-VERNEED-NEXT:       Index: 4
-VERNEED-NEXT:       Name: v1
-VERNEED-NEXT:     }
-VERNEED-NEXT:   }
-VERNEED-NEXT: }
+LLVM_VERDEF: 0x000000006FFFFFF0 VERSYM               0x24C
+LLVM_VERDEF: 0x000000006FFFFFFC VERDEF               0x25C
+LLVM_VERDEF: 0x000000006FFFFFFD VERDEFNUM            3
+
+LLVM_VERDEF: Version symbols {
+LLVM_VERDEF-NEXT:   Section Name: .gnu.version (20)
+LLVM_VERDEF-NEXT:   Address: 0x24C
+LLVM_VERDEF-NEXT:   Offset: 0x24C
+LLVM_VERDEF-NEXT:   Link: 1
+LLVM_VERDEF-NEXT:   Symbols [
+LLVM_VERDEF-NEXT:     Symbol {
+LLVM_VERDEF-NEXT:       Version: 0
+LLVM_VERDEF-NEXT:       Name: {{$}}
+LLVM_VERDEF-NEXT:     }
+LLVM_VERDEF-NEXT:     Symbol {
+LLVM_VERDEF-NEXT:       Version: 1
+LLVM_VERDEF-NEXT:       Name: _end{{$}}
+LLVM_VERDEF-NEXT:     }
+LLVM_VERDEF-NEXT:     Symbol {
+LLVM_VERDEF-NEXT:       Version: 1
+LLVM_VERDEF-NEXT:       Name: _edata{{$}}
+LLVM_VERDEF-NEXT:     }
+LLVM_VERDEF-NEXT:     Symbol {
+LLVM_VERDEF-NEXT:       Version: 3
+LLVM_VERDEF-NEXT:       Name: goo@@VERSION2
+LLVM_VERDEF-NEXT:     }
+LLVM_VERDEF-NEXT:     Symbol {
+LLVM_VERDEF-NEXT:       Version: 1
+LLVM_VERDEF-NEXT:       Name: __bss_start{{$}}
+LLVM_VERDEF-NEXT:     }
+LLVM_VERDEF-NEXT:     Symbol {
+LLVM_VERDEF-NEXT:       Version: 2
+LLVM_VERDEF-NEXT:       Name: foo@@VERSION1
+LLVM_VERDEF-NEXT:     }
+LLVM_VERDEF-NEXT:     Symbol {
+LLVM_VERDEF-NEXT:       Version: 2
+LLVM_VERDEF-NEXT:       Name: VERSION1@@VERSION1
+LLVM_VERDEF-NEXT:     }
+LLVM_VERDEF-NEXT:     Symbol {
+LLVM_VERDEF-NEXT:       Version: 3
+LLVM_VERDEF-NEXT:       Name: VERSION2@@VERSION2
+LLVM_VERDEF-NEXT:     }
+LLVM_VERDEF-NEXT:   ]
+LLVM_VERDEF-NEXT: }
+
+LLVM_VERDEF:      SHT_GNU_verdef {
+LLVM_VERDEF-NEXT:   Definition {
+LLVM_VERDEF-NEXT:     Version: 1
+LLVM_VERDEF-NEXT:     Flags: Base (0x1)
+LLVM_VERDEF-NEXT:     Index: 1
+LLVM_VERDEF-NEXT:     Hash: 430712
+LLVM_VERDEF-NEXT:     Name: blah
+LLVM_VERDEF-NEXT:   }
+LLVM_VERDEF-NEXT:   Definition {
+LLVM_VERDEF-NEXT:     Version: 1
+LLVM_VERDEF-NEXT:     Flags: 0x0
+LLVM_VERDEF-NEXT:     Index: 2
+LLVM_VERDEF-NEXT:     Hash: 175630257
+LLVM_VERDEF-NEXT:     Name: VERSION1
+LLVM_VERDEF-NEXT:   }
+LLVM_VERDEF-NEXT:   Definition {
+LLVM_VERDEF-NEXT:     Version: 1
+LLVM_VERDEF-NEXT:     Flags: 0x0
+LLVM_VERDEF-NEXT:     Index: 3
+LLVM_VERDEF-NEXT:     Hash: 175630258
+LLVM_VERDEF-NEXT:     Name: VERSION2
+LLVM_VERDEF-NEXT:     Predecessor: VERSION1
+LLVM_VERDEF-NEXT:   }
+LLVM_VERDEF-NEXT: }
+
+GNU_VERDEF: 0x000000006ffffff0 VERSYM               0x24c
+GNU_VERDEF: 0x000000006ffffffc VERDEF               0x25c
+GNU_VERDEF: 0x000000006ffffffd VERDEFNUM            3
+
+GNU_VERDEF: Dumper for .gnu.version is not implemented
+GNU_VERDEF: Dumper for .gnu.version_d is not implemented
+
+RUN: llvm-readobj -V %p/Inputs/verneed.elf-x86-64 | FileCheck %s --check-prefix=LLVM_VERNEED
+RUN: llvm-readelf -V %p/Inputs/verneed.elf-x86-64 | FileCheck %s --check-prefix=GNU_VERNEED
+
+LLVM_VERNEED:       SHT_GNU_verneed {
+LLVM_VERNEED-NEXT:   Dependency {
+LLVM_VERNEED-NEXT:     Version: 1
+LLVM_VERNEED-NEXT:     Count: 2
+LLVM_VERNEED-NEXT:     FileName: verneed1.so.0
+LLVM_VERNEED-NEXT:     Entry {
+LLVM_VERNEED-NEXT:       Hash: 1938
+LLVM_VERNEED-NEXT:       Flags: 0x0
+LLVM_VERNEED-NEXT:       Index: 3
+LLVM_VERNEED-NEXT:       Name: v2
+LLVM_VERNEED-NEXT:     }
+LLVM_VERNEED-NEXT:     Entry {
+LLVM_VERNEED-NEXT:       Hash: 1939
+LLVM_VERNEED-NEXT:       Flags: 0x0
+LLVM_VERNEED-NEXT:       Index: 2
+LLVM_VERNEED-NEXT:       Name: v3
+LLVM_VERNEED-NEXT:     }
+LLVM_VERNEED-NEXT:   }
+LLVM_VERNEED-NEXT:   Dependency {
+LLVM_VERNEED-NEXT:     Version: 1
+LLVM_VERNEED-NEXT:     Count: 1
+LLVM_VERNEED-NEXT:     FileName: verneed2.so.0
+LLVM_VERNEED-NEXT:     Entry {
+LLVM_VERNEED-NEXT:       Hash: 1937
+LLVM_VERNEED-NEXT:       Flags: 0x0
+LLVM_VERNEED-NEXT:       Index: 4
+LLVM_VERNEED-NEXT:       Name: v1
+LLVM_VERNEED-NEXT:     }
+LLVM_VERNEED-NEXT:   }
+LLVM_VERNEED-NEXT: }
+
+GNU_VERNEED: Dumper for .gnu.version is not implemented
+GNU_VERNEED: Dumper for .gnu.version_r is not implemented

test/tools/yaml2obj/verdef-section.yaml

 # RUN: yaml2obj %s -o %t
-# RUN: llvm-readelf -V %t | FileCheck %s
+# RUN: llvm-readobj -V %t | FileCheck %s
 
 # Check we are able to handle the SHT_GNU_verdef sections.
 
 # CHECK:      SHT_GNU_verdef {
 # CHECK-NEXT:   Definition {
 # CHECK-NEXT:     Version: 1
 # CHECK-NEXT:     Flags: Base
 # CHECK-NEXT:     Index: 1

test/tools/yaml2obj/verneed-section.yaml

 # RUN: yaml2obj %s -o %t
-# RUN: llvm-readelf -V %t | FileCheck %s
+# RUN: llvm-readobj -V %t | FileCheck %s
 
 # Check we are able to handle the SHT_GNU_verneed sections.
 
 # CHECK:      SHT_GNU_verneed {
 # CHECK-NEXT:   Dependency {
 # CHECK-NEXT:     Version: 1
 # CHECK-NEXT:     Count: 2
 # CHECK-NEXT:     FileName: dso.so.0
Dependencies:
         File:            dso.so.1
         Entries:
           - Name:            v3
             Hash:            1939
             Flags:           12
             Other:           2
 DynamicSymbols:
   Global:
     - Name:            f1

test/tools/yaml2obj/versym-section.yaml

 # RUN: yaml2obj %s -o %t
-# RUN: llvm-readelf -V %t | FileCheck %s
+# RUN: llvm-readobj -V %t | FileCheck %s
 
 ## Check we are able to produce a valid SHT_GNU_versym
 ## section from its description.
 
 # CHECK:      Version symbols {
 # CHECK-NEXT:   Section Name: .gnu.version
 # CHECK-NEXT:   Address: 0x200210
 # CHECK-NEXT:   Offset: 0x240

tools/llvm-readobj/ELFDumper.cpp

public:
   virtual void printSymtabMessage(const ELFFile<ELFT> *Obj, StringRef Name,
                                   size_t Offset) {}
   virtual void printSymbol(const ELFFile<ELFT> *Obj, const Elf_Sym *Symbol,
                            const Elf_Sym *FirstSym, StringRef StrTable,
                            bool IsDynamic) = 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;
   virtual void printMipsGOT(const MipsGOTParser<ELFT> &Parser) = 0;
   virtual void printMipsPLT(const MipsGOTParser<ELFT> &Parser) = 0;
   const ELFDumper<ELFT> *dumper() const { return Dumper; }
public:
   void printSymbols(const ELFO *Obj, bool PrintSymbols,
                     bool PrintDynamicSymbols) override;
   void printHashSymbols(const ELFO *Obj) override;
   void printDynamicRelocations(const ELFO *Obj) override;
   void printSymtabMessage(const ELFO *Obj, StringRef Name,
                           size_t Offset) override;
   void printProgramHeaders(const ELFO *Obj, bool PrintProgramHeaders,
                            cl::boolOrDefault PrintSectionMapping) override;
+  void printVersionSymbolSection(const ELFFile<ELFT> *Obj,
+                                 const Elf_Shdr *Sec) override;
+  void printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
+                                     const Elf_Shdr *Sec) override;
+  void printVersionDependencySection(const ELFFile<ELFT> *Obj,
+                                     const Elf_Shdr *Sec) override;
   void printHashHistogram(const ELFFile<ELFT> *Obj) override;
   void printCGProfile(const ELFFile<ELFT> *Obj) override;
   void printAddrsig(const ELFFile<ELFT> *Obj) override;
   void printNotes(const ELFFile<ELFT> *Obj) override;
   void printELFLinkerOptions(const ELFFile<ELFT> *Obj) override;
   void printMipsGOT(const MipsGOTParser<ELFT> &Parser) override;
   void printMipsPLT(const MipsGOTParser<ELFT> &Parser) override;
 
public:
   void printRelocations(const ELFO *Obj) override;
   void printRelocations(const Elf_Shdr *Sec, const ELFO *Obj);
   void printSectionHeaders(const ELFO *Obj) override;
   void printSymbols(const ELFO *Obj, bool PrintSymbols,
                     bool PrintDynamicSymbols) override;
   void printDynamicRelocations(const ELFO *Obj) override;
   void printProgramHeaders(const ELFO *Obj, bool PrintProgramHeaders,
                            cl::boolOrDefault PrintSectionMapping) override;
+  void printVersionSymbolSection(const ELFFile<ELFT> *Obj,
+                                 const Elf_Shdr *Sec) override;
+  void printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
+                                     const Elf_Shdr *Sec) override;
+  void printVersionDependencySection(const ELFFile<ELFT> *Obj,
+                                     const Elf_Shdr *Sec) override;
   void printHashHistogram(const ELFFile<ELFT> *Obj) override;
   void printCGProfile(const ELFFile<ELFT> *Obj) override;
   void printAddrsig(const ELFFile<ELFT> *Obj) override;
   void printNotes(const ELFFile<ELFT> *Obj) override;
   void printELFLinkerOptions(const ELFFile<ELFT> *Obj) override;
   void printMipsGOT(const MipsGOTParser<ELFT> &Parser) override;
   void printMipsPLT(const MipsGOTParser<ELFT> &Parser) override;
 
for (const typename ELFO::Elf_Sym &Sym : Dumper->dynamic_symbols()) {
     std::string FullSymbolName =
         Dumper->getFullSymbolName(&Sym, StrTable, true /* IsDynamic */);
     W.printNumber("Version", *P);
     W.printString("Name", FullSymbolName);
     P += sizeof(typename ELFO::Elf_Half);
   }
 }
 
-static const EnumEntry<unsigned> SymVersionFlags[] = {
-    {"Base", "BASE", VER_FLG_BASE},
-    {"Weak", "WEAK", VER_FLG_WEAK},
-    {"Info", "INFO", VER_FLG_INFO}};
-
-template <typename ELFO, class ELFT>
-static void printVersionDefinitionSection(ELFDumper<ELFT> *Dumper,
-                                          const ELFO *Obj,
-                                          const typename ELFO::Elf_Shdr *Sec,
-                                          ScopedPrinter &W) {
-  using VerDef = typename ELFO::Elf_Verdef;
-  using VerdAux = typename ELFO::Elf_Verdaux;
-
-  DictScope SD(W, "SHT_GNU_verdef");
-  if (!Sec)
-    return;
-
-  const uint8_t *SecStartAddress =
-      (const uint8_t *)Obj->base() + Sec->sh_offset;
-  const uint8_t *SecEndAddress = SecStartAddress + Sec->sh_size;
-  const uint8_t *P = SecStartAddress;
-  const typename ELFO::Elf_Shdr *StrTab =
-      unwrapOrError(Obj->getSection(Sec->sh_link));
-
-  unsigned VerDefsNum = Sec->sh_info;
-  while (VerDefsNum--) {
-    if (P + sizeof(VerDef) > SecEndAddress)
-      report_fatal_error("invalid offset in the section");
-
-    auto *VD = reinterpret_cast<const VerDef *>(P);
-    DictScope Def(W, "Definition");
-    W.printNumber("Version", VD->vd_version);
-    W.printEnum("Flags", VD->vd_flags, makeArrayRef(SymVersionFlags));
-    W.printNumber("Index", VD->vd_ndx);
-    W.printNumber("Hash", VD->vd_hash);
-    W.printString("Name",
-                  StringRef((const char *)(Obj->base() + StrTab->sh_offset +
-                                           VD->getAux()->vda_name)));
-    if (!VD->vd_cnt)
-      report_fatal_error("at least one definition string must exist");
-    if (VD->vd_cnt > 2)
-      report_fatal_error("more than one predecessor is not expected");
-
-    if (VD->vd_cnt == 2) {
-      const uint8_t *PAux = P + VD->vd_aux + VD->getAux()->vda_next;
-      const VerdAux *Aux = reinterpret_cast<const VerdAux *>(PAux);
-      W.printString("Predecessor",
-                    StringRef((const char *)(Obj->base() + StrTab->sh_offset +
-                                             Aux->vda_name)));
-    }
-
-    P += VD->vd_next;
-  }
-}
-
-template <typename ELFO, class ELFT>
-static void printVersionDependencySection(ELFDumper<ELFT> *Dumper,
-                                          const ELFO *Obj,
-                                          const typename ELFO::Elf_Shdr *Sec,
-                                          ScopedPrinter &W) {
-  using VerNeed = typename ELFO::Elf_Verneed;
-  using VernAux = typename ELFO::Elf_Vernaux;
-
-  DictScope SD(W, "SHT_GNU_verneed");
-  if (!Sec)
-    return;
-
-  const uint8_t *SecData = (const uint8_t *)Obj->base() + Sec->sh_offset;
-  const typename ELFO::Elf_Shdr *StrTab =
-      unwrapOrError(Obj->getSection(Sec->sh_link));
-
-  const uint8_t *P = SecData;
-  unsigned VerNeedNum = Sec->sh_info;
-  for (unsigned I = 0; I < VerNeedNum; ++I) {
-    const VerNeed *Need = reinterpret_cast<const VerNeed *>(P);
-    DictScope Entry(W, "Dependency");
-    W.printNumber("Version", Need->vn_version);
-    W.printNumber("Count", Need->vn_cnt);
-    W.printString("FileName",
-                  StringRef((const char *)(Obj->base() + StrTab->sh_offset +
-                                           Need->vn_file)));
-
-    const uint8_t *PAux = P + Need->vn_aux;
-    for (unsigned J = 0; J < Need->vn_cnt; ++J) {
-      const VernAux *Aux = reinterpret_cast<const VernAux *>(PAux);
-      DictScope Entry(W, "Entry");
-      W.printNumber("Hash", Aux->vna_hash);
-      W.printEnum("Flags", Aux->vna_flags, makeArrayRef(SymVersionFlags));
-      W.printNumber("Index", Aux->vna_other);
-      W.printString("Name",
-                    StringRef((const char *)(Obj->base() + StrTab->sh_offset +
-                                             Aux->vna_name)));
-      PAux += Aux->vna_next;
-    }
-    P += Need->vn_next;
-  }
-}
-
 template <typename ELFT> void ELFDumper<ELFT>::printVersionInfo() {
   // Dump version symbol section.
-  printVersionSymbolSection(this, ObjF->getELFFile(), dot_gnu_version_sec, W);
+  ELFDumperStyle->printVersionSymbolSection(ObjF->getELFFile(),
+                                            dot_gnu_version_sec);
 
   // Dump version definition section.
-  printVersionDefinitionSection(this, ObjF->getELFFile(), dot_gnu_version_d_sec, W);
+  ELFDumperStyle->printVersionDefinitionSection(ObjF->getELFFile(),
+                                                dot_gnu_version_d_sec);
 
   // Dump version dependency section.
-  printVersionDependencySection(this, ObjF->getELFFile(), dot_gnu_version_r_sec, W);
+  ELFDumperStyle->printVersionDependencySection(ObjF->getELFFile(),
+                                                dot_gnu_version_r_sec);
 }
 
 template <typename ELFT>
 StringRef ELFDumper<ELFT>::getSymbolVersion(StringRef StrTab,
                                             const Elf_Sym *symb,
                                             bool &IsDefault) const {
   // This is a dynamic symbol. Look in the GNU symbol version table.
   if (!dot_gnu_version_sec) {
static const EnumEntry<unsigned> ElfOSABI[] = {
   {"OpenVMS",      "VMS - OpenVMS",        ELF::ELFOSABI_OPENVMS},
   {"NSK",          "HP - Non-Stop Kernel", ELF::ELFOSABI_NSK},
   {"AROS",         "AROS",                 ELF::ELFOSABI_AROS},
   {"FenixOS",      "FenixOS",              ELF::ELFOSABI_FENIXOS},
   {"CloudABI",     "CloudABI",             ELF::ELFOSABI_CLOUDABI},
   {"Standalone",   "Standalone App",       ELF::ELFOSABI_STANDALONE}
 };
 
+static const EnumEntry<unsigned> SymVersionFlags[] = {
+    {"Base", "BASE", VER_FLG_BASE},
+    {"Weak", "WEAK", VER_FLG_WEAK},
+    {"Info", "INFO", VER_FLG_INFO}};
+
 static const EnumEntry<unsigned> AMDGPUElfOSABI[] = {
   {"AMDGPU_HSA",    "AMDGPU - HSA",    ELF::ELFOSABI_AMDGPU_HSA},
   {"AMDGPU_PAL",    "AMDGPU - PAL",    ELF::ELFOSABI_AMDGPU_PAL},
   {"AMDGPU_MESA3D", "AMDGPU - MESA3D", ELF::ELFOSABI_AMDGPU_MESA3D}
 };
 
 static const EnumEntry<unsigned> ARMElfOSABI[] = {
   {"ARM", "ARM", ELF::ELFOSABI_ARM}
for (const Elf_Rel &Rel : DynPLTRelRegion.getAsArrayRef<Elf_Rel>()) {
       Rela.r_offset = Rel.r_offset;
       Rela.r_info = Rel.r_info;
       Rela.r_addend = 0;
       printDynamicRelocation(Obj, Rela, false);
     }
   }
 }
 
+template <class ELFT>
+void GNUStyle<ELFT>::printVersionSymbolSection(const ELFFile<ELFT> *Obj,
+                                               const Elf_Shdr *Sec) {
+  if (!Sec)
+    return;
+
+  StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
+  OS << "Dumper for " << SecName << " is not implemented\n";
+}
+
+template <class ELFT>
+void GNUStyle<ELFT>::printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
+                                                   const Elf_Shdr *Sec) {
+  if (!Sec)
+    return;
+
+  StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
+  OS << "Dumper for " << SecName << " is not implemented\n";
+}
+
+template <class ELFT>
+void GNUStyle<ELFT>::printVersionDependencySection(const ELFFile<ELFT> *Obj,
+                                                   const Elf_Shdr *Sec) {
+  if (!Sec)
+    return;
+
+  StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
+  OS << "Dumper for " << SecName << " is not implemented\n";
+}
+
 // Hash histogram shows  statistics of how efficient the hash was for the
 // dynamic symbol table. The table shows number of hash buckets for different
 // lengths of chains as absolute number and percentage of the total buckets.
 // Additionally cumulative coverage of symbols for each set of buckets.
 template <class ELFT>
 void GNUStyle<ELFT>::printHashHistogram(const ELFFile<ELFT> *Obj) {
   // Print histogram for .hash section
   if (const Elf_Hash *HashTable = this->dumper()->getHashTable()) {
for (const Elf_Phdr &Phdr : unwrapOrError(Obj->program_headers())) {
     W.printNumber("FileSize", Phdr.p_filesz);
     W.printNumber("MemSize", Phdr.p_memsz);
     W.printFlags("Flags", Phdr.p_flags, makeArrayRef(ElfSegmentFlags));
     W.printNumber("Alignment", Phdr.p_align);
   }
 }
 
 template <class ELFT>
+void LLVMStyle<ELFT>::printVersionSymbolSection(const ELFFile<ELFT> *Obj,
+                                                const Elf_Shdr *Sec) {
+  DictScope SS(W, "Version symbols");
+  if (!Sec)
+    return;
+
+  StringRef SecName = unwrapOrError(Obj->getSectionName(Sec));
+  W.printNumber("Section Name", SecName, Sec->sh_name);
+  W.printHex("Address", Sec->sh_addr);
+  W.printHex("Offset", Sec->sh_offset);
+  W.printNumber("Link", Sec->sh_link);
+
+  const uint8_t *VersymBuf = (const uint8_t *)Obj->base() + Sec->sh_offset;
+  const ELFDumper<ELFT> *Dumper = this->dumper();
+  StringRef StrTable = Dumper->getDynamicStringTable();
+
+  // Same number of entries in the dynamic symbol table (DT_SYMTAB).
+  ListScope Syms(W, "Symbols");
+  for (const Elf_Sym &Sym : Dumper->dynamic_symbols()) {
+    DictScope S(W, "Symbol");
+    const Elf_Versym *Versym = reinterpret_cast<const Elf_Versym *>(VersymBuf);
+    std::string FullSymbolName =
+        Dumper->getFullSymbolName(&Sym, StrTable, true /* IsDynamic */);
+    W.printNumber("Version", Versym->vs_index);
+    W.printString("Name", FullSymbolName);
+    VersymBuf += sizeof(Elf_Versym);
+  }
+}
+
+template <class ELFT>
+void LLVMStyle<ELFT>::printVersionDefinitionSection(const ELFFile<ELFT> *Obj,
+                                                    const Elf_Shdr *Sec) {
+  DictScope SD(W, "SHT_GNU_verdef");
+  if (!Sec)
+    return;
+
+  const uint8_t *SecStartAddress =
+      (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(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");
+
+    const Elf_Verdef *Verdef = reinterpret_cast<const Elf_Verdef *>(VerdefBuf);
+    DictScope Def(W, "Definition");
+    W.printNumber("Version", Verdef->vd_version);
+    W.printEnum("Flags", Verdef->vd_flags, makeArrayRef(SymVersionFlags));
+    W.printNumber("Index", Verdef->vd_ndx);
+    W.printNumber("Hash", Verdef->vd_hash);
+    W.printString("Name",
+                  StringRef((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((const char *)(Obj->base() + StrTab->sh_offset +
+                                             Verdaux->vda_name)));
+    }
+    VerdefBuf += Verdef->vd_next;
+  }
+}
+
+template <class ELFT>
+void LLVMStyle<ELFT>::printVersionDependencySection(const ELFFile<ELFT> *Obj,
+                                                    const Elf_Shdr *Sec) {
+  DictScope SD(W, "SHT_GNU_verneed");
+  if (!Sec)
+    return;
+
+  const uint8_t *SecData = (const uint8_t *)Obj->base() + Sec->sh_offset;
+  const Elf_Shdr *StrTab = unwrapOrError(Obj->getSection(Sec->sh_link));
+
+  const uint8_t *VerneedBuf = SecData;
+  unsigned VerneedNum = Sec->sh_info;
+  for (unsigned I = 0; I < VerneedNum; ++I) {
+    const Elf_Verneed *Verneed =
+        reinterpret_cast<const Elf_Verneed *>(VerneedBuf);
+    DictScope Entry(W, "Dependency");
+    W.printNumber("Version", Verneed->vn_version);
+    W.printNumber("Count", Verneed->vn_cnt);
+    W.printString("FileName",
+                  StringRef((const char *)(Obj->base() + StrTab->sh_offset +
+                                           Verneed->vn_file)));
+
+    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);
+      DictScope Entry(W, "Entry");
+      W.printNumber("Hash", Vernaux->vna_hash);
+      W.printEnum("Flags", Vernaux->vna_flags, makeArrayRef(SymVersionFlags));
+      W.printNumber("Index", Vernaux->vna_other);
+      W.printString("Name",
+                    StringRef((const char *)(Obj->base() + StrTab->sh_offset +
+                                             Vernaux->vna_name)));
+      VernauxBuf += Vernaux->vna_next;
+    }
+    VerneedBuf += Verneed->vn_next;
+  }
+}
+
+template <class ELFT>
 void LLVMStyle<ELFT>::printHashHistogram(const ELFFile<ELFT> *Obj) {
   W.startLine() << "Hash Histogram not implemented!\n";
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printCGProfile(const ELFFile<ELFT> *Obj) {
   ListScope L(W, "CGProfile");
   if (!this->dumper()->getDotCGProfileSec())