[PATCH] D112450: support xcoff for llvm-nm

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Let's keep this bare-bones for now, and split off the size extraction and name demangling into separate patches.



================
Comment at: llvm/test/tools/llvm-nm/XCOFF/xcoff_nm_print_symbol.test:1
+; Test the llvm-nm for xcoff object files.
+; RUN: llvm-nm  %p/Inputs/test_gcc.o | FileCheck --check-prefix=NM-SYM %s
----------------
`;;` (actually probably `##` and `#` for comments for YAML support). Also, should it be "XCOFF" in this comment?

Also, rename the test file to remove the "xcoff" and "nm" from the name. Both are implied by the file path. Given my out-of-line comment suggesting this patch be stripped back to just regular printing, with no options etc, I'd suggest naming it `basic.test`.


================
Comment at: llvm/test/tools/llvm-nm/XCOFF/xcoff_nm_print_symbol.test:2
+; Test the llvm-nm for xcoff object files.
+; RUN: llvm-nm  %p/Inputs/test_gcc.o | FileCheck --check-prefix=NM-SYM %s
+; RUN: llvm-nm  --print-size --demangle %p/Inputs/test_gcc.o | FileCheck --check-prefix=NM-SIZE-DEMANGLE %s
----------------
Can't we use yaml2obj here now? That would allow for much better targeted testing.


================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:938-941
+  if (!SymSecOrErr) {
+    consumeError(SymSecOrErr.takeError());
+    return '?';
+  }
----------------
I know that other formats throw away errors, but I would suggest that's a mistake - how is a user supposed to know why a given symbol results in `?`? I'd recommend reporting the `Error` as a warning, rather than just throwing it away.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112450