Improve Android build (Clang + tidyness) by RandomShaper · Pull Request #6958 · godotengine/godot

RandomShaper · 2016-10-28T22:29:14Z

This is something I wanted to do for a long time.

This must be heavily tested (different combinations of target architectures and build variants, etc.). ~~Not possible on Windows until #6844 is solved. For testing on Windows, merge #6959 first.~~

### WARNING: This is not ready to be tested/merged until I fix the Mac build since I will be pushing test commits until I get it to work. I'll remove this notice as soon as that's done.

Compile source files with Clang (Google dropped support for GCC in the NDK so Clang is the only recommended compiler). Anyway, like the NDK does, the binutils are those from GNU.
Dropped NDK_TARGET/NDK_TARGET_X86 since only one Clang version is included, thus no choice possible. Regarding binutils and gnu-libstdc++ the NDK also includes only one version (4.9), so no choice there either.
Remove a lot of almost duplicate code and replace with more readable constructs and variables.
Remove dead code (old commented code).
Remove unneeded flags for android_stl=yes.
Remove explicit specification of libraries that the linker will be adding anyway (c, m, gcc).
Disable RTTI for non-STL-enabled builds (the general case).
Restrict math optimizations to release/release_debug.
Try to set the compile/link flags as close as possible to what the NDK does (based on r13), which includes new stuff like strong stack protection and other security measures.
Instruct the linker to drop unused functions (hopefully this will reduce the resulting binary size and won't break things).
Enable multithreaded linking where available.
Enabled linker optimization (not true LTO by now; mainly string optimization).
~~Fix cases of bad indentation.~~ (Superseeded by the new PEP8 effort).

akien-mga · 2016-10-29T09:12:17Z

Fails on travis:

/usr/local/opt/android-ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/darwin-x86_64/arm-linux-androideabi/bin/ranlib modules/freetype/libfreetype_builtin.android.debug.armv7.neon.a
/usr/local/opt/android-ndk/toolchains/llvm/prebuilt/darwin-x86_64/bin/clang++ -o bin/libgodot.android.debug.armv7.neon.so --sysroot=/usr/local/opt/android-ndk/platforms/android-14/arch-arm -target armv7-none-linux-androideabi -target armv7-none-linux-androideabi -Wl,--no-undefined -Wl,-z,noexecstack -Wl,-z,relro -Wl,-z,now -fno-integrated-as -gcc-toolchain /usr/local/opt/android-ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/darwin-x86_64/arm-linux-androideabi/bin -Wl,-soname,libgodot_android.so -Wl,--gc-sections -shared platform/android/os_android.os platform/android/godot_android.os platform/android/file_access_android.os platform/android/dir_access_android.os platform/android/audio_driver_opensl.os platform/android/file_access_jandroid.os platform/android/dir_access_jandroid.os platform/android/thread_jandroid.os platform/android/audio_driver_jandroid.os platform/android/ifaddrs_android.os platform/android/android_native_app_glue.os platform/android/java_glue.os platform/android/cpu-features.os platform/android/java_class_wrapper.os -L/usr/local/opt/android-ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/darwin-x86_64/lib/gcc/arm-linux-androideabi/4.9.x -L/usr/local/opt/android-ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/darwin-x86_64/arm-linux-androideabi/lib main/libmain.android.debug.armv7.neon.a modules/libmodules.android.debug.armv7.neon.a bin/tests/libtests.android.debug.armv7.neon.a drivers/libdrivers.android.debug.armv7.neon.a scene/libscene.android.debug.armv7.neon.a servers/libservers.android.debug.armv7.neon.a core/libcore.android.debug.armv7.neon.a -lOpenSLES -lEGL -lOpenSLES -landroid -llog -lGLESv1_CM -lGLESv2 -lz modules/freetype/libfreetype_builtin.android.debug.armv7.neon.a
ld: unknown option: --sysroot=/usr/local/opt/android-ndk/platforms/android-14/arch-arm
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

https://travis-ci.org/godotengine/godot/jobs/171559489

RandomShaper · 2016-10-29T10:05:06Z

Yes, on Darwin I'm having an issue with the --sysroot linker option. I'll keep pushing until I get it to work since I haven't a Mac build environment. On Windows it works flawlessly for me. If anbody could test on Linux and report, it would be appreciated.

akien-mga · 2016-10-30T11:18:38Z

It fails building for all platforms on travis, but I'm not sure why.

akien-mga · 2016-10-30T11:19:32Z

Ah now I know why, you're forcing the build to stop unless it's the android one :)

RandomShaper · 2016-10-30T21:00:32Z

Now, unless STL support is enabled, RTTI is also disabled as it looks safe Godot-wide. I only have needed to make regex aware.

I've setup a local Mac dev environment and there the build has been correct. Let's hope Travis thinks the same…

RandomShaper · 2016-11-01T00:18:35Z

Rebased.

leezh · 2016-11-01T06:50:53Z

I think from a maintenance standpoint it'd be better to use is_look_behind() on all builds, taking away the #ifdef and the dynamic cast.

Pushed with preprocessor talk dropped.

akien-mga · 2016-11-01T18:36:04Z

I'm getting this error on Linux:

/home/akien/Projects/godot/android-ndk-r11c//toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ -o platform/android/file_access_android.os -c -fno-rtti -fno-exceptions -DNO_SAFE_CAST -DFREETYPE_ENABLED -fPIC -DDEBUG_MEMORY_ALLOC -DSCI_NAMESPACE -DENABLE_DEPRECATED -isystem /home/akien/Projects/godot/android-ndk-r11c//platforms/android-14/arch-arm/usr/include -fno-integrated-as -gcc-toolchain /home/akien/Projects/godot/android-ndk-r11c//toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64 -Wno-invalid-command-line-argument -Wno-unused-command-line-argument -fpic -ffunction-sections -funwind-tables -fstack-protector-strong -fvisibility=hidden -fno-strict-aliasing -Wa,--noexecstack -DANDROID -DNO_STATVFS -DGLES2_ENABLED -D__ARM_ARCH_7__ -D__ARM_ARCH_7A__ -march=armv7-a -mfloat-abi=softfp -mfpu=neon -D__ARM_NEON__ -target armv7-none-linux-androideabi -fno-integrated-as -gcc-toolchain /home/akien/Projects/godot/android-ndk-r11c//toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/arm-linux-androideabi/bin -O0 -D_DEBUG -UNDEBUG -DDEBUG_ENABLED -DDEBUG_MEMORY_ALLOC -g -fno-limit-debug-info -DANDROID_ENABLED -DUNIX_ENABLED -DNO_FCNTL -DMPC_FIXED_POINT -DGDSCRIPT_ENABLED -DMINIZIP_ENABLED -DXML_ENABLED -Icore -Icore/math -Itools -Idrivers -I. -Iplatform/android -Ithirdparty/freetype -Ithirdparty/freetype/include -Ithirdparty/libpng platform/android/file_access_android.cpp
/..//bin/as: unrecognized option '-mfpu=neon'
clang++: error: assembler command failed with exit code 1 (use -v to see invocation)
scons: *** [platform/android/godot_android.os] Error 1
/..//bin/as: unrecognized option '-mfpu=neon'
clang++: error: assembler command failed with exit code 1 (use -v to see invocation)
scons: *** [platform/android/file_access_android.os] Error 1

RandomShaper · 2016-11-01T18:55:14Z

Could you add env.Append(CPPFLAGS=["-v"]) and start the build agai? That will make Clang tell us the exact command it's issuing to invoke the assembler.

akien-mga · 2016-11-01T19:00:45Z

$ scons p=android                       
scons: Reading SConscript files ...
Godot Android!!!!! (armv7) (with neon)
scons: done reading SConscript files.
scons: Building targets ...
/home/akien/Projects/godot/android-ndk-r11c//toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ -o platform/android/os_android.os -c -fno-rtti -fno-exceptions -DNO_SAFE_CAST -DFREETYPE_ENABLED -fPIC -DDEBUG_MEMORY_ALLOC -DSCI_NAMESPACE -DENABLE_DEPRECATED -isystem /home/akien/Projects/godot/android-ndk-r11c//platforms/android-14/arch-arm/usr/include -fno-integrated-as -gcc-toolchain /home/akien/Projects/godot/android-ndk-r11c//toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64 -Wno-invalid-command-line-argument -Wno-unused-command-line-argument -fpic -ffunction-sections -funwind-tables -fstack-protector-strong -fvisibility=hidden -fno-strict-aliasing -Wa,--noexecstack -DANDROID -DNO_STATVFS -DGLES2_ENABLED -D__ARM_ARCH_7__ -D__ARM_ARCH_7A__ -march=armv7-a -mfloat-abi=softfp -mfpu=neon -D__ARM_NEON__ -target armv7-none-linux-androideabi -fno-integrated-as -gcc-toolchain /home/akien/Projects/godot/android-ndk-r11c//toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/arm-linux-androideabi/bin -v -O0 -D_DEBUG -UNDEBUG -DDEBUG_ENABLED -DDEBUG_MEMORY_ALLOC -g -fno-limit-debug-info -DANDROID_ENABLED -DUNIX_ENABLED -DNO_FCNTL -DMPC_FIXED_POINT -DGDSCRIPT_ENABLED -DMINIZIP_ENABLED -DXML_ENABLED -Icore -Icore/math -Itools -Idrivers -I. -Iplatform/android -Ithirdparty/freetype -Ithirdparty/freetype/include -Ithirdparty/libpng platform/android/os_android.cpp
clang version 3.8.243773 
Target: armv7-none-linux-android
Thread model: posix
 "/home/akien/Projects/godot/android-ndk-r11c/toolchains/llvm/prebuilt/linux-x86_64/bin/clang++" -cc1 -triple armv7-none-linux-android -S -disable-free -disable-llvm-verifier -main-file-name os_android.cpp -mrelocation-model pic -pic-level 1 -mthread-model posix -mdisable-fp-elim -relaxed-aliasing -fmath-errno -masm-verbose -no-integrated-as -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu cortex-a8 -target-feature +soft-float-abi -target-feature -fp-only-sp -target-feature -d16 -target-feature +vfp3 -target-feature -fp16 -target-feature -vfp4 -target-feature -fp-armv8 -target-feature +neon -target-feature -crypto -target-abi aapcs-linux -mfloat-abi soft -target-linker-version 2.24 -v -g -dwarf-column-info -ffunction-sections -coverage-file /home/akien/Projects/godot/godot.git/platform/android/os_android.os -resource-dir /home/akien/Projects/godot/android-ndk-r11c/toolchains/llvm/prebuilt/linux-x86_64/bin/../lib64/clang/3.8.243773 -isystem /home/akien/Projects/godot/android-ndk-r11c//platforms/android-14/arch-arm/usr/include -D NO_SAFE_CAST -D FREETYPE_ENABLED -D DEBUG_MEMORY_ALLOC -D SCI_NAMESPACE -D ENABLE_DEPRECATED -D ANDROID -D NO_STATVFS -D GLES2_ENABLED -D __ARM_ARCH_7__ -D __ARM_ARCH_7A__ -D __ARM_NEON__ -D _DEBUG -U NDEBUG -D DEBUG_ENABLED -D DEBUG_MEMORY_ALLOC -D ANDROID_ENABLED -D UNIX_ENABLED -D NO_FCNTL -D MPC_FIXED_POINT -D GDSCRIPT_ENABLED -D MINIZIP_ENABLED -D XML_ENABLED -I core -I core/math -I tools -I drivers -I . -I platform/android -I thirdparty/freetype -I thirdparty/freetype/include -I thirdparty/libpng -internal-isystem /usr/local/include -internal-isystem /home/akien/Projects/godot/android-ndk-r11c/toolchains/llvm/prebuilt/linux-x86_64/bin/../lib64/clang/3.8.243773/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -O0 -Wno-invalid-command-line-argument -Wno-unused-command-line-argument -fdeprecated-macro -fno-dwarf-directory-asm -fdebug-compilation-dir /home/akien/Projects/godot/godot.git -ferror-limit 19 -fmessage-length 189 -fvisibility hidden -fstandalone-debug -femulated-tls -stack-protector 2 -mstackrealign -fno-rtti -fno-signed-char -fobjc-runtime=gcc -fdiagnostics-show-option -fcolor-diagnostics -o /tmp/os_android-6fadf5.s -x c++ platform/android/os_android.cpp
clang -cc1 version 3.8.243773 based upon LLVM 3.6.0svn default target x86_64-unknown-linux
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
 core
 core/math
 tools
 drivers
 .
 platform/android
 thirdparty/freetype
 thirdparty/freetype/include
 thirdparty/libpng
 /home/akien/Projects/godot/android-ndk-r11c//platforms/android-14/arch-arm/usr/include
 /usr/local/include
 /home/akien/Projects/godot/android-ndk-r11c/toolchains/llvm/prebuilt/linux-x86_64/bin/../lib64/clang/3.8.243773/include
 /usr/include
End of search list.
In file included from platform/android/os_android.cpp:29:
In file included from platform/android/os_android.h:32:
In file included from core/os/input.h:32:
core/object.h:522:8: warning: 'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true [-Wundefined-bool-conversion]
                if (!this)
                    ~^~~~
core/object.h:537:8: warning: 'this' pointer cannot be null in well-defined C++ code; pointer may be assumed to always convert to true [-Wundefined-bool-conversion]
                if (!this)
                    ~^~~~
2 warnings generated.
 "/..//bin/as" -mfpu=neon -mfloat-abi=softfp -march=armv7-a -mfpu=neon -I core -I core/math -I tools -I drivers -I . -I platform/android -I thirdparty/freetype -I thirdparty/freetype/include -I thirdparty/libpng --noexecstack -o platform/android/os_android.os /tmp/os_android-6fadf5.s
/..//bin/as: unrecognized option '-mfpu=neon'
clang++: error: assembler command failed with exit code 1 (use -v to see invocation)
scons: *** [platform/android/os_android.os] Error 1
scons: building terminated because of errors.

RandomShaper · 2016-11-01T19:25:14Z

Thanks! Hmm… I wonder why such a weird path to the assembler ("/..//bin/as").

Sorry to bother you, but could you put this at the end?: print env.Dump()
Make sure no personal information appears there! :D

If that doesn't shed any light I will suggest you upgrading your NDK to the latest release.

akien-mga · 2016-11-01T19:29:36Z

Here's the dump: http://hastebin.com/umufuleruq.go

I'll try a newer NDK yeah.

akien-mga · 2016-11-01T19:45:48Z

Same issue with latest NDK :/

RandomShaper · 2016-11-01T20:26:04Z

I've pinpointed a mistake: passing -gcc-toolchain twice, one time with an incorrect path. A possible cause.

I'm also trying to enable link-time optimization. (If I weren't able to make this work soon I'd leave it for another PR.)

I've pused both changes.

akien-mga · 2016-11-01T21:18:50Z

It seems to work much better for me :)

RandomShaper · 2016-11-01T22:13:30Z

Well, I've finally dropped my attempt at LTO and pushed again.

We found that this flag causes this error on PR godotengine#48812 which does not add any fancy inline assembly: ``` /tmp/tile_set-ce236a.s: Assembler messages: /tmp/tile_set-ce236a.s:34676: Error: selected processor does not support `bfc x0,godotengine#32,godotengine#32' clang++: error: assembler command failed with exit code 1 (use -v to see invocation) ``` That flag is mentioned in various errors related to assembler failures on arm64v8 with Clang from the Android NDK. It was added in Godot in godotengine#6958 when migrating from GCC to Clang, and is indeed referenced in the NDK's Clang migration guide: https://android.googlesource.com/platform/ndk/+/master/docs/ClangMigration.md > Especially for ARM and ARM64, Clang is much stricter about assembler rules > than GCC/GAS. Use `-fno-integrated-as` if Clang reports errors in inline > assembly or assembly files that you don't wish to modernize. We don't get those errors nowadays so it seems the flag is no longer needed.

We found that this flag causes this error on PR #48812 which does not add any fancy inline assembly: ``` /tmp/tile_set-ce236a.s: Assembler messages: /tmp/tile_set-ce236a.s:34676: Error: selected processor does not support `bfc x0,#32,#32' clang++: error: assembler command failed with exit code 1 (use -v to see invocation) ``` That flag is mentioned in various errors related to assembler failures on arm64v8 with Clang from the Android NDK. It was added in Godot in #6958 when migrating from GCC to Clang, and is indeed referenced in the NDK's Clang migration guide: https://android.googlesource.com/platform/ndk/+/master/docs/ClangMigration.md > Especially for ARM and ARM64, Clang is much stricter about assembler rules > than GCC/GAS. Use `-fno-integrated-as` if Clang reports errors in inline > assembly or assembly files that you don't wish to modernize. We don't get those errors nowadays so it seems the flag is no longer needed. (cherry picked from commit 23f7c75)

RandomShaper mentioned this pull request Oct 28, 2016

Compile android template error on Windows #6844

Closed

RandomShaper force-pushed the android-clang-tidy branch from 941022c to e6f1aaf Compare October 28, 2016 22:33

vnen added enhancement platform:android topic:buildsystem labels Oct 28, 2016

RandomShaper force-pushed the android-clang-tidy branch 2 times, most recently from 0214061 to 3aa624d Compare October 29, 2016 02:37

akien-mga added this to the 3.0 milestone Oct 29, 2016

RandomShaper force-pushed the android-clang-tidy branch 11 times, most recently from 11cb26e to bd37dbf Compare October 29, 2016 13:43

RandomShaper force-pushed the android-clang-tidy branch 2 times, most recently from c4b3074 to 0b2eac1 Compare October 30, 2016 20:56

RandomShaper force-pushed the android-clang-tidy branch 2 times, most recently from 2ff4a20 to 12e37e0 Compare November 1, 2016 00:17

leezh reviewed Nov 1, 2016

View reviewed changes

Make regex compilable with RTTI disabled

f935d7a

RandomShaper force-pushed the android-clang-tidy branch from 12e37e0 to 66dd0c6 Compare November 1, 2016 18:17

RandomShaper force-pushed the android-clang-tidy branch 3 times, most recently from 9b02232 to 19ab970 Compare November 1, 2016 20:24

RandomShaper force-pushed the android-clang-tidy branch 2 times, most recently from 4312d0b to e400a0f Compare November 1, 2016 22:12

bojidar-bg mentioned this pull request Nov 2, 2016

RegEx Revamp Ideas #6853

Closed

RandomShaper force-pushed the android-clang-tidy branch from e400a0f to 8baf5c1 Compare November 2, 2016 09:49

Improve Android build (Clang + tidyness)

b18ff94

RandomShaper force-pushed the android-clang-tidy branch from 8baf5c1 to b18ff94 Compare November 2, 2016 09:54

leonkrause mentioned this pull request Nov 2, 2016

Regex module requires runtime type information #7018

Closed

akien-mga merged commit 218c258 into godotengine:master Nov 2, 2016

RandomShaper deleted the android-clang-tidy branch November 13, 2016 22:55

akien-mga mentioned this pull request May 19, 2021

Android: Remove -fno-integrated-as, it can break arm64v8 build #48851

Merged

akien-mga mentioned this pull request Sep 22, 2022

SCons: Unify tools/target build type configuration #66242

Merged

Uh oh!

Uh oh!

Conversation

RandomShaper commented Oct 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akien-mga commented Oct 29, 2016

Uh oh!

RandomShaper commented Oct 29, 2016 via email

Uh oh!

akien-mga commented Oct 30, 2016

Uh oh!

akien-mga commented Oct 30, 2016

Uh oh!

RandomShaper commented Oct 30, 2016

Uh oh!

RandomShaper commented Nov 1, 2016

Uh oh!

leezh Nov 1, 2016

Choose a reason for hiding this comment

Uh oh!

akien-mga Nov 1, 2016

Choose a reason for hiding this comment

Uh oh!

RandomShaper Nov 1, 2016

Choose a reason for hiding this comment

Uh oh!

akien-mga commented Nov 1, 2016

Uh oh!

RandomShaper commented Nov 1, 2016

Uh oh!

akien-mga commented Nov 1, 2016

Uh oh!

RandomShaper commented Nov 1, 2016

Uh oh!

akien-mga commented Nov 1, 2016

Uh oh!

akien-mga commented Nov 1, 2016

Uh oh!

RandomShaper commented Nov 1, 2016

Uh oh!

akien-mga commented Nov 1, 2016

Uh oh!

RandomShaper commented Nov 1, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

RandomShaper commented Oct 28, 2016 •

edited

Loading