[PATCH] D73560: [llvm-readobj] - Don't crash when dumping invalid dynamic relocation.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test:383-384
+# LLVM-NO-DYNSYM:      Dynamic Relocations {
+# LLVM-NO-DYNSYM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 0: no dynamic symbol table found
+# LLVM-NO-DYNSYM-NEXT:   0x0 R_X86_64_NONE <corrupt> 0x0
+# LLVM-NO-DYNSYM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 1: no dynamic symbol table found
----------------
Do we have a test case somewhere that shows that relocations without symbols (e.g. R_X86_64_RELATIVE) are handled fine without warning?


================
Comment at: llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test:428
+
+## Show we print a warning when symbol index of a dynamic relocation is too
+## large (goes past the end of the dynamic symbol table).
----------------
when the symbol index...


================
Comment at: llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test:435
+# LLVM-INVALID-DYNSYM:      Dynamic Relocations {
+# LLVM-INVALID-DYNSYM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 2: index is larger or equal to the number of dynamic symbols (2)
+# LLVM-INVALID-DYNSYM-NEXT:   0x0 R_X86_64_NONE <corrupt> 0x0
----------------
larger or equal to -> greater than or equal to

(that's the more common way of saying '>=')


================
Comment at: llvm/test/tools/llvm-readobj/ELF/dynamic-reloc-no-section-headers.test:1
+## Test that we are able to print dynamic relocations with --dyn-relocations even when there are no section headers.
+
----------------
May be worth splitting this line into two.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:4006
+  // Symbols is zero, because there is no way to know the size of the dynamic
+  // table. We should allow this case and don't print a warning.
+  if (!Symbols.empty() && SymIndex >= Symbols.size())
----------------
don't -> not


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

https://reviews.llvm.org/D73560