[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

239–259

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

Nit: Spurious blank line?

tools/llvm-objdump/ELFDump.cpp

162

printSymbolVersionRef -> printSymbolVersionRefs or printSymbolVersionReferences

164

Why not use using here?

171

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).

179–181

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.

187

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).

195

Rather than having this big if statement, do

if (GnuVerDepSec != nullptr)
  return;

197–198

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.

202

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.

208

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

239–259

I'm not sure I follow what you mean?

tools/llvm-objdump/llvm-objdump.cpp

2224

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

164

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

239–259

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

187

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

179

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);
  }
}

185

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

191

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

207

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

[grimar]

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

5 ↗

(On Diff #187942)

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

179

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

5 ↗

(On Diff #188074)

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

179

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.

194

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

204

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

204

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

194

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

204

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

194

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

217

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

[jhenderson]

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

5 ↗

(On Diff #188074)

Looks good to me here :)

tools/llvm-objdump/ELFDump.cpp

183

Use PRIu16, not "u" here.

217

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

239–259

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/private-headers-symbol-version.test

+# RUN: llvm-objdump -p %p/Inputs/private-headers-x86_64.elf | FileCheck %s
+
+CHECK:      Version References:
+CHECK-NEXT:   required from libstdc++.so.6:
+CHECK-NEXT:     0x08922974 0x00 0x03 GLIBCXX_3.4
+CHECK-NEXT:   required from libc.so.6:
+CHECK-NEXT:     0x09691a75 0x00 0x02 GLIBC_2.2.5
+

tools/llvm-objdump/ELFDump.cpp

if (Dyn.d_tag == ELF::DT_NEEDED) {
         outs() << (Data + Dyn.d_un.d_val) << "\n";
         continue;
       }
       warn(errorToErrorCode(StrTabOrErr.takeError()).message());
       consumeError(StrTabOrErr.takeError());
     }
     outs() << format(Fmt, (uint64_t)Dyn.d_un.d_val);
   }
+  outs() << "\n";
 }
 
 template <class ELFT> void printProgramHeaders(const ELFFile<ELFT> *o) {
   typedef ELFFile<ELFT> ELFO;
   outs() << "Program Header:\n";
   auto ProgramHeaderOrError = o->program_headers();
   if (!ProgramHeaderOrError)
     report_fatal_error(
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 printSymbolVersionRef(const ELFFile<ELFT> *Elf, StringRef Filename) {
+
+  typedef ELFFile<ELFT> ELFO;
+  using VerNeed = typename ELFO::Elf_Verneed;
+  using VernAux = typename ELFO::Elf_Vernaux;
+
+  auto DynamicEntriesOrError = Elf->dynamicEntries();
+  if (!DynamicEntriesOrError)
+    report_error(Filename, DynamicEntriesOrError.takeError());
+
+  unsigned VerNeedNum = 0;
+  for (const auto &Dyn : *DynamicEntriesOrError) {
+    if (Dyn.d_tag == ELF::DT_VERNEEDNUM) {
+      VerNeedNum = Dyn.d_un.d_val;
+      break;
+    }
+  }
+
+  if (VerNeedNum == 0)
+    return;
+  outs() << "Version References:\n";
+
+  auto SectionsOrError = Elf->sections();
+  if (!SectionsOrError)
+    report_error(Filename, SectionsOrError.takeError());
+
+  const typename ELFO::Elf_Shdr *GNUVerDepSec = nullptr;
+  for (const typename ELFO::Elf_Shdr &Sec : *SectionsOrError) {
+    if (Sec.sh_type == ELF::SHT_GNU_verneed) {
+      GNUVerDepSec = &Sec;
+      break;
+    }
+  }
+
+  if (GNUVerDepSec) {
+    const uint8_t *VNeed =
+        (const uint8_t *)Elf->base() + GNUVerDepSec->sh_offset;
+    auto SymTabOrErr = Elf->getSection(GNUVerDepSec->sh_link);
+    if (!SymTabOrErr)
+      report_error(Filename, SymTabOrErr.takeError());
+    const typename ELFO::Elf_Shdr *SymTab = *SymTabOrErr;
+    for (unsigned I = 0; I < VerNeedNum; ++I) {
+      const auto *Need = reinterpret_cast<const VerNeed *>(VNeed);
+      outs() << "  required from "
+             << StringRef((const char *)Elf->base() + SymTab->sh_offset +
+                          Need->vn_file)
+             << ":\n";
+      unsigned DepCnt = Need->vn_cnt;
+      const uint8_t *VAux = VNeed + Need->vn_aux;
+      for (unsigned J = 0; J < DepCnt; ++J) {
+        const auto *Aux = reinterpret_cast<const VernAux *>(VAux);
+        outs() << "    "
+               << format("0x%08" PRIx64 " ", (uint64_t)Aux->vna_hash)
+               << format("0x%02" PRIx64 " ", (uint64_t)Aux->vna_flags)
+               << format("0x%02" PRIx64 " ", (uint64_t)Aux->vna_other)
+               << StringRef((const char *)(Elf->base() + SymTab->sh_offset +
+                                           Aux->vna_name))
+               << "\n";
+        VAux += Aux->vna_next;
+      }
+      VNeed += Need->vn_next;
+    }
+    outs() << "\n";
+  }
+}
+
 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::printELFSymbolVersionRef(const object::ObjectFile *Obj) {
+  if (const auto *ELFObj = dyn_cast<ELF32LEObjectFile>(Obj))
+    printSymbolVersionRef(ELFObj->getELFFile(), Obj->getFileName());
+  else if (const auto *ELFObj = dyn_cast<ELF32BEObjectFile>(Obj))
+    printSymbolVersionRef(ELFObj->getELFFile(), Obj->getFileName());
+  else if (const auto *ELFObj = dyn_cast<ELF64LEObjectFile>(Obj))
+    printSymbolVersionRef(ELFObj->getELFFile(), Obj->getFileName());
+  else if (const auto *ELFObj = dyn_cast<ELF64BEObjectFile>(Obj))
+    printSymbolVersionRef(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 printELFSymbolVersionRef(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

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);
+    return printELFSymbolVersionRef(o);
   }
   if (o->isCOFF())
     return printCOFFFileHeader(o);
   if (o->isWasm())
     return printWasmFileHeader(o);
   if (o->isMachO()) {
     printMachOFileHeader(o);
     if (!onlyFirst)