ASM files broken with legacy CMake toolchain file and CMake 3.22.1

[rprichard]

We saw a bug like this recently and fixed it (https://android-review.googlesource.com/c/platform/ndk/+/1894539/), but it's broken again. This issue affects the legacy CMake toolchain file when using CMake 3.22.1 (presumably 3.19 and up are broken):

FAIL build-assembly-file [x86-19-legacy]:
[1/2] /x/ndk/out/linux/android-ndk-r24-canary/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -target i686-none-linux-android19 -gcc-toolchain /x/ndk/out/linux/android-ndk-r24-canary/toolchains/llvm/prebuilt/linux-x86_64 --sysroot=/x/ndk/out/linux/android-ndk-r24-canary/toolchains/llvm/prebuilt/linux-x86_64/sysroot -Dtest_build_assembly_EXPORTS  -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -mstackrealign -D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security  -fPIC -MD -MT CMakeFiles/test_build_assembly.dir/jni/assembly-x86.S.o -MF CMakeFiles/test_build_assembly.dir/jni/assembly-x86.S.o.d -o CMakeFiles/test_build_assembly.dir/jni/assembly-x86.S.o -c /x/ndk/out/tests/obj/x86-19-legacy/cmake/build-assembly-file/jni/assembly-x86.S
FAILED: CMakeFiles/test_build_assembly.dir/jni/assembly-x86.S.o 
/x/ndk/out/linux/android-ndk-r24-canary/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -target i686-none-linux-android19 -gcc-toolchain /x/ndk/out/linux/android-ndk-r24-canary/toolchains/llvm/prebuilt/linux-x86_64 --sysroot=/x/ndk/out/linux/android-ndk-r24-canary/toolchains/llvm/prebuilt/linux-x86_64/sysroot -Dtest_build_assembly_EXPORTS  -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -mstackrealign -D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security  -fPIC -MD -MT CMakeFiles/test_build_assembly.dir/jni/assembly-x86.S.o -MF CMakeFiles/test_build_assembly.dir/jni/assembly-x86.S.o.d -o CMakeFiles/test_build_assembly.dir/jni/assembly-x86.S.o -c /x/ndk/out/tests/obj/x86-19-legacy/cmake/build-assembly-file/jni/assembly-x86.S
clang: error: unknown argument: '-gcc-toolchain'
clang: warning: /x/ndk/out/linux/android-ndk-r24-canary/toolchains/llvm/prebuilt/linux-x86_64: 'linker' input unused [-Wunused-command-line-argument]
ninja: build stopped: subcommand failed.

The legacy toolchain file is used by default for CMake versions less than 3.21.

CMake's Clang compiler version detection is broken for ASM mode, which seems like an upstream bug:

$ cat CMakeLists.txt 
cmake_minimum_required(VERSION 3.6.0)
project(test C CXX ASM)

message(STATUS "ASM ver = ${CMAKE_ASM_COMPILER_VERSION}")
message(STATUS "C ver = ${CMAKE_C_COMPILER_VERSION}")
message(STATUS "CXX ver = ${CMAKE_CXX_COMPILER_VERSION}")

message(STATUS "ASM flag = ${CMAKE_ASM_COMPILE_OPTIONS_EXTERNAL_TOOLCHAIN}")
message(STATUS "C flag = ${CMAKE_C_COMPILE_OPTIONS_EXTERNAL_TOOLCHAIN}")
message(STATUS "CXX flag = ${CMAKE_CXX_COMPILE_OPTIONS_EXTERNAL_TOOLCHAIN}")

message(STATUS "ASM toolchain = ${CMAKE_ASM_COMPILER_EXTERNAL_TOOLCHAIN}")
message(STATUS "C toolchain = ${CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN}")
message(STATUS "CXX toolchain = ${CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN}")

$ /x/ndk/prebuilts/cmake/linux-x86/bin/cmake --version
cmake version 3.22.1-gb902a97

$ CC=clang CXX=clang++ /x/ndk/prebuilts/cmake/linux-x86/bin/cmake -GNinja .
-- The C compiler identification is Clang 11.1.0
-- The CXX compiler identification is Clang 11.1.0
-- The ASM compiler identification is Clang with GNU-like command-line
-- Found assembler: /usr/bin/clang
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/clang - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/clang++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- ASM ver = 
-- C ver = 11.1.0
-- CXX ver = 11.1.0
-- ASM flag = -gcc-toolchain 
-- C flag = --gcc-toolchain=
-- CXX flag = --gcc-toolchain=
-- ASM toolchain = 
-- C toolchain = 
-- CXX toolchain = 
-- Configuring done
-- Generating done
-- Build files have been written to: /x/mess/A

It only affects the legacy toolchain because only the legacy toolchain sets the CMAKE_${LANG}_COMPILER_EXTERNAL_TOOLCHAIN flags, which result in passing -gcc-toolchain/--gcc-toolchain=. The new toolchain file doesn't.

I don't know what this flag is used for, and maybe it's not actually needed anymore, so we can remove it from the legacy toolchain.

$ /x/ndk/prebuilts/cmake/linux-x86/bin/cmake -GNinja . -DCMAKE_TOOLCHAIN_FILE=/x/android-ndk-r24-beta1/build/cmake/android.toolchain.cmake -DANDROID_ABI=armeabi-v7a -DANDROID_PLATFORM=android-19 -DANDROID_USE_LEGACY_TOOLCHAIN_FILE=OFF
...
-- ASM ver = 
-- C ver = 13.0.2
-- CXX ver = 13.0.2
-- ASM flag = -gcc-toolchain 
-- C flag = --gcc-toolchain=
-- CXX flag = --gcc-toolchain=
-- ASM toolchain = 
-- C toolchain = 
-- CXX toolchain = 
...
$ /x/ndk/prebuilts/cmake/linux-x86/bin/cmake -GNinja . -DCMAKE_TOOLCHAIN_FILE=/x/android-ndk-r24-beta1/build/cmake/android.toolchain.cmake -DANDROID_ABI=armeabi-v7a -DANDROID_PLATFORM=android-19 -DANDROID_USE_LEGACY_TOOLCHAIN_FILE=ON
...
-- ASM ver = 
-- C ver = 13.0.2
-- CXX ver = 13.0.2
-- ASM flag = -gcc-toolchain 
-- C flag = --gcc-toolchain=
-- CXX flag = --gcc-toolchain=
-- ASM toolchain = /x/android-ndk-r24-beta1/toolchains/llvm/prebuilt/linux-x86_64
-- C toolchain = /x/android-ndk-r24-beta1/toolchains/llvm/prebuilt/linux-x86_64
-- CXX toolchain = /x/android-ndk-r24-beta1/toolchains/llvm/prebuilt/linux-x86_64
...

Updating CMake from 3.18.1 to 3.22.1 breaks the legacy toolchain file, because it disables the recent NDK fix, which only applies to versions older than CMake 3.19:

https://android.googlesource.com/platform/ndk/+/refs/tags/ndk-r24-beta1/build/cmake/android-legacy.toolchain.cmake#414

A fix is to enable that compiler ID override for all versions of CMake.

It would appear that there's a range of CMake versions where ASM files currently can't be compiled, starting with 3.19 and ending with 3.20.x. 3.21 and newer use the non-legacy toolchain file and so aren't affected.

[rprichard]

I don't think this needs to block a new CMake release? By default, CMake 3.22.1 would use the new toolchain file, which isn't affected.

I'm not sure how hard it is to fix CMake's version detection.

[rprichard]

CMake's Clang compiler version detection is broken for ASM mode, which seems like an upstream bug

I filed https://gitlab.kitware.com/cmake/cmake/-/issues/22995 for this issue.

[DanAlbert]

I don't know what this flag is used for, and maybe it's not actually needed anymore, so we can remove it from the legacy toolchain.

It's not actually in the legacy toolchain (file, maybe you meant the CMake side of the legacy toolchain workflow). git grep gcc-toolchain shows nothing in the NDK. Seems CMake is adding that, but I can't explain why it's only doing it in the legacy mode.

I don't think this needs to block a new CMake release? By default, CMake 3.22.1 would use the new toolchain file, which isn't affected.

+1.

[rprichard]

It's not actually in the legacy toolchain [...]

There's too much indirection.

There are two flags:

• CMAKE_${lang}_COMPILE_OPTIONS_EXTERNAL_TOOLCHAIN: this is either -gcc-toolchain or --gcc-toolchain=
• CMAKE_${lang}_COMPILER_EXTERNAL_TOOLCHAIN: this is usually blank but can be a pathname

If ${..._COMPILER_...} is set, then CMake passes ${..._COMPILE_OPTIONS_...} ${..._COMPILER_...} to clang.

Only the legacy toolchain sets the ..._COMPILER_... variables:
https://android.googlesource.com/platform/ndk/+/refs/tags/ndk-r24-beta2/build/cmake/android-legacy.toolchain.cmake#404

I think we can remove these lines, which fixes newer NDKs. It doesn't fix the older NDK + newest CMake + legacy-toolchain-file combo, but that's presumably rare. The NDK defaults to the non-legacy toolchain, so the user must override it with -DANDROID_USE_LEGACY_TOOLCHAIN_FILE=ON.

I don't think this needs to block a new CMake release? [...]

It occurred to me that I can't update the cmake prebuilt until fixing (some part of) this issue, so I think I actually want to fix it first.