[PATCH] D54674: [llvm-objcopy] First bits for MachO

Thu Nov 29 20:30:07 PST 2018

alexshap marked an inline comment as done.
alexshap added inline comments.

================
Comment at: test/tools/llvm-objcopy/MachO/basic-big-endian-32-copy.test:116
+# CHECK: --- !mach-o
+# CHECK: FileHeader:
+# CHECK:  magic:           0xFEEDFACE
----------------
rupprecht wrote:
> Can you also verify things coming from sections/symbol tables/etc? It seems this only tests that the FileHeader is preserved and everything else could be dropped...
there is a problem with obj2yaml / yaml2obj - the support for MachO is incomplete, some things (like the content of the sections) are not dumped, maybe smth else is missing too. But in these tests (in this patch) where we copy the object files without modifications the real check is done on the line 3: cmp %t %t2, 
I've added FileCheck just to be sure that yaml2obj has done the right thing, but probably I should remove it here to avoid confusions.
Regarding your other comment (about making use of llvm-readobj for tests) -
yeah, I think llvm-readobj should work for some scenarios, although with llvm-readobj there is an issue - it doesn't print all the mach-o specific bits (i.e. all the load commands). 
So basically my proposal for this: in these tests cmp should be sufficient, later I will have to use yaml2obj & llvm-readobj to test the changes of the load commands, when I run into an issue that smth is not printed by the existing tools I will try to extend obj2yaml/llvm-readobj to support it. 

Repository:
  rL LLVM

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

https://reviews.llvm.org/D54674