[PATCH] D118526: [llvm-objcopy][MachO] Ignore LC_LINKER_OPTION when redefining symbols

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexander-shaposhnikov added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/redefine-symbol.s:46
 
+.linker_option "-lc"
 .globl _func
----------------
keith wrote:
> alexander-shaposhnikov wrote:
> > @keith - this test is not a good place for adding various load commands.
> > I'd recommend to add a separate test (until we have a test that specifically verifies all the copied-over load commands) (and remove this directive from redefine-symbol.s)
> > (if  yaml supports  LC_LINKER_OPTION - yaml-based, if it doesn't - assembly-based)
> > (the test should invoke `llvm-objcopy` without any extra flags) 
> > 
> Thanks for the feedback! I submitted https://reviews.llvm.org/D119149 which I hope can be a starting point for that, let me know what you think.
> 
> For my context how do you choose between yaml vs asm for this kind of test?
There are a few considerations / factors here. I'll list some of them below (although there might be more):
1. in general, for objcopy etc most tests use YAML: (a) human-readable (b) support for both relocatable / non-relocatable binaries (c) more control over various elements of the binary
2. For Mach-O the support on ObjYAML's side is incomplete
3. For some situations YAML can be very verbose / inconvenient 
4. If a test uses llvm-mc, then it depends on the MC layer => the test might hypothetically break if there are changes there
(but this happens rarely) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118526