Update Android NDK to 25

[bendk]

This is required to update Rust to 1.68 (#5435). The last time we tried to do this, we had linker errors (#5165).

┆Issue is synchronized with this Jira Task
┆Sprint End Date: 2023-03-24

[bendk]

And when I try this time I see the same error. AFAICT, we're running into issues because the newer NDK removed libgcc. It was replaced it with libunwind -- I guess libgcc was only needed for unwind support. Many projects have ran into this issue, here's some examples for context: rust-lang, android-ndk, rust-android-gradle. Maybe rust-android-gradle is the most relevant to us, since that's the plugin we use to build the megazord.

I've tried several things with no success:

• Using NDK 23.2.8568313
• Using NDK 25.2.9519653 (most of my testing was with 25.1.8937393 since that's what the previous PR used)
• Changing the NDK apiLevel in megazords/full/android/build.gradle.
  • 23 had the same issue
  • 25 caused the build to fail with "error occurred: Failed to find tool. Is i686-linux-android25-clang installed?"
• Upgrading Rust to 1.68 alongside the NDK update (based on the theory that the newer rust/cargo had better compatibility with the newer NDK).
• Upgrading Rust to 1.68 and the apiLevel to 23

I also hacked linker-wrapper.py to print out what it was doing and verified that the end result had -lunwind rather than -lgcc. Here's a dump of arglist, 1 item per line: https://gist.github.com/bendk/46e66c80a1f2a559af05b72701754a2b. Note: this is for the x86 library which actually works. I needed to force the script to fail to see the console output which made it hard to target the x86_64 build. However, I don't see any reason why that would be different.

[bendk]

The best way I could find to test this was to:

• Build the megazord maven package in app-services: cargo clean ; and ./gradlew clean; and ./gradlew full-megazord:assembleRelease (those clean parts seem needed, I don't think our gradle code is properly detecting changes).
• Check for the missing symbol:
  • llvm-objdump -T megazords/full/android/build/intermediates/library_and_local_jars_jni/release/jni/x86_64/libmegazord.so | grep __extenddftf2
  • Check for a line like 0000000000000000 D *UND* 0000000000000000 __extenddftf2 in the output.

[bendk]

I'm pretty sure this happens on x86-64, but not x86. When I repeat the above commands with megazords/full/android/build/intermediates/library_and_local_jars_jni/release/jni/x86/libmegazord.so, I don't see an undefined symbol.

I think this is why we have associated this issue with emulators, AFAIK x86-64 devices are rare in the wild.

[bendk]

Part of the issue may be our old friend sqlcipher. I notice that library has an undefined __extenddftf2 symbol. However, even when I disable it, I still see that undefined symbol in libmegazord.so, so maybe this is just part of the issue.

[bendk]

And I think the other end of the issue is NSS:

• libs/android/arm64-v8a/nss/lib/libnspr4.a and libs/android/x86_64/nss/lib/libnspr4.a have an undefined __extenddftf2 symbol
• I think arm64 doesn't have an issue because /home/bdk/.rustup/toolchains/1.68.0-x86_64-unknown-linux-gnu/lib/rustlib/aarch64-linux-android/lib/libcompiler_builtins-1081644b311849f7.rlib exports that symbol.
• However, /home/bdk/.rustup/toolchains/1.68.0-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-linux-android/lib/libcompiler_builtins-367de5b14a245593.rlib doesn't and that's causing our problem.

This leads to several questions:

• Is there a way to test that NSS and sqlcipher are the issue? I can remove the rusqlite/sqlcipher feature, which I think removes the sqlcipher dependency, but I don't know how to do anyhting similar with NSS.
• If this is a NSS/sqlcipher issue, who's fault is it? Should the Rust toolchain be exporting that symbol? Or should NSS/Sqlcipher not be depending on it? Should app-services be linking to libgcc somehow?
  • I don't think it's Rust's fault. The libcompiler_builtins library only exports the symbols that Rust uses, it intentionally doesn't implement __extenddftf2, since Rust doesn't support f128.

[glandium]

Can you get a complete build output (including when nspr/nss are built) from when you add -Wl,--verbose as a linker flag?

[glandium]

(FWIW, I'm fairly convinced this is Rust's fault. Also, unfortunately, what I asked for is probably not going to be helpful, but also adding -Wl,-z,defs would probably make it more useful)

[bendk]

Sorry, I got pulled away for another few issues, but I'll run some builds with those flags today.

Since moz-central landed the NDK23 update, I took a look at the Android 5.0 x86-64 debug libxul.so from autoland and it has a defined __extendddftf2 symbol:

 24346: 0000000007c19930   220 FUNC    GLOBAL DEFAULT   14 __extenddftf2

My hunch here is that libxul is statically linked to libgcc, which is why that symbol is defined. I see several reference to it that when I grep through the source.

[bendk]

Here's the build output that I got by:

• adding export LDFLAGS="-Wl,--verbose,-z,defs" to libs/build-nss-android.sh
• Running /build-all.sh android from the libs directory

build.txt is the entire output. build-nss.txt is a trimmed version that contains the NSS-specific part.
build.txt
build-nss.txt

[str4d]

Part of the issue may be our old friend sqlcipher. I notice that library has an undefined __extenddftf2 symbol.

This may actually be an SQLite problem, rather than the SQLCipher fork specifically. We see this same undefined symbol when compiling a project using the rusqlite crate with the bundled feature flag, which bundles SQLite rather than SQLCipher: mozilla/rust-android-gradle#105

[glandium]

My hunch here is that libxul is statically linked to libgcc

No, it's statically linked to libclang_rt.builtins, which contains the symbol. clang does that automatically. The problem is that rust explicitly tells it not to do that with -nodefaultlibs, and passes its own compiler-builtins instead, but completely ignores that it's not alone and that the crate could very well be building C code that uses symbols that aren't in its compiler-builtins but are in the C compiler ones.

At least that's what I'd expect the problem to be, but

build.txt is the entire output. build-nss.txt is a trimmed version that contains the NSS-specific part.

these logs don't contains libmegazord.

[bendk]

Rust may be doing something wrong, but ISTM that there's definitely something wrong with NSS/sqlcipher/sqlite, or at least the way we build them. Those are built separately, without any help from Rust/Cargo, and they produce static libraries with an undefined __extenddftf2 symbol.

I'm not sure how to get a log from the libmegazord build. We're using the rust-android-gradle to build it. Is there a good way to a) tell it to give verbose output and b) pass it linker flags?

[bendk]

What's odd is that I see many lines in the build.txt script above like this one:

  ld: /home/bdk/Android/Sdk/ndk/25.2.9519653/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/14.0.7/lib/linux/libclang_rt.builtins-x86_64-android.a

However, the end result still has undefined symbols. So it seems like it's trying to link to libclang_rt.builtins-x86_64-android.a, but failing somehow.

[bendk]

#5442 contains a hack that fixes things for me. @glandium what do you think? Is there a better way to do this?

[glandium]

What's odd is that I see many lines in the build.txt script above like this one:

  ld: /home/bdk/Android/Sdk/ndk/25.2.9519653/toolchains/llvm/prebuilt/linux-x86_64/lib64/clang/14.0.7/lib/linux/libclang_rt.builtins-x86_64-android.a

However, the end result still has undefined symbols. So it seems like it's trying to link to libclang_rt.builtins-x86_64-android.a, but failing somehow.

Those come from linking the .so files. The .a archives (static libs) don't contain the C runtime, and that's how it's supposed to be. The C runtime is only linked once, in dynamic libs/executables. Adding it to multiple .a that may all be linked together could be a source of build failures (although in practice, probably not).

#5442 contains a hack that fixes things for me. @glandium what do you think? Is there a better way to do this?

A better option would be to add libclang_rt.builtins to the link-args for rust.

[bendk]

A better option would be to add libclang_rt.builtins to the link-args for rust.

Sounds good. I updated this PR to take that approach, what you you think of this commit?

The .a archives (static libs) don't contain the C runtime, and that's how it's supposed to be.

This is surprising to me because the static libs didn't used to have the undefined symbol with NDK21 and with NDK25 only the x86-64 lib has the undefined symbol. I'm asking because I want to know how to view the changes:

• Is it a workaround for linking to faulty libraries and therefore we should try to revert the change when we do an NDK upgrade?
• Is it fixing an error in our build scripts that is only becoming apparent now and therefore we should also do the same thing for all arches, not just x86-64?

[glandium]

Sounds good. I updated this PR to take that approach, what you you think of this commit?

That seems fine.

This is surprising to me because the static libs didn't used to have the undefined symbol with NDK21 and with NDK25 only the x86-64 lib has the undefined symbol.

The undefined symbols are generated by clang, because it's using those builtins for some float operations, now. It assumes the corresponding runtime is going to be linked at the end, which doesn't happen because of -nodefaultlibs, which rust adds. I'll file a bug against the rust compiler with a reproducer without the NDK.

[glandium]

with a reproducer without the NDK

turns out, the rust compile saves itself on Linux by linking libgcc_s, so it's only really a problem with the NDK.

[bendk]

This got merged in #5442