[PATCH] D54697: [llvm-objdump] Add `Version References` dumper

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

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



================
Comment at: 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);
  }
}
```




================
Comment at: tools/llvm-objdump/ELFDump.cpp:294
+  // we use sh_info to determine the entry of SHT_GNU_verneed.
+  if (VerNeedSec->sh_info == 0)
+    report_error(Filename, createError(".gnu.version_r invalid entry"));
----------------
Actually, you do not need this I think.
You can simply iterate over `Elf_Verneed` entries until `vn_next != 0`.


================
Comment at: tools/llvm-objdump/ELFDump.cpp:300
+  const auto *VerNeed =
+      reinterpret_cast<const uint8_t *>(Elf->base() + VerNeedSec->sh_offset);
+
----------------
I would use `getSectionContents` (see sample above), because it does more checks.


================
Comment at: tools/llvm-objdump/ELFDump.cpp:316
+    const auto *VerNeedAux = VerNeed + Need->vn_aux;
+    for (uint32_t VerNeedAuxIndex = 0; VerNeedAuxIndex < Need->vn_cnt;
+         ++VerNeedAuxIndex) {
----------------
The same, I think, you can simply iterate until `vna_next != 0`.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54697/new/

https://reviews.llvm.org/D54697