[llvm-objdump] Add `Version References` dumper

[Higuoxing]

Summary

Add symbol version dumper for #30241

[Higuoxing]

[llvm-objdump] Add Version References dumper (PR30241)

formatted

[Higuoxing]

tools/llvm-objdump/ELFDump.cpp

343–363

Seems that these codes are redundant, can we simplify this?

[Higuoxing]

Updating D54697: [llvm-objdump] Add Version References dumper (PR30241)

Rename some variables

[Higuoxing]

Updating D54697: [llvm-objdump] Add Version References dumper (PR30241)

[jhenderson]

Could you add tests for the cases where a) there is no DT_VERNEEDNUM tag, and b) where it has a value of 0, please.

Could you also point me at some documentation describing the VerNeed structs etc.

test/tools/llvm-objdump/private-headers-symbol-version.test

8 ↗

(On Diff #174605)

Nit: Spurious blank line?

tools/llvm-objdump/ELFDump.cpp

271

printSymbolVersionRef -> printSymbolVersionRefs or printSymbolVersionReferences

273

Why not use using here?

280

d_val's type is different in 32 bit and 64-bit ELF, so this should be a type large enough to hold either version (i.e. uin64_t).

288–290

Does GNU objdump print the header if there is a DT_VERNEEDNUM tag with a value of 0? I feel like it would be reasonable to do so in our case.

296

You can use ELFT::Elf_Shdr here and elsewhere directly, and avoid the need for the typename.

I'm also not sure what "GNUVerDepSec" means, so that probably needs expanding a bit more (especially the "Dep" part).

304

Rather than having this big if statement, do

if (GnuVerDepSec != nullptr)
  return;

306–307

I'm trying to understand the structure of the VerNeed stuff. Is it a variable size block, and hence the need to iterate over bytes rather than struct instances?

Also, prefer reinterpret_cast instead of C-style, here and elsewhere in this function.

311

unsigned -> uint64_t to match my earlier comment.

Maybe worth replacing I with VerNeedIndex or something, and J below similarly, just to disambiguate them a bit more.

317

unsigned is probably the wrong type. Use the type of vn_cnt or just the largest uint*_t type needed.

343–363

I'm not sure I follow what you mean?

tools/llvm-objdump/llvm-objdump.cpp

1887

put the return on a new line, since printELFSymbolVersionRef doesn't return anything.

[Higuoxing]

In D54697#1304044, @jhenderson wrote:

Could you add tests for the cases where a) there is no DT_VERNEEDNUM tag, and b) where it has a value of 0, please.

Sure, I will add them

Could you also point me at some documentation describing the VerNeed structs etc.

https://refspecs.linuxfoundation.org/LSB_2.1.0/LSB-Core-generic/LSB-Core-generic/symverdefs.html
https://refspecs.linuxfoundation.org/LSB_2.1.0/LSB-Core-generic/LSB-Core-generic/symverrqmts.html

PS. Thanks for your kind reviewing, I will cover them one by one :)

[Higuoxing]

Updating D54697: [llvm-objdump] Add Version References dumper (PR30241)
Partially refactored some codes

[Higuoxing]

Updating D54697: [llvm-objdump] Add Version References dumper (PR30241)

delete #include "llvm/Object/ELFTypes.h"

[Higuoxing]

tools/llvm-objdump/ELFDump.cpp

273

I use typedef here, just like what they do in other functions.

343–363

I mean, these functions are similar (they all cast ELF<32|64><BE|LE> to ElfObj), can we simplify these functions?

printELFFileHeader
printELFDynamicSection
printELFSymbolVersionRefs

[Higuoxing]

tools/llvm-objdump/ELFDump.cpp

296

I change it to VerNeedSec

[Higuoxing]

I want to use YAML2Obj to generate the test files, but there are something wrong with that tool. So, I have to write a patch for that. This patch have to be suspended for some time, until that one landed ...

[Higuoxing]

Add dumper for symbol version definitions.

[jhenderson]

Hi @Higuoxing. Is this ready for review again? It is currently marked as WIP/changes planned.

[Higuoxing]

In D54697#1332797, @jhenderson wrote:

Hi @Higuoxing. Is this ready for review again? It is currently marked as WIP/changes planned.

This works, but there are some little refactors needed. Besides, I cannot give enough test cases right now ...

[Higuoxing]

(WIP) This is a backup.

[Higuoxing]

Sorry for not responding on this patch for 2 months. Could anyone review this? Thanks a lot.

[Higuoxing]

update

[Higuoxing]

Updating D54697: [llvm-objdump] Add Version References dumper

[jhenderson]

I'll take a look at this next week, if nobody else does in the meantime.

[Higuoxing]

Updating D54697: [llvm-objdump] Add Version References dumper

[grimar]

I did not see this patch earlier and so worked on something very close to this for some time. I stopped my work
and shared the ideas/code I have in the comments here :)

tools/llvm-objdump/ELFDump.cpp

288

If you move this loop to printSymbolVersionInfo and change the printSymbolVersionDependency signature
to printSymbolVersionDependency(const typename ELFT::Shdr &Shdr, ArrayRef<uint8_t> Contents)
that probably will be better, because for printSymbolVersionDefinition implementation you would also
need to iterate over all sections and take the content. That can be done in a single place.

I mean the code in printSymbolVersionInfo can be like:

template <class ELFT>
static void printSymbolVersionInfo(const ELFFile<ELFT> *Elf) {
  typedef typename ELFT::Shdr Elf_Shdr;

  auto SecOrErr = Elf->sections();
  if (!SecOrErr)
    report_fatal_error(errorToErrorCode(SecOrErr.takeError()).message());

  for (const Elf_Shdr &Shdr : *SecOrErr) {
    if (Shdr.sh_type != ELF::SHT_GNU_verdef &&
        Shdr.sh_type != ELF::SHT_GNU_verneed)
      continue;

    auto ContentsOrErr = Elf->getSectionContents(&Shdr);
    if (!ContentsOrErr)
      report_fatal_error(errorToErrorCode(ContentsOrErr.takeError()).message());

    if (Shdr.sh_type == ELF::SHT_GNU_verdef)
      printSymbolVersionDefinition<ELFT>(Shdr, *ContentsOrErr);
    else if (Shdr.sh_type == ELF::SHT_GNU_verneed)
      printSymbolVersionDependency<ELFT>(Shdr, *ContentsOrErr);
  }
}

294

Actually, you do not need this I think.
You can simply iterate over Elf_Verneed entries until vn_next != 0.

300

I would use getSectionContents (see sample above), because it does more checks.

316

The same, I think, you can simply iterate until vna_next != 0.

[grimar]

test/tools/llvm-objdump/verneed-wrong-info.test

6

The case looks simple, so I would remove the link and just describe it.
i.e. something like:

"We have a SHT_GNU_verneed section with a broken sh_info field that says the section
contains more entries that it actually have"

[Higuoxing]

tools/llvm-objdump/ELFDump.cpp

288

Oh, thank you @grimar , I am wandering that whether the dumping order of verneed and verdef sections is important? Sometimes, the verneed section is before verdef and sometimes verneed is after verdef.

[Higuoxing]

I found that elf2yaml codes could be used here, so I just do some copy and paste work.

I don't know if we need some checks on sh_info field and DT_VERNEED, just like what gnu-objdump do.

[grimar]

test/tools/llvm-objdump/verneed-wrong-info.test

6

that->than, have->has (sorry, did these mistakes/mistypes when suggested it).

I hope @jhenderson will re-check it though :)

tools/llvm-objdump/ELFDump.cpp

288

I did not notice that, but if so then I guess GNU objdump just print them in the same order as they are in a section table probably.
i.e. the same is done in this patch.

303

Shouldn't you be able to get Filename from Elf?

313

If you're not going to implement SHT_GNU_verneed in this patch then I think you should
get rid of Shdr.sh_type != ELF::SHT_GNU_verneed.

[grimar]

tools/llvm-objdump/ELFDump.cpp

313

I meant "if you're not going to implement ELF::SHT_GNU_verdef" of course..

[Higuoxing]

Updating D54697: [llvm-objdump] Add Version References dumper

[Higuoxing]

tools/llvm-objdump/ELFDump.cpp

303

Sorry, I cannot find a getFileName() method in Elf or something else like that.

313

Thanks for reviewing, I would like to implement it in next patch :)

[grimar]

This looks fine, thanks! (with a minor nit addressed)
Let's see if anyone else has comments.

tools/llvm-objdump/ELFDump.cpp

303

Yeah, I see now. Nevermind, I looked at the wrong class I think.

326

You do not need this if, this condition is always true now.

[jhenderson]

test/tools/llvm-objdump/verneed-wrong-info.test

6

Looks good to me here :)

tools/llvm-objdump/ELFDump.cpp

292

Use PRIu16, not "u" here.

326

You don't need this if as the early continue above means that this if will always be true.

343–363

You're welcome to try some sort of refactor, but do it in another patch.

[Higuoxing]

Updating D54697: [llvm-objdump] Add Version References dumper

[Higuoxing]

Thanks a lot!

[jhenderson]

LGTM, but please wait for @grimar to confirm he is happy.

[grimar]

LGTM.

test/tools/llvm-objdump/verneed-elf.test

+# RUN: yaml2obj %s > %t
+# RUN: llvm-objdump -p %t | FileCheck %s
+
+# CHECK:      Version References:
+# CHECK-NEXT:   required from dso.so.0:
+# CHECK-NEXT:     0x000004d2 0x0a 03 v1
+# CHECK-NEXT:     0x0000162e 0x0b 04 v2
+# CHECK-NEXT:   required from dso.so.1:
+# CHECK-NEXT:     0x000011d7 0x0c 02 v3
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+  Entry:           0x0000000000201000
+Sections:
+  - Name:            .gnu.version_r
+    Type:            SHT_GNU_verneed
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x0000000000200250
+    Link:            .dynstr
+    AddressAlign:    0x0000000000000004
+    Info:            0x0000000000000002
+    Dependencies:
+      - Version:         1
+        File:            dso.so.0
+        Entries:
+          - Name:            v1
+            Hash:            1234
+            Flags:           10
+            Other:           3
+          - Name:            v2
+            Hash:            5678
+            Flags:           11
+            Other:           4
+      - Version:         1
+        File:            dso.so.1
+        Entries:
+          - Name:            v3
+            Hash:            4567
+            Flags:           12
+            Other:           2
+DynamicSymbols:
+  Global:
+    - Name:            f1

test/tools/llvm-objdump/verneed-wrong-info.test

+# RUN: yaml2obj %s > %t
+# RUN: llvm-objdump -p %t 2>&1 | FileCheck %s
+
+# We have a SHT_GNU_verneed section with a broken sh_info field
+# that says the section contains more entries than it actually has.
+
+# CHECK:      Version References:
+# CHECK-NEXT:   required from dso.so.0:
+# CHECK-NEXT:     0x000004d2 0x0a 03 v1
+# CHECK-NEXT:     0x0000162e 0x0b 04 v2
+# CHECK-NEXT:   required from dso.so.1:
+# CHECK-NEXT:     0x000011d7 0x0c 02 v3
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+  Entry:           0x0000000000201000
+Sections:
+  - Name:            .gnu.version_r
+    Type:            SHT_GNU_verneed
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x0000000000200250
+    Link:            .dynstr
+    AddressAlign:    0x0000000000000004
+    Info:            0x0000000000009999
+    Dependencies:
+      - Version:         1
+        File:            dso.so.0
+        Entries:
+          - Name:            v1
+            Hash:            1234
+            Flags:           10
+            Other:           3
+          - Name:            v2
+            Hash:            5678
+            Flags:           11
+            Other:           4
+      - Version:         1
+        File:            dso.so.1
+        Entries:
+          - Name:            v3
+            Hash:            4567
+            Flags:           12
+            Other:           2
+DynamicSymbols:
+  Global:
+    - Name:            f1

tools/llvm-objdump/ELFDump.cpp

outs() << "off    " << format(Fmt, (uint64_t)Phdr.p_offset) << "vaddr "
            << "memsz " << format(Fmt, (uint64_t)Phdr.p_memsz) << "flags "
            << ((Phdr.p_flags & ELF::PF_R) ? "r" : "-")
            << ((Phdr.p_flags & ELF::PF_W) ? "w" : "-")
            << ((Phdr.p_flags & ELF::PF_X) ? "x" : "-") << "\n";
   }
   outs() << "\n";
 }
 
+template <class ELFT>
+void printSymbolVersionDependency(ArrayRef<uint8_t> Contents,
+                                  StringRef StrTab) {
+  typedef ELFFile<ELFT> ELFO;
+  typedef typename ELFO::Elf_Verneed Elf_Verneed;
+  typedef typename ELFO::Elf_Vernaux Elf_Vernaux;
+
+  outs() << "Version References:\n";
+
+  const uint8_t *Buf = Contents.data();
+  while (Buf) {
+    const Elf_Verneed *Verneed = reinterpret_cast<const Elf_Verneed *>(Buf);
+    outs() << "  required from "
+           << StringRef(StrTab.drop_front(Verneed->vn_file).data()) << ":\n";
+
+    const uint8_t *BufAux = Buf + Verneed->vn_aux;
+    while (BufAux) {
+      const Elf_Vernaux *Vernaux =
+          reinterpret_cast<const Elf_Vernaux *>(BufAux);
+      outs() << "    "
+             << format("0x%08" PRIx32 " ", (uint32_t)Vernaux->vna_hash)
+             << format("0x%02" PRIx16 " ", (uint16_t)Vernaux->vna_flags)
+             << format("%02u"
+                       " ",
+                       (uint16_t)Vernaux->vna_other)
+             << StringRef(StrTab.drop_front(Vernaux->vna_name).data()) << '\n';
+      BufAux = Vernaux->vna_next ? BufAux + Vernaux->vna_next : nullptr;
+    }
+    Buf = Verneed->vn_next ? Buf + Verneed->vn_next : nullptr;
+  }
+}
+
+template <class ELFT>
+void printSymbolVersionInfo(const ELFFile<ELFT> *Elf, StringRef FileName) {
+  typedef typename ELFT::Shdr Elf_Shdr;
+
+  auto SectionsOrError = Elf->sections();
+  if (!SectionsOrError)
+    report_error(FileName, SectionsOrError.takeError());
+
+  for (const Elf_Shdr &Shdr : *SectionsOrError) {
+    if (Shdr.sh_type != ELF::SHT_GNU_verneed)
+      continue;
+
+    auto ContentsOrError = Elf->getSectionContents(&Shdr);
+    if (!ContentsOrError)
+      report_error(FileName, ContentsOrError.takeError());
+
+    auto StrTabSecOrError = Elf->getSection(Shdr.sh_link);
+    if (!StrTabSecOrError)
+      report_error(FileName, StrTabSecOrError.takeError());
+
+    auto StrTabOrError = Elf->getStringTable(*StrTabSecOrError);
+    if (!StrTabOrError)
+      report_error(FileName, StrTabOrError.takeError());
+
+    if (Shdr.sh_type == ELF::SHT_GNU_verneed)
+      printSymbolVersionDependency<ELFT>(*ContentsOrError, *StrTabOrError);
+    // TODO: Implement symbol version definitions dumper.
+  }
+}
+
 void llvm::printELFFileHeader(const object::ObjectFile *Obj) {
   if (const auto *ELFObj = dyn_cast<ELF32LEObjectFile>(Obj))
     printProgramHeaders(ELFObj->getELFFile());
   else if (const auto *ELFObj = dyn_cast<ELF32BEObjectFile>(Obj))
     printProgramHeaders(ELFObj->getELFFile());
   else if (const auto *ELFObj = dyn_cast<ELF64LEObjectFile>(Obj))
     printProgramHeaders(ELFObj->getELFFile());
   else if (const auto *ELFObj = dyn_cast<ELF64BEObjectFile>(Obj))
     printProgramHeaders(ELFObj->getELFFile());
 }
 
 void llvm::printELFDynamicSection(const object::ObjectFile *Obj) {
   if (const auto *ELFObj = dyn_cast<ELF32LEObjectFile>(Obj))
     printDynamicSection(ELFObj->getELFFile(), Obj->getFileName());
   else if (const auto *ELFObj = dyn_cast<ELF32BEObjectFile>(Obj))
     printDynamicSection(ELFObj->getELFFile(), Obj->getFileName());
   else if (const auto *ELFObj = dyn_cast<ELF64LEObjectFile>(Obj))
     printDynamicSection(ELFObj->getELFFile(), Obj->getFileName());
   else if (const auto *ELFObj = dyn_cast<ELF64BEObjectFile>(Obj))
     printDynamicSection(ELFObj->getELFFile(), Obj->getFileName());
 }
+
+void llvm::printELFSymbolVersionInfo(const object::ObjectFile *Obj) {
+  if (const auto *ELFObj = dyn_cast<ELF32LEObjectFile>(Obj))
+    printSymbolVersionInfo(ELFObj->getELFFile(), Obj->getFileName());
+  else if (const auto *ELFObj = dyn_cast<ELF32BEObjectFile>(Obj))
+    printSymbolVersionInfo(ELFObj->getELFFile(), Obj->getFileName());
+  else if (const auto *ELFObj = dyn_cast<ELF64LEObjectFile>(Obj))
+    printSymbolVersionInfo(ELFObj->getELFFile(), Obj->getFileName());
+  else if (const auto *ELFObj = dyn_cast<ELF64BEObjectFile>(Obj))
+    printSymbolVersionInfo(ELFObj->getELFFile(), Obj->getFileName());
+}

tools/llvm-objdump/llvm-objdump.h

 void printMachOUnwindInfo(const object::MachOObjectFile *O);
 void printMachOExportsTrie(const object::MachOObjectFile *O);
 void printMachORebaseTable(object::MachOObjectFile *O);
 void printMachOBindTable(object::MachOObjectFile *O);
 void printMachOLazyBindTable(object::MachOObjectFile *O);
 void printMachOWeakBindTable(object::MachOObjectFile *O);
 void printELFFileHeader(const object::ObjectFile *O);
 void printELFDynamicSection(const object::ObjectFile *Obj);
+void printELFSymbolVersionInfo(const object::ObjectFile *Obj);
 void printCOFFFileHeader(const object::ObjectFile *O);
 void printCOFFSymbolTable(const object::COFFImportFile *I);
 void printCOFFSymbolTable(const object::COFFObjectFile *O);
 void printMachOFileHeader(const object::ObjectFile *O);
 void printMachOLoadCommands(const object::ObjectFile *O);
 void printWasmFileHeader(const object::ObjectFile *O);
 void printExportsTrie(const object::ObjectFile *O);
 void printRebaseTable(object::ObjectFile *O);

tools/llvm-objdump/llvm-objdump.cpp

static void printFaultMaps(const ObjectFile *Obj) {
   error(FaultMapSection.getValue().getContents(FaultMapContents));
 
   FaultMapParser FMP(FaultMapContents.bytes_begin(),
                      FaultMapContents.bytes_end());
 
   outs() << FMP;
 }
 
 static void printPrivateFileHeaders(const ObjectFile *O, bool OnlyFirst) {
   if (O->isELF()) {
     printELFFileHeader(O);
-    return printELFDynamicSection(O);
+    printELFDynamicSection(O);
+    printELFSymbolVersionInfo(O);
+    return;
   }
   if (O->isCOFF())
     return printCOFFFileHeader(O);
   if (O->isWasm())
     return printWasmFileHeader(O);
   if (O->isMachO()) {
     printMachOFileHeader(O);
     if (!OnlyFirst)