[PATCH] D104353: [lld-macho] Avoid force-loading the same archive twice

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/test/MachO/force-load.s:26
+## symlinks and hardlinks as distinct inputs too.
+# RUN: ln -s %t/foo.a %t/bar.a
+# RUN: not %lld -lSystem %t/foo.o -force_load %t/foo.a -force_load %t/bar.a \
----------------
int3 wrote:
> MaskRay wrote:
> > This needs to unsupport testing on Windows .
> as the XXX comment above says, I'm not sure this is worth committing, especially since it doesn't allow us to run the test on Windows. I just uploaded it to show you and @thakis that ld64 behaves similarly. I think the relative path check on line 21 sufficiently demonstrates that `realpath()` isn't being called, and naturally so symlinks and hardlinks wouldn't get resolved either.
> 
> I don't feel strongly about this, so lmk if you would prefer these test cases to be committed.
We treat them as distinct inputs, but ld64 apparently doesn't (didn't test hardlinks, just symlinks). And we want to realpath() eventually, so as I said I wouldn't add these tests.


================
Comment at: lld/test/MachO/force-load.s:21
+## arguments, so this is an error.
+# RUN: cd %t; not %lld %t/foo.o -force_load %t/foo.a -force_load foo.a \
+# RUN:   %t/test.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=DUP
----------------
int3 wrote:
> thakis wrote:
> > Hm, should we have an explicit test for this? ld64 does seem to do realpath(), and we kind of want to do that at some point too. So it seems a bit strange to have a test preventing this.
> > 
> > (My local ld64 test:
> > ```
> > % ln -s libfoo.a libbar.a
> > % clang mainfoo.o libfoo.a -force_load libfoo.a -force_load libfoo.a -lfoo -L . libbar.a ../llvm-project/libfoo.a
> > ```
> > )
> What are the contents of `libfoo.a` and `libbar.a` here?
> 
> This force-load.s test passes for me when I swap out LLD for ld64. I also tried using an explicit symlink instead of a CWD-relative path, and that passes for me too (i.e. raises a duplicate symbol error.)
libfoo.a contained `void foo() {}` in compiled, and libbar.a is a symlink to libffo.a.  (and mainfoo.o is `void foo(); int main() { foo(); }`) This links fine for me with ld64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104353