[PATCH] D82955: [llvm-readelf] - Do not error out when dumping symbols.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/symbols.test:126
+# RUN: llvm-readobj --symbols %t64.err1 2>&1 | \
+# RUN:  FileCheck %s -DFILE=%t64.err1 --check-prefix=STRTAB-LINK-ERR-LLVM
+# RUN: llvm-readelf --symbols %t64.err1 2>&1 | \
----------------
Nit: add an extra space for indentation of the command, here and below.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/symbols.test:132
+# STRTAB-LINK-ERR-LLVM-NEXT: warning: '[[FILE]]': unable to get the string table for the SHT_SYMTAB section: invalid section index: 255
+# STRTAB-LINK-ERR-LLVM-NEXT: warning: '[[FILE]]': st_name (0x0) is past the end of the string table of size 0x0
+# STRTAB-LINK-ERR-LLVM-NEXT:   Symbol {
----------------
I guess this is fine for now, but in an ideal world, I think we'd only print the first warning and not the second, since we already said we couldn't find the string table. In fact, in some ways the extra warning ("past the end of the string table") is slightly misleading - there is no string table at all, so you can't be "past the end" of it.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/symbols.test:192
+# SYMTAB-ENTSIZE-ERR-LLVM:      Symbols [
+# SYMTAB-ENTSIZE-ERR-LLVM-NEXT:  warning: '[[FILE]]': unable to read symbols from the SHT_SYMTAB section: section [index 1] has an invalid sh_entsize: 255
+# SYMTAB-ENTSIZE-ERR-LLVM:      ]
----------------
Small nit: I assume this warning should be indented one more space if it wants to match exactly the output.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/symbols.test:230
+# SYMTAB-SHSTRTAB-ERR-GNU:      warning: '[[FILE]]': unable to get the name of the SHT_SYMTAB section: section header string table index 255 does not exist
+# SYMTAB-SHSTRTAB-ERR-GNU:      Symbol table for image contains 2 entries:
+# SYMTAB-SHSTRTAB-ERR-GNU-NEXT:    Num:    Value          Size Type    Bind   Vis       Ndx Name
----------------
This is misleading. I think the intent is for "image" to be the dynamic symbol table derived from a dynamic table. A relocatable object isn't an "image" since that refers to the thing actually loaded.

Perhaps simply "object" or even "<?>" would be better in this context.


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

https://reviews.llvm.org/D82955