Fix ARM version for non-integrated assembler.

The non-integrated assembler does not get the proper -march flag when it is implicit in the triple. Pass it explicitly to avoid problems until we can fix the driver. Writing a test that fails to compile is proving difficult because the warnings emitted by the assembler for the provided test case are just notes rather than warnings so they can't be promoted to errors, and we can't use the __ARM_ARCH_7A__ define because those are resolved by the C preprocessor, which *does* have the right architecture set... Will attempt to add tests for this in a follow up since we need to respin r19b ASAP. Test: Manual inspection for now. Bug: android/ndk#906 Change-Id: I4ab3e9aee0775150731108e42f5f584a5f3c27ea
wimal-build · Feb 12, 2019 · e3f7078 · e3f7078
1 parent b988624
commit e3f7078
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 1 deletion.
diff --git a/build/cmake/android.toolchain.cmake b/build/cmake/android.toolchain.cmake
@@ -509,6 +509,9 @@ if(NOT ANDROID_ALLOW_UNDEFINED_SYMBOLS)
     -Wl,--no-undefined)
 endif()
 if(ANDROID_ABI MATCHES "armeabi")
+  # Clang does not set this up properly when using -fno-integrated-as.
+  # https://github.com/android-ndk/ndk/issues/906
+  list(APPEND ANDROID_COMPILER_FLAGS "-march=armv7-a")
   if(ANDROID_ARM_MODE STREQUAL thumb)
     list(APPEND ANDROID_COMPILER_FLAGS -mthumb)
   elseif(ANDROID_ARM_MODE STREQUAL arm)

diff --git a/build/core/toolchains/arm-linux-androideabi-clang/setup.mk b/build/core/toolchains/arm-linux-androideabi-clang/setup.mk
@@ -21,6 +21,10 @@ TARGET_UBSAN_BASENAME := libclang_rt.ubsan_standalone-arm-android.so
 
 TARGET_CFLAGS := -fpic
 
+# Clang does not set this up properly when using -fno-integrated-as.
+# https://github.com/android-ndk/ndk/issues/906
+TARGET_CFLAGS += -march=armv7-a
+
 TARGET_LDFLAGS :=
 
 TARGET_CFLAGS.neon := -mfpu=neon

diff --git a/docs/BuildSystemMaintainers.md b/docs/BuildSystemMaintainers.md
@@ -340,7 +340,7 @@ Lollipop (API 21) if the device has been rooted. Direct users to the
 [debuggable]: https://developer.android.com/guide/topics/manifest/application-element#debug
 [wrap.sh]: https://developer.android.com/ndk/guides/wrap-script
 
-## Required Android Specific Arguments
+## Additional Required Arguments
 
 Note: It is a bug that any of these need to be specified by the build system.
 All flags discussed in this section should be automatically selected by Clang,
@@ -358,8 +358,15 @@ Clang uses `-faddrsig` by default, but this produces output that is incompatible
 with GNU binutils. To workaround this, `-fno-addrsig` must be passed to Clang
 when using GNU binutils. See [Issue 884].
 
+Clang does not properly set the ARMv7 architecture for the non-integrated
+assembler. If using `-fno-integrated-as`, you must explicitly pass
+`-march=armv7-a` when compiling for 32-bit ARM. Note that by default Clang will
+use the integrated assembler, and this flag is not needed in that case. See
+[Issue 906].
+
 [Issue 635]: https://github.com/android-ndk/ndk/issues/635
 [Issue 884]: https://github.com/android-ndk/ndk/issues/884
+[Issue 906]: https://github.com/android-ndk/ndk/issues/906
 [Position-independent executables]: https://en.wikipedia.org/wiki/Position-independent_code#Position-independent_executables
 
 ## Useful Arguments

diff --git a/docs/changelogs/Changelog-r19.md b/docs/changelogs/Changelog-r19.md
@@ -69,6 +69,10 @@ r19b
    toolchains to compile C code.
  * [Issue 890]: Fixed `CMAKE_FIND_ROOT_PATH`. CMake projects will no longer
    search the host's sysroot for headers and libraries.
+ * [Issue 906]: Explicitly set `-march=armv7-a` for 32-bit ARM to workaround
+   Clang not setting that flag automatically when using `-fno-integrated-as`.
+   This fix only affects ndk-build and CMake. Standalone toolchains and custom
+   build systems will need to apply this fix themselves.
  * [Issue 907]: Fixed `find_path` for NDK headers in CMake.
 
 [Issue 849]: https://github.com/android-ndk/ndk/issues/849
@@ -135,6 +139,13 @@ Known Issues
  * [Issue 884]: Third-party build systems must pass `-fno-addrsig` to Clang for
    compatibility with binutils. ndk-build, CMake, and standalone toolchains
    handle this automatically.
+ * [Issue 906]: Clang does not pass `-march=armv7-a` to the assembler when using
+   `-fno-integrated-as`. This results in the assembler generating ARMv5
+   instructions. Note that by default Clang uses the integrated assembler which
+   does not have this problem. To workaround this issue, explicitly use
+   `-march=armv7-a` when building for 32-bit ARM with the non-integrated
+   assembler, or use the integrated assembler. ndk-build and CMake already
+   contain these workarounds.
  * This version of the NDK is incompatible with the Android Gradle plugin
    version 3.0 or older. If you see an error like
    `No toolchains found in the NDK toolchains folder for ABI with prefix: mips64el-linux-android`,
@@ -146,4 +157,5 @@ Known Issues
 [Issue 855]: https://github.com/android-ndk/ndk/issues/855
 [Issue 884]: https://github.com/android-ndk/ndk/issues/884
 [Issue 888]: https://github.com/android-ndk/ndk/issues/888
+[Issue 906]: https://github.com/android-ndk/ndk/issues/906
 [use plugin version 3.1 or newer]: https://developer.android.com/studio/releases/gradle-plugin#updating-plugin