Page MenuHomePhabricator

Unconditionally pass `-lto_library` to the linker on Darwin
ClosedPublic

Authored by mehdi_amini on Oct 24 2016, 9:43 PM.

Details

Summary

We're only doing it with -flto currently, however it never "hurt"
to pass it, and users that are linking without -flto can get in
trouble if one of the dependency (a static library for instance)
contains bitcode.

Diff Detail

Repository
rL LLVM

Event Timeline

mehdi_amini retitled this revision from to Unconditionally pass `-lto_library` to the linker on Darwin.
mehdi_amini updated this object.
mehdi_amini added a reviewer: dexonsmith.
mehdi_amini added a subscriber: cfe-commits.
This revision was automatically updated to reflect the committed changes.
jwhowarth reopened this revision.Oct 27 2016, 10:49 AM
jwhowarth added a subscriber: jwhowarth.

This approach doesn't work if the user installs clang in a buried subdirectory such as /sw/opt/llvm-4.0 but accesses the compilers via a /sw/bin/clang-4.0 symlink pointing at /sw/opt/llvm-4.0/bin/clang-4.0...

$ /sw/bin/clang-4.0 -flto himenoBMTxpa.c
clang-4.0: warning: libLTO.dylib relative to clang installed dir not found; using 'ld' default search path instead [-Wliblto]
...
"/usr/bin/ld" -demangle -object_path_lto /var/folders/vh/xthx1f251nqfj804049zl1wm0000gn/T/cc-083c59.o -no_deduplicate -dynamic -arch x86_64 -macosx_version_min 10.11.0 -o a.out /var/folders/vh/xthx1f251nqfj804049zl1wm0000gn/T/himenoBMTxpa-34b8ea.o -lSystem /sw/opt/llvm-4.0/bin/../lib/clang/4.0.0/lib/darwin/libclang_rt.osx.a

$ /sw/opt/llvm-4.0/bin/clang-4.0 -flto himenoBMTxpa.c -v
...
"/usr/bin/ld" -demangle -object_path_lto /var/folders/vh/xthx1f251nqfj804049zl1wm0000gn/T/cc-afbbef.o -lto_library /sw/opt/llvm-4.0/lib/libLTO.dylib -no_deduplicate -dynamic -arch x86_64 -macosx_version_min 10.11.0 -o a.out /var/folders/vh/xthx1f251nqfj804049zl1wm0000gn/T/himenoBMTxpa-44b09e.o -lSystem /sw/opt/llvm-4.0/bin/../lib/clang/4.0.0/lib/darwin/libclang_rt.osx.a

Since you re-open this revision, can you clarify how this is triggering the bug you mention? It seems like the issue already existed?

I just verified that I reproduce with -flto and an previous clang version.

Opened https://llvm.org/bugs/show_bug.cgi?id=30811 since the inability for symlinks to be handled in locating libLTO.dylib defeats the purpose of this commit.

mehdi_amini accepted this revision.Oct 27 2016, 11:06 AM
mehdi_amini added a reviewer: mehdi_amini.
This revision is now accepted and ready to land.Oct 27 2016, 11:06 AM
mehdi_amini closed this revision.Oct 27 2016, 11:06 AM

I just verified that I reproduce with -flto and an previous clang version.

This issue will only be triggered if you build with "-DCMAKE_INSTALL_PREFIX:PATH=/sw/opt/llvm-4.0" to place llvm in a buried subdirectory and access the compilers with symlinks in /sw/bin pointing at the actual compilers in /sw/opt/llvm-4.0/bin. The call to llvm::sys::path::parent_path(D.getInstalledDir()) incorrectly places the install level at /sw instead of the actual sw/opt/llvm-4.0.

I reproduced with clang that ships with XCode by doing:

    ln -s `xcrun -find clang` /tmp/clang
    /tmp/clang /tmp/main.cpp  -flto
    clang: warning: libLTO.dylib relative to clang installed dir not found; using 'ld' default search path instead
`
rnk added a subscriber: rnk.Nov 21 2016, 1:17 PM

This added a bunch of warnings to the Chromium build, which doesn't use LTO today, so we had to revert our clang roll:
https://codereview.chromium.org/2520453002/

We currently don't use LTO on Mac, so we don't build or package the LTO plugin, and in the near term aren't planning to change that. For now, we really want the previous behavior: if the linker encounters a .bc file, it should error. How can we get that behavior today?

We ship clang + libLTO + ld64 bundled in the toolchain, so even if you don't package libLTO yourself, it is already accessible from the linker: it will use the one in the toolchain when needed.

I don't have an immediate idea to prevent this and have the linker issue an error (other than removing manually libLTO from the Xcode installation).

This is the motivation behind the warning I believe: we're trying to prevent this situation where a user would have clang but not his own libLTO and may encounter an unexpected issue. Even when the user does not opt-in to build his own project with LTO enabled, there can be static archive dependencies that contain bitcode.

rnk added a comment.Nov 21 2016, 2:11 PM

We ship clang + libLTO + ld64 bundled in the toolchain, so even if you don't package libLTO yourself, it is already accessible from the linker: it will use the one in the toolchain when needed.

I don't have an immediate idea to prevent this and have the linker issue an error (other than removing manually libLTO from the Xcode installation).

So, even if clang doesn't pass -lto_library to ld64, ld64 will auto-discover the bundled libLTO that happens to be next to it? That could go badly.

This is the motivation behind the warning I believe: we're trying to prevent this situation where a user would have clang but not his own libLTO and may encounter an unexpected issue. Even when the user does not opt-in to build his own project with LTO enabled, there can be static archive dependencies that contain bitcode.

I guess ld64 doesn't expose a flag like -fno-lto to disable this completely? Maybe we could pass -lto_library /dev/null or something?

In D25932#601842, @rnk wrote:

We ship clang + libLTO + ld64 bundled in the toolchain, so even if you don't package libLTO yourself, it is already accessible from the linker: it will use the one in the toolchain when needed.

I don't have an immediate idea to prevent this and have the linker issue an error (other than removing manually libLTO from the Xcode installation).

So, even if clang doesn't pass -lto_library to ld64, ld64 will auto-discover the bundled libLTO that happens to be next to it? That could go badly.

Right: until LLVM 3.8, clang was *never* passing the -lto_library option. The only way to have your own libLTO used by ld64 instead of the one in the Xcode toolchain was setting the environment variable "DYLD_LIBRARY_PATH".
Of course was has many issues, and that's what lead us to have clang passing this option to ld64. Initially only when the driver was invoked with -flto, but recently I had issues with clients that didn't use LTO themselves but were having static archives they depend on that were containing bitcode.

This is the motivation behind the warning I believe: we're trying to prevent this situation where a user would have clang but not his own libLTO and may encounter an unexpected issue. Even when the user does not opt-in to build his own project with LTO enabled, there can be static archive dependencies that contain bitcode.

I guess ld64 doesn't expose a flag like -fno-lto to disable this completely? Maybe we could pass -lto_library /dev/null or something?

No there is no such flag, what you suggest seems to work:

$ clang -flto main.c -Wl,-lto_library,/dev/null
ld: could not process llvm bitcode object file, because /dev/null could not be loaded file '/var/folders/4z/k9mg8rls7wsfm6t6hbm0_bxw0000gn/T/main-8e28a4.o' for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Also with an non-existing path:

$ clang -flto main.c -Wl,-lto_library,imaginary_path
ld: could not process llvm bitcode object file, because imaginary_path could not be loaded file '/var/folders/4z/k9mg8rls7wsfm6t6hbm0_bxw0000gn/T/main-319618.o' for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
rnk added a comment.Nov 21 2016, 2:29 PM

Maybe we should touch ../lib/libLTO.dylib relative to clang in our package and just let the loader fail if a bitcode file comes along.

rnk added a comment.Nov 21 2016, 3:14 PM

What if we only warn if D.isUsingLTO()? Things are only going to go wrong if *this* version of clang generates LLVM IR not understood by the system libLTO. Presumably bitcode from static archives will work with the system version of libLTO.