1286788 - LLVMgold.so missing in the clang package on the Try Server, causes -flto to fail

Status: RESOLVED DUPLICATE of bug 1488307

(Firefox Build System :: Toolchains, defect)

Description

[Enes Göktaş [:egoktas]]

I tried to build Firefox with clang 3.8 and LTO enabled (-flto, which requires LLVMgold.so) in Linux 64 on the Try Server. But since LLVMgold.so is missing in the clang package on the server, the build failed early during configuration.

I added the -flto flag to build/mozconfig.common.override as follows:
CFLAGS="-flto"
CXXFLAGS="-flto"

Subsequently I pushed to the Try Server to build Firefox on the linux64-asan platform. This build failed unfortunately.

The sha512 of the clang package is:
a056a151d4f25f415b6d905136c3fa8d51d12a5a815c3df37d5663c67d59571736641a4c990884a69f78ea6b5e37a6a7bfff0417dfe38936d842d6fa0776ae54

I believe the solution would be to build clang with LLVMgold.so and create a new clang package.

Comment 1

[Mike Hommey [:glandium]]

For anyone who would want to pick this bug:
- The script used to build clang for automation is in build/build-clang.
- According to http://llvm.org/docs/GoldPlugin.html, building the gold plugin requires some manual work, it doesn't happen automatically.
- It requires the binutils headers. We don't build binutils as part of building clang, but binutils with gold and, most probably the binutils headers, are in the gcc tooltool package that is used to build clang.
- So, it might just be a case of just having to set LLVM_BINUTILS_INCDIR to the right directory when building clang.

Clang can be built on try, with the following syntax: "try: -j linux64-clang -p none" (it should also be automatically built if files are modified under build/build-clang)

Comment 2

[Enes Göktaş [:egoktas]]

Through a one-click loaner on linux64-asan I managed to build clang that includes LLVMgold.so as follows:
- add the following to the cmake_args array in build/build-clang/build-clang.py:
"-DLLVM_BINUTILS_INCDIR=%s" % (gcc_dir + "/include"),
- in build/build-clang execute:
./build-clang.py -c clang-static-analysis-linux64.json

Is there a way I could get the resulting clang.tar.xz in RelengAPI?

Also, through ssh I exported clang.tar.xz to my machine, because putting it in workspace/artifacts didn't make it show up in the Task Inspector. Is there a way to manually make this clang.tar.xz appear in Task Inspector through the one-click loaner?

Comment 3

[Mike Hommey [:glandium]]

Awesome, thanks for looking into it. So here is how we can do this:
- Turn your changes into a proper patch.
- Push it to mozreview and flag me for review.
- From there, we can push to try from mozreview with the "try: -j linux64-clang -p none" syntax
- Then we can get the resulting clang.tar.xz file from the taskcluster inspector from the build that will happen automatically.
- I can then proceed to upload the clang archive to tooltool
- And then you can update the tooltool manifests.

Comment 4

[Enes Göktaş [:egoktas]]

Attachment: Bug 1286788 - add LLVM_BINUTILS_INCDIR flag to cmake_flags, required for building LLVMgold.so

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8771157 [details] [diff] [review]
Bug 1286788 - add LLVM_BINUTILS_INCDIR flag to cmake_flags, required for building LLVMgold.so

Review of attachment 8771157 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/build-clang/build-clang.py
@@ +190,4 @@
>                    "-DCMAKE_BUILD_TYPE=%s" % build_type,
>                    "-DLLVM_TARGETS_TO_BUILD=X86;ARM",
>                    "-DLLVM_ENABLE_ASSERTIONS=%s" % ("ON" if assertions else "OFF"),
> +                  "-DLLVM_BINUTILS_INCDIR=%s/include" % gcc_dir,

This needs to be done for linux only (the script is also used to build clang on osx and windows, although that's not done on automation, at the moment, unfortunately)

Please could you use mozreview for next round, this would make a try build a click away.

Comment 6

[Enes Göktaş [:egoktas]]

Attachment: Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review commit: https://reviewboard.mozilla.org/r/64478/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64478/

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

https://reviewboard.mozilla.org/r/64478/#review61426

Thanks. I triggered a try build, but all trees are closed presently, so it will only happen when trees reopen.

Comment 8

[Enes Göktaş [:egoktas]]

https://reviewboard.mozilla.org/r/64478/#review61426

Ok thanks!

Comment 9

[Enes Göktaş [:egoktas]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64478/diff/1-2/

Comment 10

[Enes Göktaş [:egoktas]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64478/diff/2-3/

Comment 11

[Enes Göktaş [:egoktas]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64478/diff/3-4/

Comment 12

[Enes Göktaş [:egoktas]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64478/diff/4-5/

Comment 13

[Enes Göktaş [:egoktas]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64478/diff/5-6/

Comment 14

[Enes Göktaş [:egoktas]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64478/diff/6-7/

Comment 15

[Enes Göktaş [:egoktas]]

Attachment: bug 1286788 - Use binutils to build clang with LLVMgold.so plugin

Review commit: https://reviewboard.mozilla.org/r/74854/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/74854/

Comment 16

[Enes Göktaş [:egoktas]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64478/diff/7-8/

Comment 17

[Michelangelo De Simone [Offline] [:mds]]

Hi :glandium,

Enes has built a patched version of Clang (with his CFI patch) and LLVM-Gold plugin; we're at the point where we need to actually use that toolchain to build Firefox on the try servers.

In order to do that we think it'd be the time to upload the clang artifact tarball on the tooltool repository in such a way we can fetch it during the build process.

The job that built clang-cfi is https://treeherder.mozilla.org/#/jobs?repo=try&revision=fda8dc880aea&selectedJob=26489772 and the tarball is on https://queue.taskcluster.net/v1/task/cLHpTqjXSpSf4S-3vyFU7g/runs/0/artifacts/public%2Fclang.tar.xz (probably we may want to rename it with something clearer... perhaps clang-cfi.tar.xz or clang-patched-cfi.tar.xz?)

How should we proceed now to upload that tarball to the tooltool repo?:)

Comment 18

[Mike Hommey [:glandium]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

https://reviewboard.mozilla.org/r/64478/#review72968

Comment 19

[Enes Göktaş [:egoktas]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64478/diff/8-9/

Comment 20

[Enes Göktaş [:egoktas]]

We should file a new bug for patching cfi in clang 3.8.1 to make it compatible with Firefox. I removed the patch "Build clang with cfi vcall dso patch" from mozreview.

Comment 21

[Enes Göktaş [:egoktas]]

The current fix will create a clang package that includes the LLVMgold.so plugin, using the binutils tarball.

Ok to land?

Comment 22

[Mike Hommey [:glandium]]

Please add the review? flag to the relevant patches you want to land.

Comment 23

[Enes Göktaş [:egoktas]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64478/diff/9-10/

Comment 24

[Enes Göktaş [:egoktas]]

If this patch lands, I think that we should remove the binutils parts out of the gcc package.

Comment 25

[Mike Hommey [:glandium]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

https://reviewboard.mozilla.org/r/64478/#review79882

::: browser/config/tooltool-manifests/linux64/releng.manifest:3
(Diff revision 10)
> +"version": "binutils with ld.gold",
> +"size": 27710776,
> +"digest": "3ab85f17cbb26a0d91eb55c6724323de32df298caecec40559b1d7cf275344a42b8ba6d6dd9cfdfde4307dcb2af07a82dae88aa0b00590efaf6c09cf457d4a24",
> +"algorithm": "sha512",
> +"filename": "binutils.tar.xz",
> +"unpack": true
> +},

It would actually be better to create a new tooltool manifest with just gcc and binutils, put it in build/build-clang, and update the taskcluster/scripts/misc/build-clang-linux.sh script to use it.

Comment 26

[Enes Göktaş [:egoktas]]

Hey Mike, I was wondering, during the build of clang is the ld linker of the underlying OS used or the ld linker in the gcc package? 

It should actually be using the one in the binutils package right?

Comment 27

[Enes Göktaş [:egoktas]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64478/diff/10-11/

Comment 28

[Mike Hommey [:glandium]]

Currently, it's the one that comes from the gcc package. So yeah, the script will need some adjustments too to pick up binutils from the binutils package.

Comment 29

[Mike Hommey [:glandium]]

Comment on attachment 8771208 [details]
Bug 1286788 - Set binutils include path to build Clang with LLVMgold.so plugin;

https://reviewboard.mozilla.org/r/64478/#review81638

cf. last comment you'd also need to make the script use the binutils binaries from the binutils directory. For good measure, you should test with a new gcc package without binutils in it.

::: build/build-clang/build-clang.manifest:14
(Diff revision 11)
> +},
> +{
> +"version": "gcc 4.8.5 + PR64905",
> +"size": 80160264,
> +"digest": "c1a9dc9da289b8528874d16300b9d13a997cec99195bb0bc46ff665216d8535d6d6cb5af6b4b1f2749af6815dab12e703fdb3849014e5c23a70eff351a0baf4e",
> +"algorithm": "sha512", 

extra whitespace on this line

Comment 30

[Kim Moir [:kmoir] ET]

Triaging, moving bugs to Firefox Build System to close out the Firefox Build Config bucket.  :glandium is this bug still relevant or can it be closed?