[PATCH] D122505: [llvm-objdump] --private-headers: change errors to warnings for dynamic section dumping

[James Henderson via Phabricator via llvm-commits]

jhenderson added a subscriber: grimar.
jhenderson added a comment.

Mostly looks good. Some small improvements to the testing suggested.



================
Comment at: llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test:1-3
+## An empty dynamic section is invalid. Test we report a warning instead of an
+## error. objcopy --only-keep-debug may produce an empty dynamic section, it is
+## excessive to use an error.
----------------
Generally, I think dumper tools should avoid emitting errors for individual files, because it prevents later files from being dumped. @grimar and I both in the past made significant improvements to llvm-readobj and other tools (in my case llvm-dwarfdump at least) to achieve this. I don't think the same attention has been applied to llvm-objdump.

I've therefore suggested a rewording of the comment.


================
Comment at: llvm/test/tools/llvm-objdump/ELF/invalid-phdr.test:8
 # INVALID-PHOFF-NEXT: warning: '[[FILE]]': unable to read program headers: program headers are longer than binary of size 280: e_phoff = 0xffffff, e_phnum = 0, e_phentsize = 0
-# INVALID-PHOFF-NEXT: error: '[[FILE]]': program headers are longer than binary of size 280: e_phoff = 0xffffff, e_phnum = 0, e_phentsize = 0
+# INVALID-PHOFF-NEXT: warning: '[[FILE]]': program headers are longer than binary of size 280: e_phoff = 0xffffff, e_phnum = 0, e_phentsize = 0
 
----------------
Might be worth a `CHECK-EMPTY` after this line now, to show nothing is actually printed.


================
Comment at: llvm/test/tools/llvm-objdump/ELF/program-headers.test:286
 # PHENTSIZE-NEXT: warning: '[[FILE]]': unable to read program headers: invalid e_phentsize: 1
-# PHENTSIZE-NEXT: error: '[[FILE]]': invalid e_phentsize: 1
+# PHENTSIZE-NEXT: warning: '[[FILE]]': invalid e_phentsize: 1
 
----------------
Ditto.


================
Comment at: llvm/test/tools/llvm-objdump/ELF/program-headers.test:317
 # PHOFF-NEXT: warning: '[[FILE]]': unable to read program headers: program headers are longer than binary of size 408: e_phoff = [[OFF]], e_phnum = 1, e_phentsize = 56
-# PHOFF-NEXT: error: '[[FILE]]': program headers are longer than binary of size 408: e_phoff = [[OFF]], e_phnum = 1, e_phentsize = 56
+# PHOFF-NEXT: warning: '[[FILE]]': program headers are longer than binary of size 408: e_phoff = [[OFF]], e_phnum = 1, e_phentsize = 56
 
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122505