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

[Jez Ng via Phabricator via llvm-commits]

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


================
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
----------------
thakis wrote:
> int3 wrote:
> > thakis wrote:
> > > 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.
> > (I meant to ask about mainfoo.o instead of libbar.a... thanks for noticing :p)
> > 
> > I think your example links fine because the `-force_load`s aren't being applied to two different paths. Having an archive duplicated multiple times on the command line isn't an error if it's not being force-loaded.
> > 
> > E.g. these fail with ld64:
> > 
> >     clang mainfoo.o libfoo.a -force_load libfoo.a -force_load ../tmp/libfoo.a -isysroot $(xcrun -show-sdk-path)
> >     clang mainfoo.o libfoo.a -force_load libfoo.a -force_load libbar.a -isysroot $(xcrun -show-sdk-path)
> Ah derp, of course. Sorry 😳
No worries :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104353