[PATCH] D41534: Don't add empty InstalledDir as a candidate GCC location

[Jack Andersen via Phabricator via cfe-commits]

jackoalan created this revision.
jackoalan added a reviewer: cfe-commits.

I maintain a couple build tools based on Clang's Tooling framework. The tools are built and used directly out of the CMake binary directory (i.e. not installed in a meaningful way).

This non-installed setup causes issues when finding the GCC Toolchain and using the resulting system include paths. A normally-installed `clang -v` outputs the following:

  clang version 5.0.1 (tags/RELEASE_501/final)
  Target: x86_64-unknown-linux-gnu
  Thread model: posix
  InstalledDir: /usr/bin
  Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-pc-linux-gnu/7.2.1
  Found candidate GCC installation: /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.2.1
  Found candidate GCC installation: /usr/lib/gcc/x86_64-pc-linux-gnu/7.2.1
  Found candidate GCC installation: /usr/lib64/gcc/x86_64-pc-linux-gnu/7.2.1
  Selected GCC installation: /usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.2.1
  Candidate multilib: .;@m64
  Candidate multilib: 32;@m32
  Selected multilib: .;@m64

But my build-tree tool outputs this:

  clang version 5.0.1 (tags/RELEASE_501/final)
  Target: x86_64-unknown-linux-gnu
  Thread model: posix
  InstalledDir: 
  Found candidate GCC installation: /../lib/gcc/x86_64-pc-linux-gnu/7.2.1
  Found candidate GCC installation: /../lib64/gcc/x86_64-pc-linux-gnu/7.2.1
  Found candidate GCC installation: /usr/lib/gcc/x86_64-pc-linux-gnu/7.2.1
  Found candidate GCC installation: /usr/lib64/gcc/x86_64-pc-linux-gnu/7.2.1
  Selected GCC installation: /../lib64/gcc/x86_64-pc-linux-gnu/7.2.1

InstalledDir is empty, which is fine, but the resulting candidate selection is undesirable. The `/usr` rooted candidate strikes me as a much more stable option. Granted `/../lib64/gcc/x86_64-pc-linux-gnu/7.2.1` works, but on a rather hackish technicality where `/../ == /` and symlink exists at `/lib64`.

This creates problems with dependency files consumed by Ninja (which is unable to canonicalize paths that feature a mix of `..` and multi-level symlinks as explained in https://reviews.llvm.org/D37954). For example: `/../lib64/gcc/x86_64-pc-linux-gnu/7.2.1/../../../../include/c++/7.2.1/cstdlib` which ninja "canonicalizes" as `/../include/c++/7.2.1/cstdlib`, but is actually `/usr/include/c++/7.2.1/cstdlib` when taking symlinks into account.

If clang were to use the `/usr/lib64/gcc/x86_64-pc-linux-gnu/7.2.1` candidate instead, the result would be `/usr/lib64/gcc/x86_64-pc-linux-gnu/7.2.1/../../../../include/c++/7.2.1/cstdlib` which Ninja can correctly canonicalize to `/usr/include/c++/7.2.1/cstdlib`.

I'm not trying to force clang to produce canonical paths as output; rather avoid the obviously problematic `/../` candidate in the first place.


Repository:
  rC Clang

https://reviews.llvm.org/D41534

Files:
  lib/Driver/ToolChains/Gnu.cpp


Index: lib/Driver/ToolChains/Gnu.cpp
===================================================================
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -1617,7 +1617,8 @@
     }
 
     // Then look for gcc installed alongside clang.
-    Prefixes.push_back(D.InstalledDir + "/..");
+    if (!D.InstalledDir.empty())
+      Prefixes.push_back(D.InstalledDir + "/..");
 
     // Then look for distribution supplied gcc installations.
     if (D.SysRoot.empty()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41534.127982.patch
Type: text/x-patch
Size: 486 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171222/cefe4c29/attachment.bin>