[Driver] Add CLANG_PREFER_GCC_LIBRARIES which can be disabled

[Hahnfeld]

Summary

Currently, libraries installed next to GCC are preferred over those next to Clang. Make this behaviour configurable during build.

This is for example needed to make sure we get LLVM's libunwind.so.1 and don't end up with the system default non-GNU libunwind.so.8 as reported in D25008.

[Hahnfeld]

ping

[Hahnfeld]

ping

[Hahnfeld]

Ping!

[Hahnfeld]

ping

[EricWF]

I don't feel comfortable reviewing driver changes, since I never work on that code.

[Hahnfeld]

Ping. Looks like Chandler originally added this logic...

[Hahnfeld]

Ping. Could I get comments before the branching this week?

[rsmith]

This makes sense to me in principle, but I'd like @chandlerc to weigh in.

[chandlerc]

There are some issues with this patch currently.

At a basic level, it needs well crafted test cases. You can find out how to write them by looking at other Driver tests.

But at a deeper level, the change here will be an very significant and potentially very disruptive change. While libunwind is your motivation, it could well have a dramatic impact on the selected set of libraries for links. I'm very concerned to make this kind of change without a *lot* of careful testing, and a very clear rationale about why this is the right search order *for all libraries*, not just libunwind. Because this applies to all libraries. =]

[Hahnfeld]

This change doesn't break existing tests but maybe I should extend linux-ld.c and so on?

About the impact: Which libraries are there next to clang that could conflict with the system? When I look at my current installation dir:

• libLLVM*, libclang*, liblld* libraries which should be fine to use
• libc++, libc++experimental, libc++abi and libunwind which we really want to use IMO
• OpenMP runtime(s) which are fine to use (or even better than the system default)
• I'm not sure about libLTO but I *think* that users do not link to this library directly?

[Hahnfeld]

Add test case to check that

• bin/../lib is the first library path by default
• user-specified -L preceeds

[chandlerc]

In D26244#641076, @Hahnfeld wrote:

This change doesn't break existing tests but maybe I should extend linux-ld.c and so on?

About the impact: Which libraries are there next to clang that could conflict with the system? When I look at my current installation dir:

• libLLVM*, libclang*, liblld* libraries which should be fine to use
• libc++, libc++experimental, libc++abi and libunwind which we really want to use IMO
• OpenMP runtime(s) which are fine to use (or even better than the system default)
• I'm not sure about libLTO but I *think* that users do not link to this library directly?

Those are just the libriaries LLVM is installing.

If it is installed into a prefix, say, /opt/mumble/{bin,lib,...} along with other libraries, then suddenly that directory is searched in a very different place.

I'm not arguing one way or the other here. I'm pointing out that this is a *huge* change to make. It will cause a large number of users to silently have a different library search path than they have had before, something that has been fairly stable for several years. So whatever we do must be considered very carefully.

[Hahnfeld]

In D26244#641099, @chandlerc wrote:

Those are just the libriaries LLVM is installing.

If it is installed into a prefix, say, /opt/mumble/{bin,lib,...} along with other libraries, then suddenly that directory is searched in a very different place.

I'm not arguing one way or the other here. I'm pointing out that this is a *huge* change to make. It will cause a large number of users to silently have a different library search path than they have had before, something that has been fairly stable for several years. So whatever we do must be considered very carefully.

Haven't thought about that! If I don't install to /usr/lib, I usually only install one software per prefix but there might be cases where that is not true.

What do you propose? Should I send an RFC to cfe-dev?

[mgorny]

I think the easiest solution would be for you to use a path related to clang runtime directory, and install the runtimes there.

However, this would require rethinking the structure a bit to cover multilib, different ABIs, maybe cross. We have a cheap hack for this in clang-suite (I can give you link when I get home) but this really requires some smart thinking.

[Hahnfeld]

gentle ping

[Hahnfeld]

Abandoning as I lost interest in this.

CMakeLists.txt

 set(DEFAULT_SYSROOT "" CACHE PATH
   "Default <path> to all compiler invocations for --sysroot=<path>." )
 
 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")
 
 set(ENABLE_X86_RELAX_RELOCATIONS OFF CACHE BOOL
     "enable x86 relax relocations by default")
 
+option(CLANG_PREFER_GCC_LIBRARIES "Prefer libraries installed next to GCC." ON)
+
 set(CLANG_DEFAULT_LINKER "" CACHE STRING
   "Default linker to use (linker name or absolute path, empty for platform default)")
 
 set(CLANG_DEFAULT_CXX_STDLIB "" CACHE STRING
   "Default C++ stdlib to use (\"libstdc++\" or \"libc++\", empty for platform default")
 if (NOT(CLANG_DEFAULT_CXX_STDLIB STREQUAL "" OR
         CLANG_DEFAULT_CXX_STDLIB STREQUAL "libstdc++" OR
         CLANG_DEFAULT_CXX_STDLIB STREQUAL "libc++"))

include/clang/Config/config.h.cmake

 /* This generated file is for internal use. Do not include it from headers. */
 
 #ifdef CLANG_CONFIG_H
 #error config.h can only be included once
 #else
 #define CLANG_CONFIG_H
 
 /* Bug report URL. */
 #define BUG_REPORT_URL "${BUG_REPORT_URL}"
 
+/* Prefer libraries installed next to GCC. */
+#cmakedefine CLANG_PREFER_GCC_LIBRARIES
+
 /* Default linker to use. */
 #define CLANG_DEFAULT_LINKER "${CLANG_DEFAULT_LINKER}"
 
 /* Default C++ stdlib to use. */
 #define CLANG_DEFAULT_CXX_STDLIB "${CLANG_DEFAULT_CXX_STDLIB}"
 
 /* Default runtime library to use. */
 #define CLANG_DEFAULT_RTLIB "${CLANG_DEFAULT_RTLIB}"

lib/Driver/ToolChains.cpp

 //===--- ToolChains.cpp - ToolChain Implementations -------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "ToolChains.h"
 #include "clang/Basic/Cuda.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/Version.h"
 #include "clang/Basic/VirtualFileSystem.h"
-#include "clang/Config/config.h" // for GCC_INSTALL_PREFIX
+#include "clang/Config/config.h" // for CLANG_PREFER_GCC_LIBRARIES and GCC_INSTALL_PREFIX
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Distro.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
#endif
   // This was determined by running GCC in a fake filesystem, creating all
   // possible permutations of these directories, and seeing which ones it added
   // to the link paths.
   path_list &Paths = getFilePaths();
 
   const std::string OSLibDir = getOSLibDir(Triple, Args);
   const std::string MultiarchTriple = getMultiarchTriple(D, Triple, SysRoot);
 
+#ifndef CLANG_PREFER_GCC_LIBRARIES
+  // Similar to the logic for GCC below, if we currently running Clang inside
+  // of the requested system root, add its parent library paths to those
+  // searched.
+  // FIXME: It's not clear whether we should use the driver's installed
+  // directory ('Dir' below) or the ResourceDir.
+  if (StringRef(D.Dir).startswith(SysRoot)) {
+    addPathIfExists(D, D.Dir + "/../lib/" + MultiarchTriple, Paths);
+    addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths);
+    addPathIfExists(D, D.Dir + "/../lib", Paths);
+  }
+#endif
+
   // Add the multilib suffixed paths where they are available.
   if (GCCInstallation.isValid()) {
     const llvm::Triple &GCCTriple = GCCInstallation.getTriple();
     const std::string &LibPath = GCCInstallation.getParentLibPath();
     const Multilib &Multilib = GCCInstallation.getMultilib();
     const MultilibSet &Multilibs = GCCInstallation.getMultilibs();
 
     // Add toolchain / multilib specific file paths.
if (GCCInstallation.isValid()) {
     // configurations but this seems somewhere between questionable and simply
     // a bug.
     if (StringRef(LibPath).startswith(SysRoot)) {
       addPathIfExists(D, LibPath + "/" + MultiarchTriple, Paths);
       addPathIfExists(D, LibPath + "/../" + OSLibDir, Paths);
     }
   }
 
+#ifdef CLANG_PREFER_GCC_LIBRARIES
   // Similar to the logic for GCC above, if we currently running Clang inside
   // of the requested system root, add its parent library paths to
   // those searched.
   // FIXME: It's not clear whether we should use the driver's installed
   // directory ('Dir' below) or the ResourceDir.
   if (StringRef(D.Dir).startswith(SysRoot)) {
     addPathIfExists(D, D.Dir + "/../lib/" + MultiarchTriple, Paths);
     addPathIfExists(D, D.Dir + "/../" + OSLibDir, Paths);
   }
+#endif
 
   addPathIfExists(D, SysRoot + "/lib/" + MultiarchTriple, Paths);
   addPathIfExists(D, SysRoot + "/lib/../" + OSLibDir, Paths);
   addPathIfExists(D, SysRoot + "/usr/lib/" + MultiarchTriple, Paths);
   addPathIfExists(D, SysRoot + "/usr/lib/../" + OSLibDir, Paths);
 
   // Try walking via the GCC triple path in case of biarch or multiarch GCC
   // installations with strange symlinks.
addPathIfExists(D, LibPath + "/../" + GCCTriple.str() + "/lib" +
                     Paths);
 
     // See comments above on the multilib variant for details of why this is
     // only included from within the sysroot.
     if (StringRef(LibPath).startswith(SysRoot))
       addPathIfExists(D, LibPath, Paths);
   }
 
+#ifdef CLANG_PREFER_GCC_LIBRARIES
   // Similar to the logic for GCC above, if we are currently running Clang
   // inside of the requested system root, add its parent library path to those
   // searched.
   // FIXME: It's not clear whether we should use the driver's installed
   // directory ('Dir' below) or the ResourceDir.
   if (StringRef(D.Dir).startswith(SysRoot))
     addPathIfExists(D, D.Dir + "/../lib", Paths);
+#endif
 
   addPathIfExists(D, SysRoot + "/lib", Paths);
   addPathIfExists(D, SysRoot + "/usr/lib", Paths);
 }
 
 bool Linux::HasNativeLLVMSupport() const { return true; }
 
 Tool *Linux::buildLinker() const { return new tools::gnutools::Linker(*this); }