[BUG] undefined reference to `__write_chk'

[huangqinjin]

Description

CMakeLists.txt

cmake_minimum_required(VERSION 3.6)
project (Test)
add_executable(Test main.cpp)
find_package(ZLIB REQUIRED)
target_link_libraries(Test ZLIB::ZLIB)

main.cpp

#include <zlib.h>
int main() { gzwrite(0,0,0); }

cmake .. -DCMAKE_TOOLCHAIN_FILE=${NDK}/build/cmake/android.toolchain.cmake -DANDROID_PLATFORM=23 && cmake --build .

bionic/libc/include/bits/fortify/unistd.h:174: error: undefined reference to '__write_chk'

No error with NDK 20.1.5948944 or NDK API level 24 and above.

Environment Details

• NDK Version: 21.0.6113669
• Build system: CMake
• Host OS: Linux
• ABI: any
• NDK API level: 23

[enh-google]

is -DANDROID_PLATFORM not the right way to declare that you're targeting API level 23 with cmake?

here's what line 174 of bits/fortify/unistd.h looks like, so we shouldn't be making the call at API level 23:

#if __ANDROID_API__ >= 24 && __BIONIC_FORTIFY_RUNTIME_CHECKS_ENABLED
    size_t bos = __bos0(buf);

    if (!__bos_trivially_ge_no_overflow(bos, count)) {
        return __write_chk(fd, buf, count, bos);
    }
#endif

[huangqinjin]

Should be right according to ANDROID_PLATFORM. No error if replace gzwrite with write. Directly use compiler ndk/21.0.6113669/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android23-clang produces same error.

[enh-google]

i assume this is telling cmake to statically link libz? it actually looks like there's a mistake in how we built the libz for API level 23:

~/Downloads/android-ndk-r21$ readelf -aW ./platforms/android-23/arch-arm/usr/lib/libz.a | grep write_chk
00000058  0000b21c R_ARM_CALL             00000000   __write_chk
000000e0  0000b21c R_ARM_CALL             00000000   __write_chk
   178: 00000000     0 NOTYPE  GLOBAL DEFAULT  UND __write_chk
~/Downloads/android-ndk-r21$ 

[enh-google]

yeah, looks like we only have one libz.a per architecture (which i already believed to be true), but that we didn't build it at the oldest supported API level (which is surprising to me)?

4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/22/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/17/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/16/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/23/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/26/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/24/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/29/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/21/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/19/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/18/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/27/libz.a
4ea2819c68e5effe3525c2edf7387824  ./toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/28/libz.a

[enh-google]

(oddly they don't appear to be built with the usual ELF note that would tell me what API level we thought we were targeting.)

[DanAlbert]

but that we didn't build it at the oldest supported API level (which is surprising to me)?

The static libs in the sysroot are mostly only for static executables (libc.a isn't going to help in JNI), so they're only built for the latest API level. For libz we may want to do differently.

@huangqinjin are you using the static libz intentionally, or is that just by accident because it's what CMake chose for you with find_package? find_package isn't normally how you would use a library from the sysroot in Android, and I wouldn't be surprised if the CMake module that handles that is wrong for Android (perhaps only with the version of CMake you're using).

[DanAlbert]

(oddly they don't appear to be built with the usual ELF note that would tell me what API level we thought we were targeting.)

Where "they" is libz.a? They can't be. The note file is part of the CRT object. libz.a isn't linked yet.

[enh-google]

no, i looked at the corresponding .so files --- which i realize now don't really "correspond" in any useful sense, since they're just stubs.

[DanAlbert]

Ah. I think in that case it's because the stubs aren't built as NDK libraries, so they're not built with the NDK CRT objects. Like you say, they're just stubs so it doesn't make much of a difference.

[h6197627]

Same error when targeting 21 API level with zlib.a
Is it save to take zlib.a libraries from r20 NDK and use it with current r21 version?

[DanAlbert]

If you're building a static executable you need to target the maximum supported API level for that NDK. For r21 that means 29.

If you're not building a static executable, don't use libz.a; use libz.so.

[Elv1zz]

I am building a statically linked shared library. So I statically link OpenCV into my library, which itself is a .so file to be loaded by my app. I have to support Android API level 18 and later. Everything worked fine until NDK 21.0.6113669, when I also started to get the mentioned undefined reference to '__write_chk' error at link time.

So according to this solution, I would have to compile for API level 29, which compiles and links, but crashes at runtime (when loading the shared library) with an unknown reference to __register_atfork on Android devices with API level < 23. The solution to that is to compile for the minimum API level supported, which is 18 in my case. So I have two opposing requirements for which API level to compile for.

So what to do?

[hzung]

I got the same issue and I had to back to the ndk version 19.2.5345600 to make it work around.

Note: Make sure that you update the ndk configuration in local.properties to use the other version.

sdk.dir=/path/to/the/Android/sdk
ndk.dir=/path/to/the/Android/sdk/ndk/19.2.5345600

Hope this helps.

[DanAlbert]

@Elv1zz Target API 18 and use libz.so, not libz.a.

[huangqinjin]

According to find_library search process 6 and CMake source code of cmFindBase::FillCMakeSystemVariablePath() and cmSearchPath::AddPrefixPaths(), the search process is

1. ${CMAKE_SYSROOT}/${CMAKE_SYSTEM_PREFIX_PATH}/lib/${CMAKE_LIBRARY_ARCHITECTURE}
2. ${CMAKE_SYSROOT}/${CMAKE_SYSTEM_PREFIX_PATH}/lib
3. ${CMAKE_SYSROOT}/${CMAKE_SYSTEM_PREFIX_PATH} # undocumented???
4. ${CMAKE_SYSROOT}/${CMAKE_SYSTEM_LIBRARY_PATH}

And variables set in Android toolchain:

• CMAKE_SYSTEM_PREFIX_PATH="/usr/local;/usr"
• CMAKE_LIBRARY_ARCHITECTURE="arm-linux-androideabi"
• CMAKE_SYSTEM_LIBRARY_PATH="/usr/lib/arm-linux-androideabi/24"

So CMake found /usr/lib/arm-linux-androideabi/libz.a by (1) instead of /usr/lib/arm-linux-androideabi/24/libz.so by (4).

One solution is to prepend /usr/lib/arm-linux-androideabi/24 to CMAKE_SYSTEM_PREFIX_PATH, such that CMake can find libz by undocumented (3).

Another solution, is it possible to remove /usr/lib/arm-linux-androideabi/libz.a from NDK bundle @DanAlbert ? Actually this issue is same as #929 .

[DanAlbert]

Actually this issue is same as #929 .

Sure looks that way. Will dup this to that one.

Another solution, is it possible to remove /usr/lib/arm-linux-androideabi/libz.a from NDK bundle

Not without breaking a different set of users :(

I'd really like to remove both from the NDK proper and expose them as an AAR instead, since relying on the system's libz.so means getting an old libz.so on old devices, but I suspect that migration will take a long time to avoid breaking people (and I haven't made such an AAR yet).

[alexcohn]

I'd really like to remove both from the NDK proper and expose them as an AAR instead

That's true. We never use the platform libz for production.

But I should repeat that the easiest cure for the meanwhile is to drop find_package() and simply use

target_link_libraries(Test z)

[DanAlbert]

Yep, definitely using plain old target_link_libraries for system libs is the way to go. (I'd been wondering about removing the find_library(LIBLOG log) from the new project template for a while, and this has me fairly convinced)

[huangqinjin]

But I should repeat that the easiest cure for the meanwhile is to drop find_package()

I don't agree. The standard way in CMake world is to use find_package(). Actually I encoutered this issue when I built OpenCV. Not all 3rd libraries directly use target_link_libraries(Test z).

If we would like to benefit from CMake's cross-platform ablity for NDK development, we need follow the CMake way.

[DanAlbert]

Yeah, just to be clear, no argument against the idiomatic approach working on Android :) We'll get it fixed, I just can't be sure on the timeline. I suspect this one is a fair amount of work to do right.

[alexcohn]

@huangqinjin I believe you are right in the long run. But actually the correct way to treat this would be to switch to a prefab external up-to-date libz, as @DanAlbert suggested above.

My suggestion is but an immediate workaround.

[timofey-retailnext]

I found same libz.a issue with undefined __chk_write and __chk_read during opencv build (cmake).
Am I right that the only bypass is not to use libz.a, but use ure libz.so instead ? (this doesn work!)
Are there any plans to fix it in ndk r21e ?

[alexcohn]

@timofey-retailnext the best is not to rely on system libz. I believe you can choose an external libz for opencv build.

[enh-google]

#929 should be fixed in NDK r22beta1, though, right? so you should automatically get libz.so there rather than libz.a, and that should just work?

[DanAlbert]

The fix for #929 had to be reverted because it broke AGP. Unsure when we'll be able to fix it again.

(this doesn work!)

I'm not clear on what's broken. File a bug?

[enh-google]

should we just remove libz.a? i wonder whether it's causing far more trouble than it's worth. who's using it on purpose, and for what?

[DanAlbert]

Static executables won't have a choice. Removing it might break things like toybox? Unsure if it uses libz or not.

[enh-google]

but your static toybox is going to be a bit broken anyway. there's no static liblog (which is an NDK library) let alone libcutils (which isn't). and like alex said, you can always include your own libz build if that's actually deliberate.

it's the "how many people are being hurt by this?" side of the question i'm unclear on...

[timofey-retailnext]

What lib should normally provide __write_chk and __read_chk at link time ? it is linked locally to an .SO (and thus resolves these undefined symbols) ? if yes, then it should be done for .A too ... (or .A to be thrown away completely).
In contrast, we may add impl of these missing entries directly to a link list of our app/lib ...

[enh-google]

they're in libc, as part of the fortify implementation.

__write_chk: https://android.googlesource.com/platform/bionic/+/master/libc/bionic/fortify.cpp#448

__read_chk: https://android.googlesource.com/platform/bionic/+/master/libc/bionic/fortify.cpp#201

[huangqinjin]

Currently I use the first method I mentioned above and it requires CMAKE_SYSROOT to be set. @timofey-retailnext you may try the following patch.

diff --git a/android.toolchain.cmake b/android.toolchain.cmake
index c3a56a9..9118a71 100644
--- a/android.toolchain.cmake
+++ b/android.toolchain.cmake
@@ -410,12 +410,14 @@ list(APPEND CMAKE_PREFIX_PATH "${ANDROID_TOOLCHAIN_ROOT}")
 # TODO: Teach Studio to recognize Android builds based on --target.
 set(CMAKE_SYSROOT "${ANDROID_TOOLCHAIN_ROOT}/sysroot")
 
 # Allows CMake to find headers in the architecture-specific include directories.
 set(CMAKE_LIBRARY_ARCHITECTURE "${ANDROID_TOOLCHAIN_NAME}")
 
+set(CMAKE_SYSTEM_PREFIX_PATH "/usr/lib/${ANDROID_TOOLCHAIN_NAME}/${ANDROID_PLATFORM_LEVEL};${CMAKE_SYSTEM_PREFIX_PATH}")
+
 # Instructs CMake to search the correct API level for libraries.
 list(APPEND CMAKE_SYSTEM_LIBRARY_PATH
   "/usr/lib/${ANDROID_TOOLCHAIN_NAME}/${ANDROID_PLATFORM_LEVEL}")
 
 set(ANDROID_HOST_PREBUILTS "${ANDROID_NDK}/prebuilt/${ANDROID_HOST_TAG}")
 

[DanAlbert]

Let's move this discussion to the main bug: #929

[Luiz-Monad]

"how many people are being hurt by this?

A lot !

[DanAlbert]

It's already fixed in r23. See the comment before yours for the up to date info.

[Luiz-Monad]

I'll upgrade and try again, thanks.

[timofey-retailnext]

ndk r22d claimed to fix similar issue (#1391 - missing cpu_features). but __write_chk/__read_chk issues is still there. r23-beta2 also has it. and r21e.

[enh-google]

ndk r22d claimed to fix similar issue (#1391 - missing cpu_features). but __write_chk/__read_chk issues is still there. r23-beta2 also has it. and r21e.

that was unrelated: the _cpu_check_features issue was with the zlib code itself. __write_chk/__read_chk relates to the API level zlib was built for. in a big enough system, there's going to be more than one bug :-)

[DanAlbert]

The problem reported here (CMake picking the static library by default instead of the shared library) has already been fixed.

$ cat ./foo.cpp
#include <zlib.h>
int main(int argc, char** argv) {
  gzwrite(0, 0, 0);
  return 0;
}
$ cat ./CMakeLists.txt
cmake_minimum_required(VERSION 3.6.0)
project(Test)

add_executable(foo foo.cpp)
find_library(ZLIB z)
target_link_libraries(foo ${ZLIB})
$ cat ./build.sh
#!/bin/sh
if [ $# -lt 1 ]; then
  >&2 echo "usage: ./gen.sh NDK_PATH"
  exit 1
fi

set -e

NDK_PATH=$(realpath $1)

cd "$(dirname "$0")"
rm -rf build
mkdir build
cd build
cmake -DCMAKE_TOOLCHAIN_FILE=$NDK_PATH/build/cmake/android.toolchain.cmake -GNinja ..

ninja -v

This builds just fine with r23. As does the find_package variant:

find_package(ZLIB REQUIRED)
target_link_libraries(foo ZLIB::ZLIB)

If there's still a problem we don't know about it and you need to file a new bug with a repro case.