[PATCH] D84173: [llvm-readobj/readelf] - Don't fail dumping when unable to read the name of the SHT_DYNSYM section.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test:93
+# NOPHDRS-LLVM:      DynamicSymbols [
+# NOPHDRS-NAME-LLVM: warning: '[[FILE]]': unable to get the name of the SHT_DYNSYM section: a section [index 2] has an invalid sh_name (0xffffffff) offset which goes past the end of the section name string table
+# NOPHDRS-LLVM-NEXT:   Symbol {
----------------
jhenderson wrote:
> I'm not convinced we want this warning in the LLVM case - it doesn't actually need the name for anything, so emitting the warning doesn't make much sense.
> 
> Looking at the code, it looks like the SHT_SYMTAB name lookup has a similar problem? Could the name not be looked up inside the `printSymtabMessage` function?
> Looking at the code, it looks like the SHT_SYMTAB name lookup has a similar problem?

Yes.

> Could the name not be looked up inside the printSymtabMessage function?

Yes. It can be done in `GNUStyle<ELFT>::printSymtabMessage` instead of `printSymbolsHelper`.
But it should be fixed in a follow-up, I think, because the possible change relies on the `DotDynsymSec`
introduced in this patch. I will post the fix soon.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test:141-142
+# RUN:   FileCheck %s -DFILE=%t2.broken.name --check-prefixes=NOPHDRS-LLVM,NOPHDRS-NAME-LLVM
+# RUN: llvm-readelf %t2.broken.name --dyn-symbols 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t2.broken.name -DNAME="<?>" --check-prefix=NOPHDRS-GNU
 
----------------
jhenderson wrote:
> This doesn't check that the warning is emitted.
Fixed.


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

https://reviews.llvm.org/D84173