gn build: Fix Android build.

[pcc]

Summary

Remove --unwindlib=none flag that was added by D143598. It is required
to not pass this flag when linking binaries. With this flag, linking
e.g. llvm-symbolizer will fail.

There was a missing dependency from most targets to the implicitly linked
libraries (libunwind and builtins), which was causing the build for those
targets to fail when they were built on their own. I never noticed this
because I always build the implicitly linked libraries together with
their dependencies. Make the dependency explicit by introducing a new
target //llvm/utils/gn/build/libs/implicit representing the platform's
implicitly linked libraries, and depend on that from the LLVM libraries,
so that "normal" binaries like llvm-symbolizer will have all of their
dependencies available.

D143598 set the arch subdirectory to i686 for Android. However, the arch
subdirectory on Android is i386, and has been for a long time.

$ find android-ndk-r* -name i386
android-ndk-r18b/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/7.0.2/lib/linux/i386
android-ndk-r19/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/8.0.2/lib/linux/i386
android-ndk-r20/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/8.0.7/lib/linux/i386
android-ndk-r21-beta1/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/9.0.8/lib/linux/i386
android-ndk-r21d/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/9.0.8/lib/linux/i386
android-ndk-r22/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/11.0.5/lib/linux/i386
android-ndk-r23-beta5/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/12.0.5/lib/linux/i386
android-ndk-r24/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/14.0.1/lib/linux/i386
android-ndk-r25c/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/14.0.7/lib/linux/i386

Using the wrong name prevents Clang from being able to find libunwind.a on
x86. Fix the code to use i386 as the arch subdirectory, consistent with
the NDK.

Bring back -Wl,-z,defs which was removed for Android by D143598, presumably
because of the missing unwind library.

[pcc]

Fix commit message

[aeubanks]

yeah this was very confusing when I was looking at this

$ fd i386 ~/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/12.0.5/lib/linux/i386/
$ fd i686 ~/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/
...
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/12.0.5/lib/linux/libclang_rt.ubsan_standalone_cxx-i686-android.a # and lots of other compiler-rt libs
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/i686-linux-android/
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/i686-linux-android/
...

building with this patch, I still get ld.lld: error: unable to find library -l:libunwind.a for all the different arches

perhaps the Android NDK in Chromium is not the standard one? are you using the one bundled in Chromium?

[srhines]

In D146266#4200993, @aeubanks wrote:

yeah this was very confusing when I was looking at this

$ fd i386 ~/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/12.0.5/lib/linux/i386/
$ fd i686 ~/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/
...
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/12.0.5/lib/linux/libclang_rt.ubsan_standalone_cxx-i686-android.a # and lots of other compiler-rt libs
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/i686-linux-android/
/usr/local/google/home/aeubanks/repos/chromium/src/third_party/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/i686-linux-android/
...

building with this patch, I still get ld.lld: error: unable to find library -l:libunwind.a for all the different arches

perhaps the Android NDK in Chromium is not the standard one? are you using the one bundled in Chromium?

I'm pretty sure that Peter is building/using the toolchain with the Android platform, so it might be picking up NDK parts from the platform build. As for the one in Chromium, I see a "12.0.5" in there, which means that it is quite old. You may want to consider updating that in any case. Even NDK r25 from last year is on ~Clang-14. https://github.com/android/ndk/wiki/Changelog-r25#r25c shows a lot of other nice improvements too in terms of repo size, so that might be a real nice win for Chromium devs.

[pcc]

After an out of band discussion with Arthur, we figured out the root cause of his problem and I was able to reproduce. This fixes it for me.

[aeubanks]

llvm/utils/gn/secondary/llvm/lib/Support/BUILD.gn

7 ↗

(On Diff #505968)

why put this in Support rather than something like executable (which you had mentioned trying)?

[pcc]

llvm/utils/gn/secondary/llvm/lib/Support/BUILD.gn

7 ↗

(On Diff #505968)

Unfortunately that wouldn't work, because the deps attribute is not allowed on config.

https://gn.googlesource.com/gn/+/main/docs/reference.md#func_config

[pcc]

Bring back -Wl,-z,defs for Android

[aeubanks]

I'd like @thakis to review this change, he probably has more thoughts

[pcc]

llvm/utils/gn/secondary/llvm/lib/Support/BUILD.gn

7 ↗

(On Diff #505968)

I just remembered how this is done in Chromium, they define template wrappers around the target types that invoke the linker: https://source.chromium.org/chromium/chromium/src/+/main:build/config/BUILDCONFIG.gn;l=512

That's a much better approach, so I'll switch to that.

[pcc]

Switch to template wrappers

[aeubanks]

ping @thakis

[pcc]

Ping; still needed to build for Android.

[thakis]

lg

(aeubanks, feel free to approve changes like this!)

llvm/utils/gn/build/BUILDCONFIG.gn

53

This is a bit yucky. Maybe we could do this if (current_os == "android") for now, since we at least currently only need it there? (If so, let group("implicit") only exist on android too)

[aeubanks]

(reverse ping)

llvm/utils/gn/build/BUILD.gn

config("no_rtti") {
   }
   cflags_objcc = cflags_cc
 }
 
 config("zdefs") {
   # -Wl,-z,defs doesn't work with sanitizers.
   # https://clang.llvm.org/docs/AddressSanitizer.html
   if (current_os != "ios" && current_os != "mac" && current_os != "win" &&
-      current_os != "android" && !(use_asan || use_tsan || use_ubsan)) {
+      !(use_asan || use_tsan || use_ubsan)) {
     ldflags = [ "-Wl,-z,defs" ]
   }
 }
 
 # To make an archive that can be distributed, you need to remove this config and
 # set complete_static_lib.
 config("thin_archive") {
   if (current_os != "ios" && current_os != "mac" && current_os != "win") {

llvm/utils/gn/build/BUILDCONFIG.gn

 
 if (host_os == "win") {
   host_toolchain = "//llvm/utils/gn/build/toolchain:win"
 } else {
   host_toolchain = "//llvm/utils/gn/build/toolchain:unix"
 }
 
 set_default_toolchain(host_toolchain)
+
+if (current_os == "android") {
+  foreach(target_type,
+          [
+            "executable",
+            "loadable_module",
+            "shared_library",
+          ]) {
+    template(target_type) {
+      target(target_type, target_name) {
+        forward_variables_from(invoker, "*")
+        if (!defined(deps)) {
+          deps = []
+        }
+        deps += [ "//llvm/utils/gn/build/libs/implicit" ]
+      }
+    }
+  }
+}

llvm/utils/gn/build/libs/implicit/BUILD.gn

+# This target represents the library dependencies that are implicitly linked by
+# the compiler.
+if (current_os == "android") {
+  group("implicit") {
+    deps = [
+      "//compiler-rt/lib/builtins",
+      "//libunwind/src:unwind_static",
+    ]
+  }
+}

llvm/utils/gn/build/toolchain/target_flags.gni

 
 if (current_os == "android") {
   target_flags += [
     "--target=$llvm_current_triple",
     "--sysroot=$android_ndk_path/toolchains/llvm/prebuilt/linux-x86_64/sysroot",
     "--gcc-toolchain=$android_ndk_path/toolchains/llvm/prebuilt/linux-x86_64",
     "-fno-emulated-tls",
   ]
-  target_ldflags += [
-    "-static-libstdc++",
-    "--unwindlib=none",
-  ]
+  target_ldflags += [ "-static-libstdc++" ]
   if (current_cpu == "arm") {
     target_flags += [ "-march=armv7-a" ]
   }
 } else if (current_os == "ios" || current_os == "mac") {
   if (current_cpu == "arm64") {
     target_flags += [
       "-arch",
       "arm64",

llvm/utils/gn/secondary/libunwind/src/BUILD.gn

 if (current_os == "android") {
   if (current_cpu == "arm64") {
     unwind_output_dir = "$crt_current_out_dir/aarch64"
   } else if (current_cpu == "arm") {
     unwind_output_dir = "$crt_current_out_dir/arm"
   } else if (current_cpu == "x64") {
     unwind_output_dir = "$crt_current_out_dir/x86_64"
   } else if (current_cpu == "x86") {
-    unwind_output_dir = "$crt_current_out_dir/i686"
+    unwind_output_dir = "$crt_current_out_dir/i386"
   }
 } else {
   unwind_output_dir = runtimes_dir
 }
 
 config("unwind_config") {
   cflags = []
   cflags_c = [ "-std=c99" ]