[PATCH] D87899: [llvm-readobj/elf] - Stop reporting invalid extended indexes in warnings for unnamed section symbols.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test:365-370
+# VERSIONED-SEC-SYM-XINDEX-LLVM:       Name: (0)
+# VERSIONED-SEC-SYM-XINDEX-LLVM:       Name: foo (12)
+# VERSIONED-SEC-SYM-XINDEX-LLVM:      warning: '[[FILE]]': extended symbol index (2) is past the end of the SHT_SYMTAB_SHNDX section of size 0
+# VERSIONED-SEC-SYM-XINDEX-LLVM-NEXT: Symbol {
+# VERSIONED-SEC-SYM-XINDEX-LLVM-NEXT:  Name: <?> (0)
+# VERSIONED-SEC-SYM-XINDEX-LLVM:       Name: <?> (0)
----------------
Do we need all these checks for the test purpose? If so, why do we need comparatively fewer in the GNU version?


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:5846
       const Elf_Sym *Sym = Parser.getGotSym(&E);
+      Elf_Sym_Range DynSyms = this->dumper()->dynamic_symbols();
       std::string SymName = this->dumper()->getFullSymbolName(
----------------
Maybe just get the Elf_Sym here, and pass that in? I also think you should do `[0]` as it's more explicit than `.data()`.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:6392
+    W.printString("Name", this->dumper()->getFullSymbolName(
+                              &Syms[I], Syms.data(), StrTable,
+                              /*IsDynamic=*/true));
----------------
For the same reason.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:6779
       std::string SymName = this->dumper()->getFullSymbolName(
-          Sym, this->dumper()->getDynamicStringTable(), true);
+          Sym, Parser.getGotDynSyms().data(),
+          this->dumper()->getDynamicStringTable(), true);
----------------
Ditto.


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

https://reviews.llvm.org/D87899