[clang][darwin] Fix search path logic for C_INCLUDE_DIRS

[mhinz]

Summary

For each absolute path given to C_INCLUDE_DIRS, we want it to be added as-is to
the include search path. Relative paths should be prefixed with the sysroot.

[ldionne]

Good find! The following drivers (at least) appear to have the same problem:

clang/lib/Driver/ToolChains/WebAssembly.cpp:236
clang/lib/Driver/ToolChains/Solaris.cpp:247
clang/lib/Driver/ToolChains/Linux.cpp:677
clang/lib/Driver/ToolChains/Hurd.cpp:148
clang/lib/Driver/ToolChains/Fuchsia.cpp:289
clang/lib/Driver/ToolChains/Darwin.cpp:1862

This makes me wonder whether C_INCLUDE_DIRS is useful at all. Does anybody use it? Also, if we keep C_INCLUDE_DIRS, it would be nice to add a test for this.

[mhinz]

Apply fix to all toolchains.

[mhinz]

I applied the fix to all toolchains now, but I'm unsure how to write a test for it. Neither could I find tests for similar options like -DDEFAULT_SYSROOT.

My use case: I want to build a clang that has the same search path as the clang shipped with macOS.

$ /usr/bin/clang -E -xc -v /dev/null
[snip]
#include <...> search starts here:
 /usr/local/include
 /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/11.0.0/include
 /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include
 /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include
 /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks (framework directory)
End of search list.
[snip]

For this, I need to pass this to cmake:

-DC_INCLUDE_DIRS="/usr/local/include:/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include:/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include"
-DDEFAULT_SYSROOT="$(xcrun --show-sdk-path)"

[ldionne]

Do you have commit access?

[mhinz]

In D69221#1848425, @ldionne wrote:

Do you have commit access?

No. :)

clang/lib/Driver/ToolChains/Darwin.cpp

void DarwinClang::AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 
   // Check for configure-time C include directories.
   llvm::StringRef CIncludeDirs(C_INCLUDE_DIRS);
   if (!CIncludeDirs.empty()) {
     llvm::SmallVector<llvm::StringRef, 5> dirs;
     CIncludeDirs.split(dirs, ":");
     for (llvm::StringRef dir : dirs) {
       llvm::StringRef Prefix =
-          llvm::sys::path::is_absolute(dir) ? llvm::StringRef(Sysroot) : "";
+          llvm::sys::path::is_absolute(dir) ? "" : llvm::StringRef(Sysroot);
       addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
     }
   } else {
     // Otherwise, add <sysroot>/usr/include.
     SmallString<128> P(Sysroot);
     llvm::sys::path::append(P, "usr", "include");
     addExternCSystemInclude(DriverArgs, CC1Args, P.str());
   }

clang/lib/Driver/ToolChains/Fuchsia.cpp

void Fuchsia::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
 
   // Check for configure-time C include directories.
   StringRef CIncludeDirs(C_INCLUDE_DIRS);
   if (CIncludeDirs != "") {
     SmallVector<StringRef, 5> dirs;
     CIncludeDirs.split(dirs, ":");
     for (StringRef dir : dirs) {
       StringRef Prefix =
-          llvm::sys::path::is_absolute(dir) ? StringRef(D.SysRoot) : "";
+          llvm::sys::path::is_absolute(dir) ? "" : StringRef(D.SysRoot);
       addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
     }
     return;
   }
 
   if (!D.SysRoot.empty()) {
     SmallString<128> P(D.SysRoot);
     llvm::sys::path::append(P, "include");

clang/lib/Driver/ToolChains/Hurd.cpp

void Hurd::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
 
   // Check for configure-time C include directories.
   StringRef CIncludeDirs(C_INCLUDE_DIRS);
   if (CIncludeDirs != "") {
     SmallVector<StringRef, 5> Dirs;
     CIncludeDirs.split(Dirs, ":");
     for (StringRef Dir : Dirs) {
       StringRef Prefix =
-          llvm::sys::path::is_absolute(Dir) ? StringRef(SysRoot) : "";
+          llvm::sys::path::is_absolute(Dir) ? "" : StringRef(SysRoot);
       addExternCSystemInclude(DriverArgs, CC1Args, Prefix + Dir);
     }
     return;
   }
 
   // Lacking those, try to detect the correct set of system includes for the
   // target triple.
   if (getTriple().getArch() == llvm::Triple::x86) {

clang/lib/Driver/ToolChains/Linux.cpp

void Linux::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
 
   // Check for configure-time C include directories.
   StringRef CIncludeDirs(C_INCLUDE_DIRS);
   if (CIncludeDirs != "") {
     SmallVector<StringRef, 5> dirs;
     CIncludeDirs.split(dirs, ":");
     for (StringRef dir : dirs) {
       StringRef Prefix =
-          llvm::sys::path::is_absolute(dir) ? StringRef(SysRoot) : "";
+          llvm::sys::path::is_absolute(dir) ? "" : StringRef(SysRoot);
       addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
     }
     return;
   }
 
   // Lacking those, try to detect the correct set of system includes for the
   // target triple.
 

clang/lib/Driver/ToolChains/Solaris.cpp

void Solaris::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
 
   // Check for configure-time C include directories.
   StringRef CIncludeDirs(C_INCLUDE_DIRS);
   if (CIncludeDirs != "") {
     SmallVector<StringRef, 5> dirs;
     CIncludeDirs.split(dirs, ":");
     for (StringRef dir : dirs) {
       StringRef Prefix =
-          llvm::sys::path::is_absolute(dir) ? StringRef(D.SysRoot) : "";
+          llvm::sys::path::is_absolute(dir) ? "" : StringRef(D.SysRoot);
       addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
     }
     return;
   }
 
   // Add include directories specific to the selected multilib set and multilib.
   if (GCCInstallation.isValid()) {
     const MultilibSet::IncludeDirsFunc &Callback =

clang/lib/Driver/ToolChains/WebAssembly.cpp

void WebAssembly::AddClangSystemIncludeArgs(const ArgList &DriverArgs,
 
   // Check for configure-time C include directories.
   StringRef CIncludeDirs(C_INCLUDE_DIRS);
   if (CIncludeDirs != "") {
     SmallVector<StringRef, 5> dirs;
     CIncludeDirs.split(dirs, ":");
     for (StringRef dir : dirs) {
       StringRef Prefix =
-          llvm::sys::path::is_absolute(dir) ? StringRef(D.SysRoot) : "";
+          llvm::sys::path::is_absolute(dir) ? "" : StringRef(D.SysRoot);
       addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
     }
     return;
   }
 
   if (getTriple().getOS() != llvm::Triple::UnknownOS) {
     const std::string MultiarchTriple =
         getMultiarchTriple(D, getTriple(), D.SysRoot);