Build for Android with Clang #2229
Conversation
|
So this currently adds 12 new travis targets that only build. I'm not sure that it's useful to try and build all those targets. Is it possible to run the test suite for any of them? |
Configurations/unix-Makefile.tmpl
Outdated
| @@ -170,10 +170,10 @@ HTMLSUFFIX=html | |||
|
|
|||
|
|
|||
| CROSS_COMPILE= {- $config{cross_compile_prefix} -} | |||
| CC= $(CROSS_COMPILE){- $target{cc} -} | |||
| CC= {- $target{cc} -} | |||
There was a problem hiding this comment.
This will most likely break things, nor do I see why you're removing it.
There was a problem hiding this comment.
Presumably reason is that you cross-compile by calling clang with no prefix. Instead you rely on -target option...
Anyway, I'd also say that this is inappropriate. Instead just don't set any CROSS_COMPILE variable when using clang.
|
I've been looking at travis config. This adds whole hour of CPU time. It's hardly appropriate. Ironic thing is that significant portion of time is used unpacking NDK zip (in each build), as well as installing compilers that won't be used... But even if optimized, I'd also say that we should pick just one target for continuous testing. I suppose clang would be most appropriate since they apparently plan to eventually remove gcc from NDK... |
|
I agree we don't want to add all those build-only targets. At most one. And all those android config targets should all be moved into a separate file. |
|
Thanks for all the feedbacks. Please let me explain them one by one. About bloated In response to @kroeckx's comment:
Currently I use the Termux app to get a working copy of Perl on Android. Unfortunately, seems it doesn't provide useful APIs to control it - human operations are necessary to get openssl's test suite running on Android. About the problem of If that sounds fine, I'll drop CROSS_COMPILE-related patches. Last, about @richsalz's comment:
Reasonable. What's the naming convention for files in Configurations? Is "50-android.conf" a good name? |
This is not strictly true. If linked statically OpenSSL Android binaries can be executed on Linux host. If Android target is x86[_64], then ( |
Or you can modify $PATH so that it uses target-specific un-prefixed tools. I mean there are un-prefixed binaries too, e.g. in toolchains/aarch64-linux-android-4.9/prebuilt/linux-x86_64/aarch64-linux-android/bin/... Trouble with special treatment of CROSS_COMPILE triggered by a word clang is that it becomes less flexible and non-uniform. For example CROSS_COMPILE can as well be a directory name, not necessarily |
I'd vote for 20-android.conf to denote that it's not exclusively community-supported targets. I mean 10-* is what we officially support( ( |
Configurations/10-main.conf
Outdated
| @@ -893,7 +893,7 @@ sub vms_info { | |||
| # CROSS_COMPILE=arm-linux-adroideabi- | |||
| # PATH=$ANDROID_NDK/toolchains/arm-linux-androideabi-4.8/prebuild/linux-x86_64/bin | |||
| # | |||
| "android" => { | |||
| "android-base" => { | |||
| inherit_from => [ "linux-generic32" ], | |||
There was a problem hiding this comment.
Do use template => 1 in intermediate targets!
Configurations/10-main.conf
Outdated
| "android-armeabi" => { | ||
| inherit_from => [ "android", asm("armv4_asm") ], | ||
| "android-armeabi-base" => { | ||
| inherit_from => [ asm("armv4_asm") ], |
There was a problem hiding this comment.
This are practically indistinguishable from assemby module templates. One can argue that there should be common ELF-specific templates that can be used in both Linux and Android targets. Meanwhile I'd suggest to omit them and inherit directly from *_asm in compiler-specific targets. Yes, there will be duplicates, but as already said, it makes more sense to unify all ELF targets, which would be separate effort.
There was a problem hiding this comment.
I feel this strange. Take android-mips-base for example:
"android-mips-base" => {
inherit_from => [ asm("mips32_asm") ],
template => 1,
perlasm_scheme => "o32",
},
I thought perlasm_scheme => "o32" is an extra directive for asm("mips32_asm")? Move asm("mips32_asm") to compiler-specific targets sounds strange. Or I should move perlasm_scheme as well?
There was a problem hiding this comment.
Move asm("mips32_asm") to compiler-specific targets sounds strange.
Not to seasoned MIPS programmer :-) Normally, i.e. in cases other than Android, MIPS systems support multiple ABIs. And in such cases you'd actually "tie" perlasm_scheme and compiler flags together, i.e. they have to match. But my objection is really more about minimizing templates. It does make code more readable. I mean when you look at say android-mips, you have to unravel one less template. And it's also better for sheer amount of lines. I mean pair of perlasm_scheme in android-mips and android-mips-clang is shorter than template...
There was a problem hiding this comment.
when you look at say android-mips, you have to unravel one less template.
I meant "you would have to unravel one less template [if there was no android-mips-base]".
Configurations/10-main.conf
Outdated
| inherit_from => [ "android-base" ], | ||
| cc => "clang", | ||
| cxx => "clang++", | ||
| cflags => add(picker(default => "-gcc-toolchain \$(GCC_TOOLCHAIN) -fno-integrated-as")), |
There was a problem hiding this comment.
As for -fno-integrated-as. Is it actually so that all platforms "require" it? "Require" in quotes is reference to the fact that there was bug reported with ARM (right?) integrated clang assembler. If only single platform is problematic, then I'd argue that only that one should get the flag...
There was a problem hiding this comment.
Currently on NDK r14 beta 1, arm, aarch64 mips and x86 requires it, so I add -fno-integrated-as to those targets.
Here are failing commands in those targets: errors.txt. I guess x86 and mips can be fixed in NDK or LLVM. For it's fixed I suggest disabling integrated assemblers.
Configurations/10-main.conf
Outdated
| "android64-x86_64" => { | ||
| inherit_from => [ "android64", asm("x86_64_asm") ], | ||
| "android64-x86_64-base" => { | ||
| inherit_from => [ asm("x86_64_asm") ], |
There was a problem hiding this comment.
There actually is a bug here. Not contributor's fault! It was there all along. One has to omit RC4_CHAR inherited from linux-generic64...
First, I don't think running Android binaries on Linux brings persuasive test results. Unlike ELF on FreeBSD or PE/PE+ on Unix (via Wine), Linux is not designed to be compatible with Android binaries. Second, static builds are broken (android/ndk#272)
Got it. I remove CROSS_COMPILE related changes.
Done! |
|
Modification is difficult to read - to understand modification I have to split into 2 parts : to extract existing android targets into new file and then updates from this issue. Question is why to use -mandroid for gcc? I think that removing option -mandroid is enough to allow build with clang. Users of oldest gcc has to specify -mandroid and clang users should use -target LLVM_TRIPLET. |
|
In *-clang targets, there are more flags than -target, for example -fno-integrated-as. It sounds a bad idea to force OpenSSL users to understand which targets require -fno-integrated-as.
I guess OpenSSL aims to support as old toolchains as possible without bothering users with compiler internals. |
Right. |
It's safer to put it into config than to let user mistype it. Well, user still has a number of environment variables to get wrong, but goal is to minimize amount of things to get wrong, not to maximize. Besides the triplet is tied to assembly pack, so that it's really not something that user is free to mix-n-match.
??? Cross-compile prefix is enough with gcc. As for clang suggestion is to not set cross-compile prefix, but manipulate PATH so that you have clang as well as un-prefixed tools such as |
When it comes to statically linked executable it all boils down to system call numbers. So the question is if Android changes any of Linux system call numbers, even more specifically those used by OpenSSL test suite. And there is a lot of evidence that it doesn't. As already mentioned, 'make test' does run and produced meaningful output on Linux, be it native or qemu-user-assisted. The keyword, again, is that Android is built on top of Linux kernel and [most of] its user-kernel interface is unmodified. Also once again note that we are not looking at arbitrary Android application, but specifically at OpenSSL test programs which are just "console" applications.
Well, this of course can be problematic. But I'd count on linker to tell me when it becomes impossible to use them. I mean as long as one can link and as long as tests produce meaningful result, I see no reason not to trust them. |
In other words suggestion is to have unified Android target[s] and put responsibility of setting CC environment variable and passing compiler-specific things like -target at config-time on user. While this is an option, and there is kind of "tradition" to give users quite a bit of power at config-time, I'd say that the said "tradition" is not really about unconstrained flexibility. I mean there are boundaries for the provided flexibility. This of course asks for question where/how to draw this boundary. For example there is no boundary between gcc and clang on Linux, right? In sense that there is no dedicated clang target and you can switch to clang by setting CC environment variable. Yes, but in such "traditional" case we are looking at so to say single-triplet installations, and there are no additional tidbits that have to be tied up together. This is not case here, as already mentioned, the triplet has to match assembly pack (and not to forget integrated-as limitation), and I'd say that arranging dedicated clang targets is justified here... |
|
Andy Polyakov wrote:
>> Question is why to use -mandroid for gcc?
> I guess OpenSSL aims to support as old toolchains as possible without bothering users with compiler internals.
Right.
Removal of flag -mandroid from current build configuration does not mean
to stop support for old compilers.
It is enough to update current comment in 10-main.conf.
On remark: for abi filters "armeabi", "armeabi-v7a" application has to
pack separate libraries even the build flags are same. NDK build pass
...-march=armv5te... for first abi and ...-march=armv7-a ... for second.
|
|
Andy Polyakov wrote:
> I think that removing option -mandroid is enough to allow build with clang. Users of oldest gcc has to specify -mandroid and clang users should use -target LLVM_TRIPLET.
In other words suggestion is to have unified Android target[s] and put responsibility of setting CC environment variable and passing compiler-specific things like -target at config-time on user.
Yes . Idea is to minimize build configurations and in same way do not
stop users to build for each supported by NDK ABI.
The list of supported abiFilters in NDK r13b is following: "armeabi",
"armeabi-v7a", "x86", "mips", "arm64-v8a", "x86_64", "mips64".
ABI armeabi-v7a-hard is no longer supported.
How many build configurations to define?
6(7 if distinguish two arm ) plus 2 generic (no-asm) for GCC and same
number for CLANG?
[SNIP]This is not case here, as already mentioned, the triplet *has to* match assembly pack
What you mean by "triplet *has to* match assembly pack"?
|
|
Hi Andi,
Andy Polyakov wrote:
> clang users should use -target LLVM_TRIPLET.
It's safer to put it into config than to let user mistype it.
Build is quote complex. I can not see way to minimize chance user to
"mistype" some of options.
- 7 abi filters
- 6 GCC target(host) triplets (not compatible with above, i.e. host_cpu
!= ABI)
- 6 platform directories (sysroot) (also not compatible with above)
- 7 LLVM target triplets (partially compatible with GCC)
A lot of room for failures.
For sample lets see one clang compilation command for a native library
(abi armeabi) :
/opt/android/sdk/ndk-bundle/toolchains/llvm/prebuilt/linux-x86_64/bin/clang++
\
-target armv5te-none-linux-androideabi \
-gcc-toolchain
/opt/android/sdk/ndk-bundle/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64
\
--sysroot=/opt/android/sdk/ndk-bundle/platforms/android-9/arch-arm ...
Even with correct llvm triplet from OpenSSL build configuration user
could use set wrong toolchain path or sysroot.
My opinion is do not share responsibility between openssl configuration
and user build command.
What about just to comment existing build configuration?
For instance:
======
# a) abiFilters "armeabi"
# gcc triplet arm-linux-androideabi
# llvm triplet armv5te-none-linux-androideabi
# b) abiFilters "armeabi-v7a"
# gcc triplet arm-linux-androideabi
# llvm triplet armv7-none-linux-androideabi
"android-armeabi" => {
inherit_from => [ "android", asm("armv4_asm") ],
},
======
Well, user still has a number of environment variables to get wrong, but goal is to minimize amount of things to get wrong, not to maximize. Besides the triplet is tied to assembly pack, so that it's really not something that user is free to mix-n-match.
What is relation between triplet and "assembly pack"?
Roumen
|
|
Some initial comments on build with llvm (after remove of -mandroid).
Build of master branch succeed with gcc 4.9 for all 7 ABI-filters. |
Well, following this logic we don't need any config lines whatsoever. Why not let user specify everything? I'm talking about everything, not just Android :-) Yes, I recognize that user has enough ways to screw it up by setting wrong environment variables, yet, I'd consider retaining control over things we can reliably control as desirable. "triplet matching assembly pack" means that you can't use -target i686-none-linux-android with ARM pack. But I think I see your point now. You're saying that there is need for specifying distinct -targets for same assembly pack, more specifically ARMv4 pack. Right? In gcc case you have freedom to pass additional |
Platform headers in platforms/android-X/arch-Y/usr/include are not reliable. Use unified headers instead - this is my next goal after this PR. Also, I'd like to hear NDK developers for Clang bugs (?) on x86 and MIPS before moving on. |
To be consistent with openssl/openssl#2229
To be consistent with openssl/openssl#2229
openssl-machine
added
the
hold: cla required
|
Hello, NDK r14 is out, and the aforementioned two bugs are still in the NDK, so -fno-integrated-as workaround is still necessary. I think this pull request is ready for a second review. For CLA: I'll read it and submit it soon |
yan12125
changed the title
openssl-machine
added
the
hold: cla required
* PROBLEM:
* most recent version (r14b) of `android-ndk` uses `clang` for cross-compilation
* BUT: `openssl` cannot compile successfully w/ `clang`
* AND: we depend on `openssl` transitively through `ics-openvpn`
while trying to use `android-ndk` r14b
* FIX:
* downgrade to `android-ndk` (12b) (most recent versoin that still
uses `gcc` instead of `clang`)
* modify some of the default
* REMAINING PROBLEMS:
* some string translations for Jamaica now break the build (unclear
why -- outdated country abbreviation? ja for jm???)
* we are now using a version of ndk that is 2 versions old and a
version of ics-openvpn (pinned to a 3.1.2016 commit via submodule)
that depends on an outdated version of `openssl`, which raises
security concerns. updating to the most recent version will force
us to wade into all the dependency problems amongst `ics-openvpn`/`openssl`/`ndk`
* REFERENCES:
* on `openssl` incompatibility w/ clang: openssl/openssl#2229
* on `ics-openvpn` problems with `ndk`: android/ndk#144
|
Any hope of this landing soon? |
|
This still needs a CLA in place ... |
|
I'm currently not working on this and I don't think I can continue it in near future. I have a local patch at https://github.com/yan12125/python3-android/blob/563bd3388b48c7cf5cfaee92339953b2e393fed6/mk/openssl/ndk-clang-targets.patch, which should work for NDK r16. If anyone wants to finish this PR, I guess it's legal to start from my local patch as the latter is released with WTFPL. |
|
@yan12125 hello,I just use clang to build openssl according to your PL but encounter some errors.My environments are as follows:
I just update the two conf files: 10-main.conf and 20-android.conf,the option about pick os/compiler from: is android-armeabi-clang.When issue ./Configure,I found that two variables cannot get the values:
so I input them in 20-android.conf manually. The configure results are as follows: `./Configure zlib-dynamic no-shared --openssldir=/Users/zexu/github/ijkplayer/android/contrib/build/openssl-armv7a/output --cross-compile-prefix=arm-linux-androideabi- android-armeabi-clang THIRTY_TWO_BIT mode Configured for android-armeabi-clang. ` the compile is OK but link errors happen at last:
./libcrypto.a(armcap.o):crypto/armcap.c:function OPENSSL_rdtsc: error: undefined reference to '_armv7_tick' can you please give me some tips about how to resolve this problem?? ==========================================================================
echo "" no erros appear this time ,but executable file openssl is not generated,but two static librarieslibcrypto.a and libssl.a are generated successfully. openssldir is empty |
|
Harlan Chen wrote:
@yan12125 hello,I just use clang to build openssl according to your PL but encounter all kinds of errors.My environments are as follows:
[SNIP]
clang38: error: unknown argument: '-mandroid'
[SNIP]
can you please provide me the configure options or some detailed steps ,thanks.
I can not remember issue where was discussed that removal if '-mandroid'
is enough to build.
If is an GCC flag - applied automatically since gcc 4.6.
Roumen
|
|
As both OpenSSL and Android NDK are changing fast, I'm not sure I have time catching up both. As a result, it's better to close this pull request to prevent more confusions. Thanks for all review comments and testing feedbacks. |
|
I am using android-ndk 14 and i am getting the clang:error: unknown argument: '-mandroid' when i am trying to build openssl can any one help me |
|
@jainabhinav249 https://github.com/openssl/openssl/blob/master/NOTES.ANDROID should be helpful. Also make sure you're using the latest OpenSSL 1.1.1. Older OpenSSL versions may not work fine with Android. |
|
Thank you so much @yan12125 |
|
So bitcoin/bitcoin#16110 is merged... |
Checklist
Description of change
Add support for building for Android with NDK's clang. Targets master yet I guess 1.1 branch can be applied as well.
Documentation incomplete. See util/configure-android.sh and .travis.yml for usage of ./Configure. See also https://travis-ci.org/yan12125/openssl/builds/191712918 for a test build.