Unconditionally pass `-lto_library` to the linker on Darwin

[mehdi_amini]

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.

[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

[mehdi_amini]

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

[mehdi_amini]

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

[jwhowarth]

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.

[jwhowarth]

In D25932#581167, @mehdi_amini wrote:

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.

[mehdi_amini]

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]

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?

[mehdi_amini]

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]

In D25932#601820, @mehdi_amini 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.

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?

[mehdi_amini]

In D25932#601842, @rnk wrote:

In D25932#601820, @mehdi_amini 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]

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]

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.

clang/lib/Driver/Tools.cpp

if (D.isUsingLTO()) {
     // dsymutil step.
     if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
       const char *TmpPath = C.getArgs().MakeArgString(
           D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object)));
       C.addTempFile(TmpPath);
       CmdArgs.push_back("-object_path_lto");
       CmdArgs.push_back(TmpPath);
     }
+  }
 
-    // Use -lto_library option to specify the libLTO.dylib path. Try to find
-    // it in clang installed libraries. If not found, the option is not used
-    // and 'ld' will use its default mechanism to search for libLTO.dylib.
-    if (Version[0] >= 133) {
-      // Search for libLTO in <InstalledDir>/../lib/libLTO.dylib
-      StringRef P = llvm::sys::path::parent_path(D.getInstalledDir());
-      SmallString<128> LibLTOPath(P);
-      llvm::sys::path::append(LibLTOPath, "lib");
-      llvm::sys::path::append(LibLTOPath, "libLTO.dylib");
-      if (llvm::sys::fs::exists(LibLTOPath)) {
-        CmdArgs.push_back("-lto_library");
-        CmdArgs.push_back(C.getArgs().MakeArgString(LibLTOPath));
-      } else {
-        D.Diag(diag::warn_drv_lto_libpath);
-      }
-    }
-  }
 
   // ld64 version 262 and above run the deduplicate pass by default.
   if (Version[0] >= 262 && shouldLinkerNotDedup(C.getJobs().empty(), Args))
     CmdArgs.push_back("-no_deduplicate");
 
   // Derived from the "link" spec.
   Args.AddAllArgs(CmdArgs, options::OPT_static);
   if (!Args.hasArg(options::OPT_static))

clang/test/Driver/darwin-ld-lto.c

 // REQUIRES: system-darwin
 
 // Check that ld gets "-lto_library" and warnings about libLTO.dylib path.
 
 // RUN: mkdir -p %T/bin
 // RUN: mkdir -p %T/lib
 // RUN: touch %T/lib/libLTO.dylib
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
-// RUN:   -ccc-install-dir %T/bin -mlinker-version=133 -flto 2> %t.log
+// RUN:   -ccc-install-dir %T/bin -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
 //
 // LINK_LTOLIB_PATH: {{ld(.exe)?"}}
 // LINK_LTOLIB_PATH: "-lto_library"
 
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
-// RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 -flto 2> %t.log
+// RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH_WRN %s -input-file %t.log
 //
 // LINK_LTOLIB_PATH_WRN: warning: libLTO.dylib relative to clang installed dir not found; using 'ld' default search path instead
 
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
-// RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 -Wno-liblto -flto 2> %t.log
+// RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 -Wno-liblto 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH_NOWRN %s -input-file %t.log
 //
 // LINK_LTOLIB_PATH_NOWRN-NOT: warning: libLTO.dylib relative to clang installed dir not found; using 'ld' default search path instead