[driver] Search for compatible Android runtime directories

[smeenai]

Summary

Android triples include a version number, which makes direct triple
comparisons for per-target runtime directory searching not always work.
Instead, look for the triple with the highest compatible version number
and use that per-target runtime directory instead. This maintains the
existing fallback to a triple without any version number, but I'm hoping
we can remove that in the future. https://discourse.llvm.org/t/62717
discusses this further.

The one remaining triple mismatch after this is that Android armv7
triples usually have an environment of androideabi, which Clang
normalizes to android. If you use the androideabi triple when
building the runtimes with a per-target runtimes dir, the directory will
get created with androideabi in its name, but Clang's triple search
uses the normalized triple and will look for an android directory
instead. https://reviews.llvm.org/D140925 will fix that by normalizing
triples when creating the per-target runtimes directories as well.

[MaskRay]

// Android target triples contain a target version. If we don't have libraries for the exact target version, we should fall back to the next newest version or a versionless path, if any.

An Android maintainer can override my opinion but my feeling is that this ad-hoc rule probably should be the responsibility of the file hierarchy instead of clangDriver. This really easy to cause mix-and-match situations where the problem is on the user side but the user may blame Clang.

This can be implemented by adding symlinks in the file hierarchy.

[smeenai]

In D158476#4605360, @MaskRay wrote:

// Android target triples contain a target version. If we don't have libraries for the exact target version, we should fall back to the next newest version or a versionless path, if any.

An Android maintainer can override my opinion but my feeling is that this ad-hoc rule probably should be the responsibility of the file hierarchy instead of clangDriver. This really easy to cause mix-and-match situations where the problem is on the user side but the user may blame Clang.

This can be implemented by adding symlinks in the file hierarchy.

That was one of the alternatives I brought up in https://discourse.llvm.org/t/handling-version-numbers-in-per-target-runtime-directories/62717, but symlinks are complicated on Windows, and you'd also have to add build system code to create those symlinks, so I don't think it'll be a clear win.

Are you concerned with falling back to the next newest version or the versionless path (or both)? I think the next newest version is pretty inline with general OS compatibility: if you don't have version N, look for version N - 1, N - 2, etc (since they're forward compatible, see also https://discourse.llvm.org/t/handling-version-numbers-in-per-target-runtime-directories/62717/13). The versionless path is a little less principled IMO (though I guess you could see it as implicitly being compatible with any version) ... maybe we could get rid of that if Chrome is okay moving to a versioned directory.

(Aside: why do we need to produce runtime libraries for different versions in the first place? We want one for the minimum OS version we support, of course, but then there's important additions like version 29 adding ELF TLS support and version 31 adding a thread properties API used by LSAN, so building libraries targeting those important versions and having them be dynamically selected based on your target OS version is super useful. We could build the runtime libraries for every single OS version we wanted to support, but that's a lot of versions (at least 21 through 31, times four architectures), which is wasteful for both build times and toolchain distribution size.)

[thakis]

I think this is a useful feature, for the reasons mentioned on the thread.

Since this is a superset of D115049 (... right?), this should probably revert the changes over there?

Maybe we should dump searched paths in -v mode?

[smeenai]

In D158476#4608428, @thakis wrote:

I think this is a useful feature, for the reasons mentioned on the thread.

Since this is a superset of D115049 (... right?), this should probably revert the changes over there?

Wouldn't it break Chrome if I reverted those changes as part of this diff? I was thinking I could land this, Chrome could move over to a versioned dir (assuming that works for it), and then we could undo D115049. I'm happy to make the revert as part of this change if it'll work out for Chrome though.

Maybe we should dump searched paths in -v mode?

As in output all the versioned directories that were found and considered?

[srhines]

This approach LGTM, assuming that https://reviews.llvm.org/D140925 lands as well to ensure that the triple does use androideabi, since it would prevent custom build setups from needing additional help.

(Aside: why do we need to produce runtime libraries for different versions in the first place? We want one for the minimum OS version we support, of course, but then there's important additions like version 29 adding ELF TLS support and version 31 adding a thread properties API used by LSAN, so building libraries targeting those important versions and having them be dynamically selected based on your target OS version is super useful. We could build the runtime libraries for every single OS version we wanted to support, but that's a lot of versions (at least 21 through 31, times four architectures), which is wasteful for both build times and toolchain distribution size.)

I don't think we need all the versions, but today we really only use 21 (lowest level), and 30 (a conservative level for the platform that supports all our apexes). Ideally we'd also pick up the latest platform version, and we probably should explicitly call out 29 for ELF TLS, so that would be ~4 supported levels, but maybe we can use 29 instead of 30 to cut it to ~3 for most architectures.

[smeenai]

In D158476#4609243, @srhines wrote:

This approach LGTM, assuming that https://reviews.llvm.org/D140925 lands as well to ensure that the triple does use androideabi, since it would prevent custom build setups from needing additional help.

@phosek is working on that one, but I think this is useful even without it, because you just need to use android in your triple when building the runtimes, and can use androideabi everywhere else. I'll put up an example runtimes build setup with that to demonstrate.

(Aside: why do we need to produce runtime libraries for different versions in the first place? We want one for the minimum OS version we support, of course, but then there's important additions like version 29 adding ELF TLS support and version 31 adding a thread properties API used by LSAN, so building libraries targeting those important versions and having them be dynamically selected based on your target OS version is super useful. We could build the runtime libraries for every single OS version we wanted to support, but that's a lot of versions (at least 21 through 31, times four architectures), which is wasteful for both build times and toolchain distribution size.)

I don't think we need all the versions, but today we really only use 21 (lowest level), and 30 (a conservative level for the platform that supports all our apexes). Ideally we'd also pick up the latest platform version, and we probably should explicitly call out 29 for ELF TLS, so that would be ~4 supported levels, but maybe we can use 29 instead of 30 to cut it to ~3 for most architectures.

Ah, yeah, https://reviews.llvm.org/D85927 dynamically detects the thread properties API, so we don't need an explicit build for 31. An explicit build with ELF TLS is still required though.

[smeenai]

Rebase

[smeenai]

D158798 demonstrates a working Android runtimes setup with per-target runtime directories once this change is in place. It'll be nicer once D140925 lands, but it's workable even without that.

[smeenai]

Rebase

[smeenai]

Kick off pre-merge checks

[smeenai]

@thakis I'm inclined to leave the fallback for a triple without any version in place for now, since it seems like Mozilla might be relying on it too, and we can remove it once everyone's moved over to the new fallback mechanism. What do you think?

[thakis]

Works for me. How do you imagine the transition happening? Should we emit some kind of warning if the old fallback is hit?

[smeenai]

In D158476#4632188, @thakis wrote:

Works for me. How do you imagine the transition happening? Should we emit some kind of warning if the old fallback is hit?

That's a good call. I'll add that plus a release note.

[smeenai]

Add warning and release note

[smeenai]

Ping. I added the warning and release note. I haven't made search paths be dumped in -v mode because I thought it could be pretty noisy (especially since you'll be doing three searches, for resource dir libraries, libc++ per-target headers, and non-resource dir libraries), but I'm happy to do so if people think it'll be useful (or add it under a different flag).

[smeenai]

Ping.

[phosek]

LGTM

[pirama]

LGTM from the Android platform/NDK side.

[MaskRay]

nits

clang/docs/ReleaseNotes.rst

295 ↗

(On Diff #555213)

--target=

clang/test/Driver/android-unversioned-fallback-warning.cpp

14 ↗

(On Diff #555213)

--target=

-target has been deprecated since Clang 3.4

[smeenai]

In D158476#4647652, @MaskRay wrote:

nits

Sorry, this came in right as I committed the diff. I pushed rG915ebb07dfc53486eccf0dc09b6411929a463e98 to address it.

clang/include/clang/Driver/ToolChain.h

private:
   /// The effective clang triple for the current Job.
   mutable llvm::Triple EffectiveTriple;
 
   /// Set the toolchain's effective clang triple.
   void setEffectiveTriple(llvm::Triple ET) const {
     EffectiveTriple = std::move(ET);
   }
 
+  std::optional<std::string>
+  getFallbackAndroidTargetPath(StringRef BaseDir) const;
+
   mutable std::optional<CXXStdlibType> cxxStdlibType;
   mutable std::optional<RuntimeLibType> runtimeLibType;
   mutable std::optional<UnwindLibType> unwindLibType;
 
 protected:
   MultilibSet Multilibs;
   llvm::SmallVector<Multilib> SelectedMultilibs;
 

clang/lib/Driver/ToolChain.cpp

 }
 
 const char *ToolChain::getCompilerRTArgString(const llvm::opt::ArgList &Args,
                                               StringRef Component,
                                               FileType Type) const {
   return Args.MakeArgString(getCompilerRT(Args, Component, Type));
 }
 
+// Android target triples contain a target version. If we don't have libraries
+// for the exact target version, we should fall back to the next newest version
+// or a versionless path, if any.
+std::optional<std::string>
+ToolChain::getFallbackAndroidTargetPath(StringRef BaseDir) const {
+  llvm::Triple TripleWithoutLevel(getTriple());
+  TripleWithoutLevel.setEnvironmentName("android"); // remove any version number
+  const std::string &TripleWithoutLevelStr = TripleWithoutLevel.str();
+  unsigned TripleVersion = getTriple().getEnvironmentVersion().getMajor();
+  unsigned BestVersion = 0;
+
+  SmallString<32> TripleDir;
+  std::error_code EC;
+  for (llvm::vfs::directory_iterator LI = getVFS().dir_begin(BaseDir, EC), LE;
+       !EC && LI != LE; LI = LI.increment(EC)) {
+    StringRef DirName = llvm::sys::path::filename(LI->path());
+    StringRef DirNameSuffix = DirName;
+    if (DirNameSuffix.consume_front(TripleWithoutLevelStr)) {
+      if (DirNameSuffix.empty() && TripleDir.empty()) {
+        TripleDir = DirName;
+      } else {
+        unsigned Version;
+        if (!DirNameSuffix.getAsInteger(10, Version) && Version > BestVersion &&
+            Version < TripleVersion) {
+          BestVersion = Version;
+          TripleDir = DirName;
+        }
+      }
+    }
+  }
+
+  if (!TripleDir.empty()) {
+    SmallString<128> P(BaseDir);
+    llvm::sys::path::append(P, TripleDir);
+    return std::string(P);
+  }
+
+  return {};
+}
+
 std::optional<std::string>
 ToolChain::getTargetSubDirPath(StringRef BaseDir) const {
   auto getPathForTriple =
       [&](const llvm::Triple &Triple) -> std::optional<std::string> {
     SmallString<128> P(BaseDir);
     llvm::sys::path::append(P, Triple.str());
     if (getVFS().exists(P))
       return std::string(P);
ToolChain::getTargetSubDirPath(StringRef BaseDir) const {
   // target runtime directory layout.
   if (getTriple().getArch() == Triple::arm && !getTriple().isArmMClass()) {
     llvm::Triple ArmTriple = getTriple();
     ArmTriple.setArch(Triple::arm);
     if (auto Path = getPathForTriple(ArmTriple))
       return *Path;
   }
 
-  // Android targets may include an API level at the end. We still want to fall
-  // back on a path without the API level.
-  if (getTriple().isAndroid() &&
-      getTriple().getEnvironmentName() != "android") {
-    llvm::Triple TripleWithoutLevel = getTriple();
-    TripleWithoutLevel.setEnvironmentName("android");
-    if (auto Path = getPathForTriple(TripleWithoutLevel))
-      return *Path;
-  }
+  if (getTriple().isAndroid())
+    return getFallbackAndroidTargetPath(BaseDir);
 
   return {};
 }
 
 std::optional<std::string> ToolChain::getRuntimePath() const {
   SmallString<128> P(D.ResourceDir);
   llvm::sys::path::append(P, "lib");
   return getTargetSubDirPath(P);

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

 // Check that we only find libc++ in the installation directory when it contains
 // an Android-specific include directory.
 
 // RUN: mkdir -p %t1/bin
 // RUN: mkdir -p %t1/include/c++/v1
 // RUN: mkdir -p %t1/sysroot
 // RUN: %clang -target aarch64-none-linux-android -ccc-install-dir %t1/bin \
 // RUN:   --sysroot=%t1/sysroot -stdlib=libc++ -fsyntax-only \
 // RUN:   %s -### 2>&1 | FileCheck %s
 // CHECK-NOT: "-internal-isystem" "{{.*}}v1"
 
 // RUN: mkdir -p %t2/bin
 // RUN: mkdir -p %t2/include/c++/v1
 // RUN: mkdir -p %t2/sysroot
 // RUN: mkdir -p %t2/include/aarch64-none-linux-android/c++/v1
+// RUN: mkdir -p %t2/include/aarch64-none-linux-android23/c++/v1
 
 // RUN: %clang -target aarch64-none-linux-android -ccc-install-dir %/t2/bin \
 // RUN:   --sysroot=%t2/sysroot -stdlib=libc++ -fsyntax-only \
 // RUN:   %s -### 2>&1 | FileCheck --check-prefix=ANDROID-DIR -DDIR=%/t2/bin %s
 
 // RUN: %clang -target aarch64-none-linux-android21 -ccc-install-dir %/t2/bin \
 // RUN:   --sysroot=%t2/sysroot -stdlib=libc++ -fsyntax-only \
 // RUN:   %s -### 2>&1 | FileCheck --check-prefix=ANDROID-DIR -DDIR=%/t2/bin %s
 
 // ANDROID-DIR: "-internal-isystem" "[[DIR]][[SEP:/|\\\\]]..[[SEP]]include[[SEP]]aarch64-none-linux-android[[SEP]]c++[[SEP]]v1"
 // ANDROID-DIR-SAME: "-internal-isystem" "[[DIR]][[SEP]]..[[SEP]]include[[SEP]]c++[[SEP]]v1"
+
+// RUN: %clang -target aarch64-none-linux-android23 -ccc-install-dir %/t2/bin \
+// RUN:   --sysroot=%t2/sysroot -stdlib=libc++ -fsyntax-only \
+// RUN:   %s -### 2>&1 | FileCheck --check-prefix=ANDROID23-DIR -DDIR=%/t2/bin %s
+
+// RUN: %clang -target aarch64-none-linux-android28 -ccc-install-dir %/t2/bin \
+// RUN:   --sysroot=%t2/sysroot -stdlib=libc++ -fsyntax-only \
+// RUN:   %s -### 2>&1 | FileCheck --check-prefix=ANDROID23-DIR -DDIR=%/t2/bin %s
+
+// ANDROID23-DIR: "-internal-isystem" "[[DIR]][[SEP:/|\\\\]]..[[SEP]]include[[SEP]]aarch64-none-linux-android23[[SEP]]c++[[SEP]]v1"
+// ANDROID23-DIR-SAME: "-internal-isystem" "[[DIR]][[SEP]]..[[SEP]]include[[SEP]]c++[[SEP]]v1"

clang/test/Driver/linux-per-target-runtime-dir.c

 // CHECK-PER-TARGET-RUNTIME: "-internal-isystem" "{{.*}}{{/|\\\\}}..{{/|\\\\}}include{{/|\\\\}}c++{{/|\\\\}}v1"
 // CHECK-PER-TARGET-RUNTIME: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
 // CHECK-PER-TARGET-RUNTIME: "--sysroot=[[SYSROOT]]"
 // CHECK-PER-TARGET-RUNTIME: "-L{{.*}}{{/|\\\\}}..{{/|\\\\}}lib{{/|\\\\}}x86_64-unknown-linux-gnu"
 
 // RUN: %clang --target=aarch64-unknown-linux-android21 -print-file-name=libc++.so 2>&1 \
 // RUN:     -ccc-install-dir %S/Inputs/basic_android_libcxx_tree/usr/bin \
 // RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-ANDROID21 %s
-// CHECK-LIBCXX-ANDROID21: ..{{/|\\}}lib{{/|\\}}aarch64-unknown-linux-android21{{/|\\}}libc++.so
+// CHECK-LIBCXX-ANDROID21: ..{{/|\\}}lib{{/|\\}}aarch64-unknown-linux-android{{/|\\}}libc++.so
 
 // RUN: %clang --target=aarch64-unknown-linux-android23 -print-file-name=libc++.so 2>&1 \
 // RUN:     -ccc-install-dir %S/Inputs/basic_android_libcxx_tree/usr/bin \
 // RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-ANDROID23 %s
-// CHECK-LIBCXX-ANDROID23: ..{{/|\\}}lib{{/|\\}}aarch64-unknown-linux-android{{/|\\}}libc++.so
+// CHECK-LIBCXX-ANDROID23: ..{{/|\\}}lib{{/|\\}}aarch64-unknown-linux-android23{{/|\\}}libc++.so
+
+// RUN: %clang --target=aarch64-unknown-linux-android26 -print-file-name=libc++.so 2>&1 \
+// RUN:     -ccc-install-dir %S/Inputs/basic_android_libcxx_tree/usr/bin \
+// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-ANDROID23 %s
+
+// RUN: %clang --target=aarch64-unknown-linux-android29 -print-file-name=libc++.so 2>&1 \
+// RUN:     -ccc-install-dir %S/Inputs/basic_android_libcxx_tree/usr/bin \
+// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-ANDROID29 %s
+// CHECK-LIBCXX-ANDROID29: ..{{/|\\}}lib{{/|\\}}aarch64-unknown-linux-android29{{/|\\}}libc++.so
+
+// RUN: %clang --target=aarch64-unknown-linux-android31 -print-file-name=libc++.so 2>&1 \
+// RUN:     -ccc-install-dir %S/Inputs/basic_android_libcxx_tree/usr/bin \
+// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-ANDROID29 %s
 
 // RUN: %clang --target=aarch64-unknown-linux-android -print-file-name=libc++.so 2>&1 \
 // RUN:     -ccc-install-dir %S/Inputs/basic_android_libcxx_tree/usr/bin \
 // RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-ANDROID %s
 // CHECK-LIBCXX-ANDROID: ..{{/|\\}}lib{{/|\\}}aarch64-unknown-linux-android{{/|\\}}libc++.so
 
 // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \
 // RUN:     --target=x86_64-unknown-linux-gnu \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-CLANGRT-X8664 %s
 // CHECK-CLANGRT-X8664: lib{{/|\\}}x86_64-unknown-linux-gnu{{/|\\}}libclang_rt.builtins.a
 
 // RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
 // RUN:     --target=x86_64-unknown-linux-gnu \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-X8664 %s
 // CHECK-FILE-NAME-X8664: lib{{/|\\}}x86_64-unknown-linux-gnu{{/|\\}}libclang_rt.builtins.a
 
 // RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
 // RUN:     --target=aarch64-unknown-linux-android21 \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID21 %s
-// CHECK-FILE-NAME-ANDROID21: lib{{/|\\}}aarch64-unknown-linux-android21{{/|\\}}libclang_rt.builtins.a
+// CHECK-FILE-NAME-ANDROID21: lib{{/|\\}}aarch64-unknown-linux-android{{/|\\}}libclang_rt.builtins.a
 
 // RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
 // RUN:     --target=aarch64-unknown-linux-android23 \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID23 %s
-// CHECK-FILE-NAME-ANDROID23: lib{{/|\\}}aarch64-unknown-linux-android{{/|\\}}libclang_rt.builtins.a
+// CHECK-FILE-NAME-ANDROID23: lib{{/|\\}}aarch64-unknown-linux-android23{{/|\\}}libclang_rt.builtins.a
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN:     --target=aarch64-unknown-linux-android26 \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID23 %s
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN:     --target=aarch64-unknown-linux-android29 \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID29 %s
+// CHECK-FILE-NAME-ANDROID29: lib{{/|\\}}aarch64-unknown-linux-android29{{/|\\}}libclang_rt.builtins.a
+
+// RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
+// RUN:     --target=aarch64-unknown-linux-android31 \
+// RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID29 %s
 
 // RUN: %clang -rtlib=compiler-rt -print-file-name=libclang_rt.builtins.a 2>&1 \
 // RUN:     --target=aarch64-unknown-linux-android \
 // RUN:     -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN:   | FileCheck --check-prefix=CHECK-FILE-NAME-ANDROID %s
 // CHECK-FILE-NAME-ANDROID: lib{{/|\\}}aarch64-unknown-linux-android{{/|\\}}libclang_rt.builtins.a