[PATCH] D133751: [llvm-objdump] Change printSymbolVersionDependency to use ELFFile API

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Object/ELF.h:1041
     if (Verneed->vn_file < StrTab.size())
-      VN.File = std::string(StrTab.drop_front(Verneed->vn_file));
+      VN.File = std::string(StrTab.data() + Verneed->vn_file);
     else
----------------
Should this be a separate patch?


================
Comment at: llvm/test/tools/llvm-objdump/ELF/verneed-invalid.test:2
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objdump -p %t 2>&1 | FileCheck %s --check-prefix=BROKEN-AUX -DFILE=%t
+
----------------
Similarly, this test seems unrelated to the commit message?


================
Comment at: llvm/tools/llvm-objdump/ELFDump.cpp:301
 
-    const uint8_t *BufAux = Buf + Verneed->vn_aux;
-    while (BufAux) {
-      auto *Vernaux = reinterpret_cast<const typename ELFT::Vernaux *>(BufAux);
-      outs() << "    "
-             << format("0x%08" PRIx32 " ", (uint32_t)Vernaux->vna_hash)
-             << format("0x%02" PRIx16 " ", (uint16_t)Vernaux->vna_flags)
-             << format("%02" PRIu16 " ", (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;
+  raw_fd_ostream &OS = outs();
+  for (const VerNeed &VN : *V) {
----------------
Nit: this local variable seems a little bit unnecessary?


================
Comment at: llvm/tools/llvm-objdump/ELFDump.cpp:352-356
     ArrayRef<uint8_t> Contents =
         unwrapOrError(Elf.getSectionContents(Shdr), FileName);
     const typename ELFT::Shdr *StrTabSec =
         unwrapOrError(Elf.getSection(Shdr.sh_link), FileName);
     StringRef StrTab = unwrapOrError(Elf.getStringTable(*StrTabSec), FileName);
----------------
These variables are only used in the `else` part now. Could we move them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133751