[PATCH] D74823: [XCOFF/objdump] Fix crash in objdump when trying to use -r with XCOFF binaries

Mon Feb 24 07:35:44 PST 2020

jhenderson added a comment.

It's good that we don't crash, but I'm wondering why not just implement relocation printing for XCOFF? Also, is the intention to not print relocations mixed with disassembly only (i.e. -r + -d/D etc), or also relocation printing without disassembly (i.e. -r on its own)? If the latter, there needs to be a test for that too.

================
Comment at: llvm/test/tools/llvm-objdump/xcoff-disassemble-all.test:19
 CHECK: Inputs/xcoff-section-headers.o:	file format aixcoff-rs6000
+NO-RELOC-OPTION-NOT: -r/--relocs option not supported for XCOFF object files, ignoring option
+RELOC-OPTION: -r/--relocs option not supported for XCOFF object files, ignoring option
----------------
You could avoid the need for this -NOT check, by using CHECK-EMPTY/CHECK-NEXT lines to match between the previous CHECK and the CHECK: disassembly... lines. That way, you don't need to have two identical strings here.

================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2198
+
+  // Relocation processing is not yet implemented for XCOFF binaries
+  if(Relocations && isa<XCOFFObjectFile>(O)) {
----------------
Nit: missing trailing full stop.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74823