[BUG] Invalid ARMv7 assembly generated by clang r19b with optimizations (-Os) enabled

[falken42]

Description

After changing some code in our cross-platform engine which makes use of atomics, our Android builds began failing. Here's the full compile command line and output from clang:

/Users/falken/dev/android-ndk-r19b/toolchains/llvm/prebuilt/darwin-x86_64/bin/clang++ -x c++ -MMD -MP -target armv7-none-linux-androideabi -march=armv7-a -mfloat-abi=softfp -mfpu=vfpv3-d16 -mthumb -fno-integrated-as -DCFRELEASE -DCFMINSIZE -Os -fno-stack-protector -funsigned-char -fno-common -std=c++1y -stdlib=libc++ -fno-exceptions -fno-rtti -gcc-toolchain /Users/falken/dev/android-ndk-r19b/toolchains/arm-linux-androideabi-4.9/prebuilt/darwin-x86_64 -fpic -ffunction-sections -fdata-sections -funwind-tables -fvisibility=hidden -Wno-invalid-command-line-argument -Wno-unused-command-line-argument -no-canonical-prefixes -Wa,--noexecstack -Wformat -Werror=format-security --sysroot /Users/falken/dev/android-ndk-r19b/sysroot -isystem /Users/falken/dev/android-ndk-r19b/sysroot/usr/include/arm-linux-androideabi -I/Users/falken/dev/android-ndk-r19b/sources/cxx-stl/llvm-libc++/include -I/Users/falken/dev/android-ndk-r19b/sources/cxx-stl/llvm-libc++abi/include -I/Users/falken/dev/android-ndk-r19b/sources/android/support/include -Iinclude -Isrc -Isrc/3rdparty -c src/CFAudio/IMTRenderer.cpp -o build/Android-armv7-MinSizeRel/src/CFAudio/IMTRenderer.cpp.o
/var/folders/r5/595y00cs2c535nqsj5bd47580000gn/T/IMTRenderer-f8fd0c.s: Assembler messages:
/var/folders/r5/595y00cs2c535nqsj5bd47580000gn/T/IMTRenderer-f8fd0c.s:1752: Error: bad immediate value for offset (4016)
clang++: error: assembler command failed with exit code 1 (use -v to see invocation)
make: *** [build/Android-armv7-MinSizeRel/src/CFAudio/IMTRenderer.cpp.o] Error 1

The assembly at the line causing the error is:

    ldrex   r0, [lr, #4016]

We were originally using r16b, so this seemed like a good time to update to r19b, unfortunately it still resulted in this exact same error. I haven't changed or updated any of the command line parameters yet, so some things might be outdated or unnecessary.

Trying without -fno-integrated-as results in mostly the same error:

test.s:54:12: error: invalid operand for instruction
        ldrex   r0, [lr, #4016]
                    ^

A build pass without optimizations (-O0) compiles fine without problems.

I'm currently going through and trimming down a 2.2MB preprocessed source file to provide a repro case, and will comment again once I have something more.

Environment Details

Not all of these will be relevant to every bug, but please provide as much
information as you can.

• NDK Version: r19b
• Build system: custom / Makefile
• Host OS: Mac OS X / darwin
• Compiler: Android (5058415 based on r339409) clang version 8.0.2
• ABI: ARMv7
• STL: libc++

[falken42]

I've managed to get a somewhat reasonably sized repro case, although it's still a bit large at 135 lines of code. It might still be able to be trimmed down further, so I'll try again tomorrow.

Here's a link to the repo: https://github.com/Falken42/ndk-r19b-armv7-asm-repro

Running ./test.sh results in:

raichu:ndk-r19b-armv7-asm-repro falken$ ./test.sh 
rm -f ./libs/armeabi-v7a/*
rm -f ./libs/armeabi-v7a/gdbserver
rm -f ./libs/armeabi-v7a/gdb.setup
[armeabi-v7a] Prebuilt       : libc++_shared.so <= <NDK>/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a/
cp -f /Users/falken/dev/android-ndk-r19b/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a/libc++_shared.so obj/local/armeabi-v7a/libc++_shared.so
[armeabi-v7a] Compile++ thumb: test <= test.cpp
rm -f ./obj/local/armeabi-v7a/objs/test/test.o
/Users/falken/dev/android-ndk-r19b/toolchains/llvm/prebuilt/darwin-x86_64/bin/clang++ -MMD -MP -MF ./obj/local/armeabi-v7a/objs/test/test.o.d -target armv7-none-linux-androideabi24 -fdata-sections -ffunction-sections -fstack-protector-strong -funwind-tables -no-canonical-prefixes  --sysroot /Users/falken/dev/android-ndk-r19b/toolchains/llvm/prebuilt/darwin-x86_64/sysroot -g -Wno-invalid-command-line-argument -Wno-unused-command-line-argument  -fno-addrsig -fno-exceptions -fno-rtti -fpic -mfpu=vfpv3-d16  -march=armv7-a -mthumb -Oz -DNDEBUG  -mfpu=neon -I/Users/falken/dev/android-ndk-r19b/sources/cxx-stl/llvm-libc++/include -I/Users/falken/dev/android-ndk-r19b/sources/cxx-stl/llvm-libc++/../llvm-libc++abi/include -Ijni -std=c++11  -march=armv7-a -mfloat-abi=softfp -mfpu=vfpv3-d16 -mthumb -fno-integrated-as -Os -fno-stack-protector -funsigned-char -fno-common -std=c++1y -stdlib=libc++ -fno-exceptions -fno-rtti -fpic -ffunction-sections -fdata-sections -funwind-tables -fvisibility=hidden -Wno-invalid-command-line-argument -Wno-unused-command-line-argument -no-canonical-prefixes -Wa,--noexecstack -Wformat -Werror=format-security  -DANDROID  -nostdinc++ -Wa,--noexecstack -Wformat -Werror=format-security   -c  jni/test.cpp -o ./obj/local/armeabi-v7a/objs/test/test.o
[armeabi-v7a] Install        : libc++_shared.so => libs/armeabi-v7a/libc++_shared.so
install -p ./obj/local/armeabi-v7a/libc++_shared.so ./libs/armeabi-v7a/libc++_shared.so
/Users/falken/dev/android-ndk-r19b/toolchains/llvm/prebuilt/darwin-x86_64/bin/arm-linux-androideabi-strip --strip-unneeded  ./libs/armeabi-v7a/libc++_shared.so
/var/folders/r5/595y00cs2c535nqsj5bd47580000gn/T/test-f19349.s: Assembler messages:
/var/folders/r5/595y00cs2c535nqsj5bd47580000gn/T/test-f19349.s:93: Error: bad immediate value for offset (4048)
clang++: error: assembler command failed with exit code 1 (use -v to see invocation)
make: *** [obj/local/armeabi-v7a/objs/test/test.o] Error 1

Trimming the code down resulted in a different offset (4048 here, compared to 4016 earlier), which occurred while removing member variables in the Array template class.

While integrating the repro case with ndk-build, I remembered the earlier discussions from #579 and #573 and on a whim decided to try building with -Oz instead. It turns out -Oz works fine in this case while -Os fails.

[DanAlbert]

It also works if you remove -fno-integrated-as. @jmgao pointed out that 4048 might not even be a valid offset for that instruction, but if that's the case then it's weird that it works with Clang's assembler. @stephenhines, any ideas?

[jmgao]

Here's a more minified version: https://gist.github.com/jmgao/c7d5c8d2c582eb6acb4a2cbf23861324

[jmgao]

There's some weirdness going on (obviously...):

clang++ -target armv7-none-linux-androideabi24 -mthumb -Os -fintegrated-as jni/test.cpp -c -o test.integrated.o "works", and emits ldrex r0, [sp, #16], which doesn't seem right.

clang++ -target armv7-none-linux-androideabi24 -mthumb -Os -fintegrated-as jni/test.cpp -s -o test.integrated.s; clang++ -target armv7-none-linux-androideabi24 -mthumb -Os -fintegrated-as test.integrated.s -c -o test.integrated.o emits the bogus ldrex r0, [sp, #1040] which gets rejected by the integrated assembler.

It seems like we're just horribly miscompiling atomics in structs??? This code seems to turn into this assembly, which seems to be using an obviously wrong offset for the atomic.

This seems to only happen with the NDK clang? r346389c and r349610 from the platform seems less broken.

[jmgao]

@rprichard found https://reviews.llvm.org/D51678. Atomic loads of variables on your local stack frame when using thumb have apparently been broken in clang for a pretty long time, and was only somewhat recently fixed. As a workaround, try specifying -marm?

[DanAlbert]

Good find. The master branch (r20) already has a Clang newer than that patch and the problem doesn't repro with that Clang.

[falken42]

Big thanks for minifying the repro case down even further and finding the source of the bug!

It appears that as long as the atomic is the first member in a struct, the correct assembly is generated without any need for an immediate offset:

    ldrex   r1, [r0]

I've added a quick patch to our codebase that moves the atomic to the base of the AutoMutex class and confirmed it works as expected. This lets us keep -mthumb and will do fine as a workaround for now until r20 is generally available.

Thanks once again!