Driver: Don't look for libc++ headers in the install directory on Android.

[pcc]

Summary

The NDK uses a separate set of libc++ headers in the sysroot. Any headers
in the installation directory are not going to work on Android, not least
because they use a different name for the inline namespace (std::1 instead
of std::ndk1).

This effectively makes it impossible to produce a single toolchain that is
capable of targeting both Android and another platform that expects libc++
headers to be installed in the installation directory, such as Mac.

In order to allow this scenario to work, stop looking for headers in the
install directory on Android.

[merge_guards_bot]

Build result: pass - 60573 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt, CMakeCache.txt

[danalbert]

clang/test/Driver/android-no-installed-libcxx.cpp

9

This looks like it should be failing. The NDK headers are installed to <NDK>/sysroot/usr/include/c++/v1. Presumably it's passing because the trivial sysroot in the test directory is not complete enough?

Will need a more specific CHECK-NOT here, and also a CHECK to make sure the correct directory is being searched.

[pcc]

clang/test/Driver/android-no-installed-libcxx.cpp

9

The %t/include/c++/v1 path here represents the install directory, not the sysroot. Note that the test isn't passing a -sysroot flag.

We already have a test that the correct directory is being searched, see test/Driver/android-ndk-standalone.cpp.

[danalbert]

clang/test/Driver/android-no-installed-libcxx.cpp

9

Ah, I'd forgotten that the tests required an explicit --sysroot flag. That's not required for a typical NDK clang call, but it is here. Never mind then, this LGTM.

[davezarzycki]

Can we revert this? It doesn't seem to work on my Fedora 31 box:

FAIL: Clang :: Driver/android-no-installed-libcxx.cpp (9249 of 59761)
******************** TEST 'Clang :: Driver/android-no-installed-libcxx.cpp' FAILED ********************
Script:
--
: 'RUN: at line 4';   mkdir -p /tmp/_update_lc/r/tools/clang/test/Driver/Output/android-no-installed-libcxx.cpp.tmp/bin
: 'RUN: at line 5';   mkdir -p /tmp/_update_lc/r/tools/clang/test/Driver/Output/android-no-installed-libcxx.cpp.tmp/include/c++/v1
: 'RUN: at line 6';   /tmp/_update_lc/r/bin/clang -target aarch64-linux-android -ccc-install-dir /tmp/_update_lc/r/tools/clang/test/Driver/Output/android-no-installed-libcxx.cpp.tmp/bin    -stdlib=libc++ -fsyntax-only /home/dave/s/lp/clang/test/Driver/android-no-installed-libcxx.cpp -### 2>&1 | /tmp/_update_lc/r/bin/FileCheck /home/dave/s/lp/clang/test/Driver/android-no-installed-libcxx.cpp
--
Exit Code: 1

Command Output (stderr):
--
/home/dave/s/lp/clang/test/Driver/android-no-installed-libcxx.cpp:8:15: error: CHECK-NOT: excluded string found in input
// CHECK-NOT: "-internal-isystem" "{{.*}}v1"
              ^
<stdin>:5:653: note: found here
 "/tmp/_update_lc/r/bin/clang-10" "-cc1" "-triple" "aarch64-unknown-linux-android" "-fsyntax-only" "-disable-free" "-disable-llvm-verifier" "-discard-value-names" "-main-file-name" "android-no-installed-libcxx.cpp" "-mrelocation-model" "pic" "-pic-level" "2" "-pic-is-pie" "-mthread-model" "posix" "-mframe-pointer=all" "-fno-rounding-math" "-masm-verbose" "-mconstructor-aliases" "-fuse-init-array" "-target-cpu" "generic" "-target-feature" "+neon" "-target-abi" "aapcs" "-mllvm" "-aarch64-fix-cortex-a53-835769=1" "-fallow-half-arguments-and-returns" "-dwarf-column-info" "-debugger-tuning=gdb" "-resource-dir" "/tmp/_update_lc/r/lib64/clang/10.0.0" "-internal-isystem" "/usr/include/c++/v1" "-internal-isystem" "/usr/local/include" "-internal-isystem" "/tmp/_update_lc/r/lib64/clang/10.0.0/include" "-internal-externc-isystem" "/include" "-internal-externc-isystem" "/usr/include" "-fdeprecated-macro" "-fdebug-compilation-dir" "/tmp/_update_lc/r/tools/clang/test/Driver" "-ferror-limit" "19" "-fmessage-length" "0" "-fno-signed-char" "-fgnuc-version=4.2.1" "-fobjc-runtime=gcc" "-fcxx-exceptions" "-fexceptions" "-fdiagnostics-show-option" "-x" "c++" "/home/dave/s/lp/clang/test/Driver/android-no-installed-libcxx.cpp"

[davezarzycki]

Sorry, I had to revert this for breaking things. Please ping me when you have a new patch.

[pcc]

Relanding with a fix for the problem found by @davezarzycki. The test needed to be adjusted to set --sysroot, otherwise it will fail in the case where a directory named /usr/include/c++/v1 or /usr/local/include/c++/v1 exists.

clang/lib/Driver/ToolChains/Linux.cpp

for (llvm::vfs::directory_iterator LI = vfs.dir_begin(base, EC), LE;
     }
   }
   return MaxVersion ? (base + "/" + MaxVersionString).str() : "";
 }
 
 void Linux::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
                                   llvm::opt::ArgStringList &CC1Args) const {
   const std::string& SysRoot = computeSysRoot();
-  const std::string LibCXXIncludePathCandidates[] = {
-      DetectLibcxxIncludePath(getVFS(), getDriver().Dir + "/../include/c++"),
-      // If this is a development, non-installed, clang, libcxx will
-      // not be found at ../include/c++ but it likely to be found at
-      // one of the following two locations:
-      DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/local/include/c++"),
-      DetectLibcxxIncludePath(getVFS(), SysRoot + "/usr/include/c++") };
-  for (const auto &IncludePath : LibCXXIncludePathCandidates) {
-    if (IncludePath.empty() || !getVFS().exists(IncludePath))
-      continue;
-    // Use the first candidate that exists.
-    addSystemInclude(DriverArgs, CC1Args, IncludePath);
+  auto AddIncludePath = [&](std::string Path) {
+    std::string IncludePath = DetectLibcxxIncludePath(getVFS(), Path);
+    if (IncludePath.empty() || !getVFS().exists(IncludePath))
+      return false;
+    addSystemInclude(DriverArgs, CC1Args, IncludePath);
+    return true;
+  };
+  // Android never uses the libc++ headers installed alongside the toolchain,
+  // which are generally incompatible with the NDK libraries anyway.
+  if (!getTriple().isAndroid())
+    if (AddIncludePath(getDriver().Dir + "/../include/c++"))
+      return;
+  if (AddIncludePath(SysRoot + "/usr/local/include/c++"))
+    return;
+  if (AddIncludePath(SysRoot + "/usr/include/c++"))
     return;
-  }
 }
 
 void Linux::addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
                                      llvm::opt::ArgStringList &CC1Args) const {
   // We need a detected GCC installation on Linux to provide libstdc++'s
   // headers.
   if (!GCCInstallation.isValid())
     return;

clang/test/Driver/android-no-installed-libcxx.cpp

+// Check that we don't find the libc++ in the installation directory when
+// targeting Android.
+
+// RUN: mkdir -p %t/bin
+// RUN: mkdir -p %t/include/c++/v1
+// RUN: mkdir -p %t/sysroot
+// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN:   --sysroot=%t/sysroot -stdlib=libc++ -fsyntax-only \
+// RUN:   %s -### 2>&1 | FileCheck %s
+// CHECK-NOT: "-internal-isystem" "{{.*}}v1"

clang/test/Driver/stdlibxx-isystem.cpp

 // Backslash escaping makes matching against the installation directory fail on
 // Windows. Temporarily disable the test there until we add an option to print
 // the installation directory unescaped.
 // UNSUPPORTED: system-windows
 
 // By default, we should search for libc++ next to the driver.
 // RUN: mkdir -p %t/bin
 // RUN: mkdir -p %t/include/c++/v1
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=LIBCXX %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
 // RUN:   -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=LIBCXX %s
 // LIBCXX: InstalledDir: [[INSTALLDIR:.+$]]
 // LIBCXX: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
 
 // Passing -stdlib++-isystem should suppress the default search.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NODEFAULT %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NODEFAULT %s
 // NODEFAULT: InstalledDir: [[INSTALLDIR:.+$]]
 // NODEFAULT-NOT: "-internal-isystem" "[[INSTALLDIR]]/../include/c++/v1"
 
 // And we should add it as an -internal-isystem.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=INCPATH %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -stdlib=libc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=INCPATH %s
 // INCPATH: "-internal-isystem" "/tmp/foo" "-internal-isystem" "/tmp/bar"
 
 // We shouldn't pass the -stdlib++-isystem to cc1.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=NOCC1 %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp -stdlib=libc++ -fsyntax-only %s -### 2>&1 | \
 // RUN:   FileCheck -check-prefix=NOCC1 %s
 // NOCC1-NOT: "-stdlib++-isystem" "/tmp"
 
 // It should respect -nostdinc++.
-// RUN: %clang -target aarch64-linux-android -ccc-install-dir %t/bin \
+// RUN: %clang -target aarch64-linux-gnu -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -nostdinc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NOSTDINCXX %s
 // RUN: %clang -target x86_64-apple-darwin -ccc-install-dir %t/bin \
 // RUN:   -stdlib++-isystem /tmp/foo -stdlib++-isystem /tmp/bar -nostdinc++ \
 // RUN:   -fsyntax-only %s -### 2>&1 | FileCheck -check-prefix=NOSTDINCXX %s
 // NOSTDINCXX-NOT: "-internal-isystem" "/tmp/foo" "-internal-isystem" "/tmp/bar"