[PATCH] D112450: support xcoff for llvm-nm

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-nm/XCOFF/basic.test:1
+## Test llvm-nm for XCOFF object files.
+# RUN: llvm-nm  %p/Inputs/test_xlclang.o | FileCheck --check-prefix=NM-SYM %s
----------------
Add to this comment what exact behaviours for XCOFF objects and llvm-nm you are testing, e.g. showing that llvm-nm uses can identify the symbols in such an object file and provide appropriate letter codes for them.


================
Comment at: llvm/test/tools/llvm-nm/XCOFF/basic.test:17
+
+# NM-SYM:      00000000 t
+# NM-SYM-NEXT: 00000040 T ._Z5func1i
----------------
I'd recommend adding --match-full-lines to FileCheck, to show there's no garbage after symbol names (or in this particular case, that there's no name at all).


================
Comment at: llvm/test/tools/llvm-nm/XCOFF/basic.test:24-25
+# NM-SYM-NEXT: 00000000 t .text
+
+# NM-SYM:      00000104 d _Z5func1i
+# NM-SYM-NEXT: 0000010c D _Z5func1i
----------------
DiggerLin wrote:
> jhenderson wrote:
> > Why are you not using check-next here? Are there symbols you are deliberately ignoring in this case?
> yes,  for the convenience to review.  there are some symbols are ignore in the case. 
This then raises the question of why are those symbols in the input?

I'm guessing it's so you can test other test cases without having multiple checked-in objects, but that just highlights one of the problems with canned binaries: you cannot tailor them to the individual test case, if you want to avoid having lots to them. I think this just shows you should focus on helping @Esme get the yaml2obj work finished, above getting wider support for XCOFF in the tools.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112450