[PATCH] D78342: [lld] Add archive file support to Mach-O backend

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added a comment.

Couple of nits but this mostly looks good. If you can make sure it's ASAN/UBSAN clean I can land it



================
Comment at: lld/test/MachO/Inputs/archive2.s:3
+_boo:
+  mov $2, %rax
+  ret
----------------
can we drop the `mov` commands in the archive input files even if we're not inlining them?


================
Comment at: lld/test/MachO/archive.s:18
+
+## Linking with the archive first in the command line shouldn't change anything
+# RUN: lld -flavor darwinnew %t/test.a %t/main.o -o %t/test.out
----------------
You could reuse the same CHECK commands instead of having a different prefix with the same checks -- that's what I did in my order file diff. But this is fine too


================
Comment at: lld/test/MachO/symbol-order.s:6
+# RUN: echo ".section __TEXT,test_f2; .global f"            | llvm-mc -filetype=obj -triple=x86_64-apple-darwin -o %t/f2.o
+# RUN: echo ".section __TEXT,test_fg; .global f; .global g" | llvm-mc -filetype=obj -triple=x86_64-apple-darwin -o %t/fg.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t/test.o
----------------
`.global` takes multiple variable names, so you can have `.globl f, g`. (You can also omit the 'a' in .global if you'd like)

Also, I think it's more conventional (and maybe more correct? dunno) to write it as ` .globl f, g; .section __TEXT,test_fg; f: g:` -- i.e. `.globl f` is a declaration and the definition determines which section it actually lands in. At least, it seems like the ELF tests don't omit the definition when defining these inputs


================
Comment at: lld/test/MachO/symbol-order.s:9
+
+## lld -flavor darwinnew
+# RUN: lld -flavor darwinnew -dylib -o %t/libf1.dylib %t/f1.o
----------------
intentional comment?


================
Comment at: lld/test/MachO/symbol-order.s:21
+# DYLIB-FIRST:      SYMBOL TABLE:
+# DYLIB-FIRST-DAG:  {{[0-9a-z]+}} g     O __TEXT,test_g g
+# DYLIB-FIRST:      Lazy bind table:
----------------
CHECK lines are substring matches, so this could just be `__TEXT,test_g g`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78342