[PATCH] D65539: [llvm-objcopy][MachO] Implement a layout algorithm for executables/shared libraries

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Just a couple of minor comments from me. I don't know enough about Mach-O to be able to review the majority of this.



================
Comment at: llvm/test/tools/llvm-objcopy/MachO/basic-executable-copy.test:1
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy %t %t2
----------------
Add a comment to the top of this test saying what the purpose of the test is.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/basic-executable-copy.test:3
+# RUN: llvm-objcopy %t %t2
+# RUN: llvm-readobj --file-header --sections %t2 | FileCheck %s
+
----------------
--file-headers is the canonical option, I believe (we provide --file-header as an alias, but we should probably try to only use the real one here).


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/basic-executable-copy.test:5
+
+--- !mach-o
+FileHeader:
----------------
There's a lot going on in this YAML. Is it the minimal it can be to show all the behaviour, or can it be reduced further?


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/basic-shared-library-copy.test:1
+# RUN: yaml2obj %s > %t
+# RUN: llvm-objcopy %t %t2
----------------
Same comments as previous test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65539