Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arm and libatomic #3092

Closed
Grimler91 opened this issue Nov 27, 2018 · 8 comments
Closed

Arm and libatomic #3092

Grimler91 opened this issue Nov 27, 2018 · 8 comments
Labels
bug report Something is not working properly

Comments

@Grimler91
Copy link
Member

Consider this code:

#include <atomic>
std::atomic<float> x(0.0f);
int main() { return (float)x; }

Which is taken from cmake's $PREFIX/lib/cmake/llvm/CheckCompilerVersion.cmake and used for checking for libstdc++4.8 or newer. When compiling this on arm with clang++ tmp.cpp I get

/data/data/com.termux/files/usr/bin/arm-linux-androideabi-ld: /data/data/com.termux/files/usr/tmp/tmp-77121f.o: in function `main':
tmp.cpp:(.text+0x40): undefined reference to `__atomic_load_4'
clang-7: error: linker command failed with exit code 1 (use -v to see invocation)

We get the same error for some i686 (and arm?) packages when cross-compiling, but there it can be solved with -latomic. Compiling on device with clang++ -latomic tmp.cpp has no effect though, it gives the same error message.

I suppose this is due to the difference between android's linker and the standard gnu/Linux linker, and we can solve this by linking one of llvm/clang's libraries against libatomic.a. But which one?

@Grimler91 Grimler91 added the bug report Something is not working properly label Nov 27, 2018
@tomty89
Copy link
Contributor

tomty89 commented Nov 27, 2018

This might be of your interest:

$ ln -sf /system/lib/libc++.so $PREFIX/arm-linux-androideabi/lib/libc++_shared.so
$ cp $PREFIX/arm-linux-androideabi/lib/crt* $PREFIX/lib/
$ clang++ -m32 -L $PREFIX/arm-linux-androideabi/lib/ -L /system/lib -Wl,-rpath-link $PREFIX/arm-linux-androideabi/lib/ -Wl,-rpath-link /system/lib -latomic -v test.cpp
clang version 7.0.0 (tags/RELEASE_700/final)
Target: arm--linux-android
Thread model: posix
InstalledDir: /data/data/com.termux/files/usr/bin
 "/data/data/com.termux/files/usr64/bin/clang-7" -cc1 -triple armv4t--linux-android -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name test.cpp -mrelocation-model pic -pic-level 2 -pic-is-pie -mthread-model posix -mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases -fuse-init-array -target-cpu arm7tdmi -target-feature +soft-float -target-feature +soft-float-abi -target-feature -fp-only-sp -target-feature -d16 -target-feature -vfp2 -target-feature -vfp3 -target-feature -fp16 -target-feature -vfp4 -target-feature -fp-armv8 -target-feature -neon -target-feature -crypto -target-feature +strict-align -target-abi aapcs-linux -msoft-float -mfloat-abi soft -fallow-half-arguments-and-returns -dwarf-column-info -debugger-tuning=gdb -v -resource-dir /data/data/com.termux/files/usr64/lib/clang/7.0.0 -internal-isystem /data/data/com.termux/files/usr64/bin/../include/c++/v1 -internal-isystem /usr/local/include -internal-isystem /data/data/com.termux/files/usr64/lib/clang/7.0.0/include -internal-externc-isystem /data/data/com.termux/files/usr/include -fdeprecated-macro -fdebug-compilation-dir /data/data/com.termux/files/home -ferror-limit 19 -fmessage-length 154 -fno-signed-char -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -o /data/data/com.termux/files/usr/tmp/test-072861.o -x c++ test.cpp -faddrsig
clang -cc1 version 7.0.0 based upon LLVM 7.0.0 default target aarch64-linux-android
ignoring nonexistent directory "/usr/local/include"
#include "..." search starts here:
#include <...> search starts here:
 /data/data/com.termux/files/usr64/bin/../include/c++/v1
 /data/data/com.termux/files/usr64/lib/clang/7.0.0/include
 /data/data/com.termux/files/usr/include
End of search list.
 "/data/data/com.termux/files/usr/bin/aarch64-linux-android-ld" -pie -X --enable-new-dtags --eh-frame-hdr -m armelf_linux_eabi -dynamic-linker /system/bin/linker -o a.out /data/data/com.termux/files/usr64/bin/../lib/crtbegin_dynamic.o -L/data/data/com.termux/files/usr/arm-linux-androideabi/lib/ -L/system/lib -L/data/data/com.termux/files/usr64/bin/../lib -L/lib/../lib -L/data/data/com.termux/files/usr64/bin/../lib -L/lib -rpath-link /data/data/com.termux/files/usr/arm-linux-androideabi/lib/ -rpath-link /system/lib -latomic /data/data/com.termux/files/usr/tmp/test-072861.o -lc++_shared -lm -lgcc -lgcc -ldl -lc -lgcc -lgcc -ldl /data/data/com.termux/files/usr64/bin/../lib/crtend_android.o
/data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: /data/data/com.termux/files/usr/tmp/test-072861.o: in function `main':
test.cpp:(.text+0x40): undefined reference to `__atomic_load_4'
clang-7: error: linker command failed with exit code 1 (use -v to see invocation)
$ clang++ -m32 -c -v test.cpp
clang version 7.0.0 (tags/RELEASE_700/final)
Target: arm--linux-android
Thread model: posix
InstalledDir: /data/data/com.termux/files/usr/bin
 "/data/data/com.termux/files/usr64/bin/clang-7" -cc1 -triple armv4t--linux-android -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name test.cpp -mrelocation-model pic -pic-level 2 -pic-is-pie -mthread-model posix -mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases -fuse-init-array -target-cpu arm7tdmi -target-feature +soft-float -target-feature +soft-float-abi -target-feature -fp-only-sp -target-feature -d16 -target-feature -vfp2 -target-feature -vfp3 -target-feature -fp16 -target-feature -vfp4 -target-feature -fp-armv8 -target-feature -neon -target-feature -crypto -target-feature +strict-align -target-abi aapcs-linux -msoft-float -mfloat-abi soft -fallow-half-arguments-and-returns -dwarf-column-info -debugger-tuning=gdb -v -coverage-notes-file /data/data/com.termux/files/home/test.gcno -resource-dir /data/data/com.termux/files/usr64/lib/clang/7.0.0 -internal-isystem /data/data/com.termux/files/usr64/bin/../include/c++/v1 -internal-isystem /usr/local/include -internal-isystem /data/data/com.termux/files/usr64/lib/clang/7.0.0/include -internal-externc-isystem /data/data/com.termux/files/usr/include -fdeprecated-macro -fdebug-compilation-dir /data/data/com.termux/files/home -ferror-limit 19 -fmessage-length 154 -fno-signed-char -fobjc-runtime=gcc -fcxx-exceptions -fexceptions -fdiagnostics-show-option -fcolor-diagnostics -o test.o -x c++ test.cpp -faddrsig
clang -cc1 version 7.0.0 based upon LLVM 7.0.0 default target aarch64-linux-android
ignoring nonexistent directory "/usr/local/include"
#include "..." search starts here:
#include <...> search starts here:
 /data/data/com.termux/files/usr64/bin/../include/c++/v1
 /data/data/com.termux/files/usr64/lib/clang/7.0.0/include
 /data/data/com.termux/files/usr/include
End of search list.
$ "/data/data/com.termux/files/usr/bin/aarch64-linux-android-ld" -pie -X --enable-new-dtags --eh-frame-hdr -m armelf_linux_eabi -dynamic-linker /system/bin/linker -o a.out /data/data/com.termux/files/usr64/bin/../lib/crtbegin_dynamic.o -L/data/data/com.termux/files/usr/arm-linux-androideabi/lib/ -L/system/lib -L/data/data/com.termux/files/usr64/bin/../lib -L/lib/../lib -L/data/data/com.termux/files/usr64/bin/../lib -L/lib -rpath-link /data/data/com.termux/files/usr/arm-linux-androideabi/lib/ -rpath-link /system/lib -latomic test.o -lc++_shared -lm -lgcc -lgcc -ldl -lc -lgcc -lgcc -ldl /data/data/com.termux/files/usr64/bin/../lib/crtend_android.o
/data/data/com.termux/files/usr/bin/aarch64-linux-android-ld: test.o: in function `main':
test.cpp:(.text+0x40): undefined reference to `__atomic_load_4'
$ "/data/data/com.termux/files/usr/bin/aarch64-linux-android-ld" -pie -X --enable-new-dtags --eh-frame-hdr -m armelf_linux_eabi -dynamic-linker /system/bin/linker -o a.out /data/data/com.termux/files/usr64/bin/../lib/crtbegin_dynamic.o -L/data/data/com.termux/files/usr/arm-linux-androideabi/lib/ -L/system/lib -L/data/data/com.termux/files/usr64/bin/../lib -L/lib/../lib -L/data/data/com.termux/files/usr64/bin/../lib -L/lib -rpath-link /data/data/com.termux/files/usr/arm-linux-androideabi/lib/ -rpath-link /system/lib test.o -latomic -lc++_shared -lm -lgcc -lgcc -ldl -lc -lgcc -lgcc -ldl /data/data/com.termux/files/usr64/bin/../lib/crtend_android.o
$

The only difference of the linking commands issued (manually) is the position of -latomic.

@Grimler91
Copy link
Member Author

Interesting, so we should perhaps patch llvm (and try not to break anything else)?

I wonder if any of the LDFLAGS linking we do in various packages can be fixed in the same way..

@tomty89
Copy link
Contributor

tomty89 commented Nov 27, 2018

Not really sure tbh. Just happen to have bumped into similar problem and noticed the weird positioning of the link flags.

I have been curious why clang/ld would only link to libgcc but not libatomic by default as well. Though apparently when on aarch64, libatomic is not needed (for this) at all (like, it builds even if I have the file deleted). (Does it hurt / make a difference to link to a static library that is not used though.)

@Grimler91
Copy link
Member Author

Here's a comment in the NDK repo which says that the linking order is important, as you have shown: android/ndk#589 (comment).

I also think I read some discussion about -latomic being needed on 32bit arches but not 64bit but can't fins it now. I did find this comment in the ndk r16 changelog that -latomic shouldn't be needed anymore: https://github.com/android-ndk/ndk/wiki/Changelog-r16#ndk

@its-pointless
Copy link
Contributor

A quick hack to this is add -u __atomic_load_4 -latomic to ldflags but that comes with downsides since it might increase size of all libs and executables unnecessarily,

@Grimler91
Copy link
Member Author

@its-pointless Yes, that works! Both clang++ -u __atomic_load_4 -latomic tmp.cpp and clang++ tmp.cpp -latomic works.

The mentioned cmake test in $PREFIX/lib/cmake/llvm/CheckCompilerVersion.cmake doesn't use LDFLAGS though, so we have to solve that in some other way

@stale
Copy link

stale bot commented Nov 18, 2021

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix Issue won't be fixed label Nov 18, 2021
@Grimler91
Copy link
Member Author

With clang 13, and libc++ from ndk-r23, this does not seem to be a problem anymore

thunder-coding added a commit that referenced this issue Dec 1, 2021
Additional changes made:
- The patch `deps-v8-src-trap-handler-trap-handler.h.patch` should no longer be needed needed. This should also have been fixed nodejs/node#36287. If this PR succeeds to build, I will report that this has been fixed by nodejs/node@e3f8988#diff-f350b1b5e53caf9ecde4240ba8f544b3be4f278e4f8efa86b54c30ea625b8b1d
- The libatomic hack has been removed since #3092 has been reported to have been fixed with newer NDK
thunder-coding added a commit that referenced this issue Dec 1, 2021
Additional changes made:
- The libatomic hack has been removed since #3092 has been reported to have been fixed with newer NDK
thunder-coding added a commit that referenced this issue Dec 2, 2021
Additional changes made:
- The libatomic hack has been removed since #3092 has been reported to have been fixed with newer NDK
@xtkoba xtkoba removed the wontfix Issue won't be fixed label Oct 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Something is not working properly
Projects
None yet
Development

No branches or pull requests

4 participants