Android clang should include x86-64
Categories
(Firefox Build System :: Toolchains, enhancement)
Tracking
(firefox68 fixed)
| Tracking | Status | |
|---|---|---|
| firefox68 | --- | fixed |
People
(Reporter: truber, Assigned: truber)
References
Details
Attachments
(1 file)
To build x64_64 ASan for Android, clang needs it added as a target.
|
Assignee | |
Comment 1•6 years ago
|
||
|
|
Assignee | |
Updated•6 years ago
|
Pushed by nerli@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e0cacf5d936f
Add x86_64 target to Android Clang build configuration. r=chmanchester
|
||
Comment 3•6 years ago
|
||
Backed out changeset e0cacf5d936f (bug 1537751)for causing instrumented-build-android bustage
push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&selectedJob=235328305&revision=e0cacf5d936fbc2f277af6e5b680eaa4d95eec84
backout: https://hg.mozilla.org/integration/autoland/rev/0f0c2192b22fde935172600941710cd5696b09a5
|
|
Assignee | |
Comment 4•6 years ago
|
||
The default target is changing to x86_64--linux-android, but for some reason --target=x86_64-linux-gnu is no longer passed to HOST_CC, so the Fennec build fails on the first host utility in the build.
Nathan, do you know why --target would be dropped from HOST_CC?
|
||
Comment 5•6 years ago
|
||
ISTR glandium making changes in this area a while back...but you shouldn't need --target for HOST_CC, because the clang you're using is obviously running on the host and therefore doesn't need any special --target'ing. But the host clang is getting --gcc-toolchain, which doesn't make a lot of sense to me:
[task 2019-03-22T07:14:37.359Z] 07:14:37 INFO - /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang --gcc-toolchain=/builds/worker/workspace/build/src/clang -std=gnu99 -o host_nsinstall.o -c -DXP_UNIX -O3 -DNDEBUG=1 -DTRIMMED=1 -D_UNICODE -DUNICODE -I/builds/worker/workspace/build/src/config -I/builds/worker/workspace/build/src/obj-firefox/config -I/builds/worker/workspace/build/src/obj-firefox/dist/include -MD -MP -MF .deps/host_nsinstall.o.pp -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr /builds/worker/workspace/build/src/config/nsinstall.c
[task 2019-03-22T07:14:37.359Z] 07:14:37 INFO - In file included from /builds/worker/workspace/build/src/config/nsinstall.c:10:
[task 2019-03-22T07:14:37.359Z] 07:14:37 INFO - In file included from /usr/include/stdio.h:27:
[task 2019-03-22T07:14:37.359Z] 07:14:37 INFO - /usr/include/features.h:364:12: fatal error: 'sys/cdefs.h' file not found
[task 2019-03-22T07:14:37.359Z] 07:14:37 INFO - # include <sys/cdefs.h>
[task 2019-03-22T07:14:37.359Z] 07:14:37 INFO - ^~~~~~~~~~~~~
[task 2019-03-22T07:14:37.360Z] 07:14:37 INFO - 1 error generated.
[task 2019-03-22T07:14:37.360Z] 07:14:37 INFO - /builds/worker/workspace/build/src/config/rules.mk:724: recipe for target 'host_nsinstall.o' failed
[task 2019-03-22T07:14:37.360Z] 07:14:37 ERROR - make[4]: *** [host_nsinstall.o] Error 1
--gcc-toolchain comes from here: https://searchfox.org/mozilla-central/rev/56705678f5fc363be5e0237e1686f619b0d23009/mobile/android/config/mozconfigs/common#13
Fixing bug 1451104 might have resolved the issues that required --gcc-toolchain to be added, see bug 1428158 comment 34 and following. So your options are:
- Remove the
--gcc-toolchainbits from the above. - Remove
-gcc-toolchainjust fromHOST_CC, as the comment about C++ libraries shouldn't apply to compiling C. That doesn't really resolve the question of<sys/cdefs.h>not being available, but maybe it will matter less for C++...?
Report back with trying those and we'll see where we sit then?
|
|
Assignee | |
Comment 6•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d2453b0c01bd1e17f460bbaeb804cd059539fd8a
Entirely removing --gcc-toolchain broke in gradle-dependencies (I couldn't see the error, and couldn't figure out where to add --debug to get gradle to show me).
Removing it from just HOST_CC got me to the same error (missing <sys/cdefs.h>), so I guess it isn't --gcc-toolchain causing that.
|
|
||
Comment 8•6 years ago
|
||
Ah, so the arm build(s) are using linux64-clang, and the x86-64 build is using linux64-clang-android-cross (the instrumented-build-android build also happens to be using linux64-clang-android-cross). And running the clang from that toolchain says:
root@72f4ce21c775:~/workspace/build/src# ./clang/bin/clang -std=gnu99 -o x.o -c -DXP_UNIX -O3 config/pathsub.c -v
clang version 7.0.1 (tags/RELEASE_701/final) (llvm/tags/RELEASE_701/final 349247)
Target: x86_64--linux-android
Thread model: posix
InstalledDir: /builds/worker/workspace/build/src/./clang/bin
"/builds/worker/workspace/build/src/clang/bin/clang-7" -cc1 -triple x86_64--linux-android -emit-obj -disable-free -disable-llvm-verifier
-discard-value-names -main-file-name pathsub.c -mrelocation-model pic -pic-level 2 -pic-is-pie -mthread-model posix -fmath-errno -masm-ver
bose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -target-feature +sse4.2 -target-feature +popcnt -dwarf-colu
mn-info -debugger-tuning=gdb -momit-leaf-frame-pointer -v -coverage-notes-file /builds/worker/workspace/build/src/x.gcno -resource-dir /bu
ilds/worker/workspace/build/src/clang/lib64/clang/7.0.1 -D XP_UNIX -internal-isystem /usr/local/include -internal-isystem /builds/worker/w
orkspace/build/src/clang/lib64/clang/7.0.1/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -O3 -std=gnu9
9 -fdebug-compilation-dir /builds/worker/workspace/build/src -ferror-limit 19 -fmessage-length 138 -fobjc-runtime=gcc -fdiagnostics-show-o
ption -fcolor-diagnostics -vectorize-loops -vectorize-slp -o x.o -x c config/pathsub.c -faddrsig
clang -cc1 version 7.0.1 based upon LLVM 7.0.1 default target x86_64-linux-android
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
/usr/local/include
/builds/worker/workspace/build/src/clang/lib64/clang/7.0.1/include
/usr/include
End of search list.
In file included from config/pathsub.c:10:
In file included from /usr/include/assert.h:35:
/usr/include/features.h:364:12: fatal error: 'sys/cdefs.h' file not found
# include <sys/cdefs.h>
^~~~~~~~~~~~~
1 error generated.
Notice that the include search path doesn't include any directory with sys/cdefs.h in it; the normal linux64-clang does look in /usr/include/x86_64-linux-gnu. And that's because this clang is compiling for...-triple x86_64--linux-android, which is completely wrong. And I think that happens because of code hereabouts:
https://searchfox.org/mozilla-central/source/build/build-clang/build-clang.py#275
which on reflection, doesn't look correct.
I guess what happens for our current builds is that LLVM has a particular default target, but since it doesn't match the host architecture (?), clang defaults back to x86_64-linux-gnu. But for Jesse's builds, we now have a default target, x86-linux-android, that looks vaguely like our host architecture, so we try to use that, and everything falls over? That's just guessing, though.
I think the bandaid fix is to change:
https://searchfox.org/mozilla-central/source/build/build-clang/build-clang.py#557
to use object_pairs_hook=collections.OrderedDict or something like that, so that the ordering in our json file is respected, rather than getting whatever target Python feels like returning today. And then when we build clang with Jesse's patch stack, we'll get a default target of something other than x86_64-linux-android, just like we have today.
But I think we should also consider removing -DLLVM_DEFAULT_TARGET_TRIPLE or figuring out how to specify it when we're building stuff for Android. Because that seems like a huge footgun. (Chris, since you added that code, do you remember why we need LLVM_DEFAULT_TARGET_TRIPLE? Also, egg on my face for reviewing that change...)
|
||
Comment 9•6 years ago
|
||
Thanks for looking into it, and yep, that certainly doesn't look right. I just tested on try and I think we can just remove LLVM_DEFAULT_TARGET_TRIPLE. I will remove in a separate bug.
|
|
||
Comment 10•6 years ago
|
||
Jesse, can you try Chris's patch in bug 1542400 along with your patch stack?
|
|
Assignee | |
Comment 11•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7beac64a0d816c1f93a69cb1ef3314d334bd12da
I added the instrumented build to that try, which is what busted when this landed previously. So can I update this patch to land again and figure out why the fuzzing build doesn't work in bug 1482167?
|
|
||
Comment 12•6 years ago
|
||
(In reply to Jesse Schwartzentruber (:truber) from comment #11)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7beac64a0d816c1f93a69cb1ef3314d334bd12da
I added the instrumented build to that try, which is what busted when this landed previously. So can I update this patch to land again and figure out why the fuzzing build doesn't work in bug 1482167?
That seems reasonable. The asan build error is the C compiler getting invoked from rust not receiving the correct build flags, which seems very solvable. Obviously the C compiler gets the correct build flags in all other Android configurations (?), so it's just a matter of tracing down where those build flags don't get transferred down to Rust correctly. Happy to answer questions about that.
|
|
Assignee | |
Comment 13•6 years ago
|
||
Thanks!
|
||
Comment 14•6 years ago
|
||
Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2ee12a893122
Add x86_64 target to Android Clang build configuration. r=chmanchester
|
||
Comment 15•6 years ago
|
||
| bugherder | ||
Description
•