[BUG] NDK r23 gives confusing error message for -fno-integrated-as

[bog-dan-ro]

Description

Steps to reproduce:

• create a simple .c file $ cat test.c :

int main() {return 0;}

• Try to compile it:

$ /path/to/ndk/23.0.7599858/toolchains/llvm/prebuilt/linux-x86_64/bin/clang test.c --target=armv7a-linux-androideabi28 -fno-integrated-as -pipe -D_FILE_OFFSET_BITS=64 -c
/usr/bin/as: unrecognized option '-EL'
clang-12: error: assembler command failed with exit code 1 (use -v to see invocation)

Without -fno-integrated-as it compiles just fine.

I need to pass -fno-integrated-as to workaround pixman issues: https://gitlab.freedesktop.org/pixman/pixman/-/issues/45

[enh-google]

yeah, -fno-integrated-as isn't supported in r23. (i don't whether we should configure things so that if you do use it [and don't manually specify yasm or whatever] we run a script or something that says "-fno-integrated-as is not supported" rather than just quietly running /usr/bin/as which is just misleading.)

we have some docs here about how to fix assembler: https://android.googlesource.com/platform/ndk/+/master/docs/ClangMigration.md#assembler-issues

the .func error in particular is just dead code for gas too, so that's easy. i didn't see the .if error when we fixed all of Android, so i don't have any specific advice to give there (but in general, and especially since the source file is already .S rather than .s, "just use the C preprocessor instead" was useful to me!).

does it work if you move all the .if stuff below the push? the only somewhat-relevant references to this error i found on the internet implied that there are issues before the first instruction is emitted, though they seemed to be .ifs that involved calculations relative to . that doesn't apply here. (that's also supposed to have been fixed in 2019, so we should have that fix already.)

[enh-google]

wait... it's just occurred to me that you're talking about r23, which still has gas, not r24, which is the first release that doesn't.

so for r23, you've probably just got a misconfigured build that's calling the wrong as. (but for r24 you'll need code that can actually build with LLVM.)

[rprichard]

r23 still has gas, but not with the filename that Clang expects.

r22 has:

1. toolchains/${target}/prebuilt/${host}/${triple}/bin/as
2. toolchains/${target}/prebuilt/${host}/bin/${triple}-as
3. toolchains/llvm/prebuilt/${host}/${triple}/bin/as
4. toolchains/llvm/prebuilt/${host}/bin/${triple}-as

r23 only has # 4, but I think we also need # 3 for -fno-integrated-as.

[bog-dan-ro]

Instead to force everyone to change their asm file to make them llvm compatible, isn't better to make llvm as compatible with gas?
This way you can nuke gas without no worries.

[bog-dan-ro]

Just for the record, compiling pixman with r21 & r22 with -fno-integrated-as spawns tones of Rd and Rm should be different in mul but at least it compiles ... :)

[stephenhines]

Instead to force everyone to change their asm file to make them llvm compatible, isn't better to make llvm as compatible with gas?
This way you can nuke gas without no worries.

There are inconsistencies and syntax relaxations for gas that ARM (among other vendors) have said that they won't support in any new compilers/assemblers. If the syntax being used is UAL (unified assembly language), then we can work with ARM to add the missing support. In that particular case, LLVM is correct in telling you that the instruction you're using isn't valid, and gas is merely transforming it into something equivalent. According to this link, "Rd cannot be the same as Rm."

[bog-dan-ro]

At first look it doesn't seem to be an ASM language problem, but llvm as is missing some features:

/path/to/android-sdk/ndk/21.1.6352462/toolchains/llvm/prebuilt/linux-x86_64/bin/clang -
Ipixman/libpixman-arm-simd.a.p -Ipixman -I../pixman -Xclang -fcolor-diagnostics -pipe -D_FILE_OFFSE
T_BITS=64 -Wall -Winvalid-pch -O2 -g -Wdeclaration-after-statement -fno-strict-aliasing -fvisibilit
y=hidden -Wundef -ftrapping-math -Wno-unused-local-typedefs -DHAVE_CONFIG_H --target=armv7a-linux-a
ndroideabi28 -fPIC -MD -MQ pixman/libpixman-arm-simd.a.p/pixman-arm-simd-asm.S.o -MF pixman/libpixm
an-arm-simd.a.p/pixman-arm-simd-asm.S.o.d -o pixman/libpixman-arm-simd.a.p/pixman-arm-simd-asm.S.o 
-c ../pixman/pixman-arm-simd-asm.S
<instantiation>:1:1: error: unknown directive
.func fname
^
../pixman/pixman-arm-simd-asm.h:599:5: note: while in macro instantiation
    pixman_asm_function fname
    ^
/tmp/pixman-arm-simd-asm-0ff417.s:913:1: note: while in macro instantiation
generate_composite_function pixman_composite_src_8888_8888_asm_armv6, 32, 0, 32, FLAG_DST_WRITEONLY
 | FLAG_COND_EXEC | FLAG_SPILL_LINE_VARS_WIDE | FLAG_PROCESS_PRESERVES_SCRATCH, 4, blit_init, nop_m
acro, nop_macro, blit_process_head, nop_macro, blit_inner_loop
^
../pixman/pixman-arm-simd-asm.h:614:6: error: expected absolute expression
 .if prefetch_distance == 0
     ^
/tmp/pixman-arm-simd-asm-0ff417.s:913:1: note: while in macro instantiation
generate_composite_function pixman_composite_src_8888_8888_asm_armv6, 32, 0, 32, FLAG_DST_WRITEONLY
 | FLAG_COND_EXEC | FLAG_SPILL_LINE_VARS_WIDE | FLAG_PROCESS_PRESERVES_SCRATCH, 4, blit_init, nop_m
acro, nop_macro, blit_process_head, nop_macro, blit_inner_loop
^
../pixman/pixman-arm-simd-asm.h:620:6: error: expected absolute expression
 .if src_bpp == 32
     ^

Attached the entire output in case you need it : out.txt

[bog-dan-ro]

According to this link, "Rd cannot be the same as Rm."

Just for the record, I'm seeing those messages when I'm compiling .c files with -fno-integrated-as not .s files.

[rprichard]

I think the problems with .if results from Clang's incomplete .altmacro support. pixman-arm-simd-asm.S uses .altmacro. Here's the non-altmacro behavior:

$ cat >test.S <<EOF
    .macro foo arg_
    .set arg, arg_
    .if arg != 42
    .error "arg must be 42"
    .endif
    .endm
    foo 42
EOF

$ gcc -c test.S
test.S: Assembler messages:
test.S:7: Error: arg must be 42

$ clang -c test.S
<instantiation>:2:9: error: expected absolute expression
    .if arg != 42
        ^
...

It can be fixed using \arg_:

$ cat >test.S <<EOF
    .macro foo arg_
    .set arg, \arg_
    .if arg != 42
    .error "arg must be 42"
    .endif
    .endm
    foo 42
EOF

$ gcc -c test.S
<success>

$ clang -c test.S
<success>

But .altmacro changes a bunch of stuff, and it can also be used instead of escaping the macro argument:

$ cat >test.S <<EOF
    .altmacro
    .macro foo arg_
    .set arg, arg_
    .if arg != 42
    .error "arg must be 42"
    .endif
    .endm
    foo 42
EOF

$ gcc -c test.S
<success>

$ clang -c test.S
<instantiation>:2:9: error: expected absolute expression
    .if arg != 42
        ^

There are a few open .altmacro bugs already, but I don't think this one is open yet:
https://bugs.llvm.org/buglist.cgi?quicksearch=altmacro

[rprichard]

FWIW: ART uses .altmacro but only with GNU as:
https://android.googlesource.com/platform/art/+/refs/tags/android-11.0.0_r40/runtime/arch/x86_64/asm_support_x86_64.S#30

ART uses .altmacro to work around this GCC-vs-Clang preprocessor difference. My guess, though, is that we could rewrite that code to work without .altmacro (and use the same code for GCC and Clang).

[enh-google]

FWIW: ART uses .altmacro but only with GNU as:
https://android.googlesource.com/platform/art/+/refs/tags/android-11.0.0_r40/runtime/arch/x86_64/asm_support_x86_64.S#30

ART uses .altmacro to work around this GCC-vs-Clang preprocessor difference. My guess, though, is that we could rewrite that code to work without .altmacro (and use the same code for GCC and Clang).

well, given that ART only builds with clang, we could just delete that code :-)

[DanAlbert]

Just putting back toolchains/llvm/prebuilt/${host}/${triple}/bin/as doesn't seem to be enough. I wonder if we need to add back all the no-op crt objects to get Clang to recognize the "GCC" directory...

[DanAlbert]

Yeah, I think that is it. The actual path that gets used for as in r22:

"/work/src/android-ndk-r22b/toolchains/llvm/prebuilt/linux-x86_64/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../arm-linux-androideabi/bin/as"

🤮

[DanAlbert]

https://android-review.googlesource.com/c/platform/ndk/+/1817218 fixes the problem. Sorry about that. In our effort to make sure we were -fno-integrated-as clean before asking users to do so we of course removed all our own uses of it, but neglected to add a test to cover that case. I've added an explicit test of -fno-integrated-as to go with the fix :)

I forked the missing altmacro support discussion into a new bug.

[DanAlbert]

The changelog update so we don't forget to cherry-pick it: https://android-review.googlesource.com/c/platform/ndk/+/1817316

[Grimler91]

The as symlinks that were added in https://android-review.googlesource.com/c/platform/ndk/+/1817218 seem to be broken in r23b:

$ ls -l toolchains/llvm/prebuilt/linux-x86_64/*android*/bin/
toolchains/llvm/prebuilt/linux-x86_64/aarch64-linux-android/bin/:
total 4
lrwxrwxrwx 1 root root 131 Oct 23 15:10 as -> /buildbot/src/android/ndk-release-r23/out/linux/android-ndk-r23b/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android-as

toolchains/llvm/prebuilt/linux-x86_64/arm-linux-androideabi/bin/:
total 4
lrwxrwxrwx 1 root root 131 Oct 23 15:10 as -> /buildbot/src/android/ndk-release-r23/out/linux/android-ndk-r23b/toolchains/llvm/prebuilt/linux-x86_64/bin/arm-linux-androideabi-as

toolchains/llvm/prebuilt/linux-x86_64/i686-linux-android/bin/:
total 4
lrwxrwxrwx 1 root root 128 Oct 23 15:10 as -> /buildbot/src/android/ndk-release-r23/out/linux/android-ndk-r23b/toolchains/llvm/prebuilt/linux-x86_64/bin/i686-linux-android-as

toolchains/llvm/prebuilt/linux-x86_64/x86_64-linux-android/bin/:
total 4
lrwxrwxrwx 1 root root 130 Oct 23 15:10 as -> /buildbot/src/android/ndk-release-r23/out/linux/android-ndk-r23b/toolchains/llvm/prebuilt/linux-x86_64/bin/x86_64-linux-android-as

Fixing so they instead point towards ../../bin/{triple}-as makes -fno-integrated-as work as intended.

[rprichard]

I uploaded a fix making the as symlink target relative: https://android-review.googlesource.com/c/platform/ndk/+/1875878

[DanAlbert]

And of course the test passes because it's being run on the same machine (－‸ლ)

[rprichard]

And of course the test passes because it's being run on the same machine (－‸ლ)

I think detecting this issue would have required building the NDK and the tests on different machines.

Alternately, maybe we could have a test that walks the NDK install directory and verifies that all symlinks are relative.

[enh-google]

gah, i make this mistake all the time when a configure script or whatever creates an absolute symlink. why do they even do that?! looking at you, pcre2... :-)

[orifmilod]

For those who are facing the same issue as @Grimler91 - (seeing /buildbot/...). Please run the command below, it should remove the old broken sym-link files and create new ones.

rm ./toolchains/llvm/prebuilt/linux-x86_64/aarch64-linux-android/bin/as && \
ln -sv ./toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android-as ./toolchains/llvm/prebuilt/linux-x86_64/aarch64-linux-android/bin/as && \
rm ./toolchains/llvm/prebuilt/linux-x86_64/arm-linux-androideabi/bin/as && \
ln -sv ./toolchains/llvm/prebuilt/linux-x86_64/bin/arm-linux-androideabi-as ./toolchains/llvm/prebuilt/linux-x86_64/arm-linux-androideabi/bin/as && \
rm ./toolchains/llvm/prebuilt/linux-x86_64/i686-linux-android/bin/as && \
ln -sv ./toolchains/llvm/prebuilt/linux-x86_64/bin/i686-linux-android-as ./toolchains/llvm/prebuilt/linux-x86_64/i686-linux-android/bin/as && \
rm ./toolchains/llvm/prebuilt/linux-x86_64/x86_64-linux-android/bin/as && \
ln -sv ./toolchains/llvm/prebuilt/linux-x86_64/bin/x86_64-linux-android-as ./toolchains/llvm/prebuilt/linux-x86_64/x86_64-linux-android/bin/as

[DanAlbert]

Should be fixed in r23 build 8486889.