Currently, we are using binutils 2.25.1 when building with GCC from tooltool because the GCC package from tooltool contains binutils. That's however not the case when building with clang from tooltool because the clang package from tooltool doesn't contain binutils. The result is that we end up using whatever binutils are on the underlying OS, which in most cases is CentOS 6, which comes with ancient binutils. Which are suspected to cause problems in bug 1253299. So, it would be good to have binutils shipped with clang as well. There are multiple ways we can do this: - Build binutils when building clang from build-clang.py, like it's built in build-gcc.sh. - Copy the necessary binutils tools from the GCC archive to the clang archive in build-clang.py (we already copy files like headers and libgcc this way, so it's not entirely crazy) - Build a separate tooltool package for binutils. The latter would have the advantage of allowing independent updates of binutils in the future, but would require more work (especially since I'll insist on having the package built on taskcluster the same way the GCC and clang packages are built on taskcluster).

:glandium do you know who might be able to work on this and in what timeframe? Trying to assess this as getting the ASAN builds on TC is a Q2 releng goal

I'd say anyone (and I can give directions if necessary), which then makes it more of a management question, which I guess would fall on jgriffin or dburns.

Note that to unblock bug 1253299 or at least check that this would, indeed, unblock bug 1253299, it would be enough to pull the gcc tarball from tooltool (i.e. add it to the relevant manifest) and add $topsrcdir/gcc/bin to $PATH so that the `ld` program is pulled from there instead of /usr/bin.

I think gbrown may be able to help with at least comment #3

Sure, I'll check on comment 3 right away.

thanks gbrown!

That seems to work just fine: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4fc6e08234e8 Thanks glandium!

Thanks gbrown, I really appreciate it, I'll attach some new patches in bug 1272629

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

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

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64742/diff/1-2/

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64742/diff/2-3/

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64742/diff/3-4/

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package https://reviewboard.mozilla.org/r/64742/#review62086 ::: build/unix/build-binutils/build-binutils.sh:3 (Diff revision 4) > +#!/bin/bash > + > +binutils_version=2.26 Let's start with keeping the same version as the one we're currently using: 2.25.1 ::: build/unix/build-binutils/build-binutils.sh:19 (Diff revision 4) > +wget -c -P $TMPDIR ftp://ftp.gnu.org/gnu/bison/bison-${bison_version}.tar.xz || exit 1 > +tar xf $TMPDIR/bison-${bison_version}.tar.xz > + > +# Build Bison > +mkdir bison-objdir > +cd bison-objdir > + > +../bison-${bison_version}/configure --prefix /tools/bison/ || exit 1 > +make $make_flags || exit 1 > +make install $make_flags DESTDIR=$root_dir || exit 1 It would be better to avoid doing this at all. The simple solution is just to install bison in the taskcluster docker image used. You can do this by changing testing/docker/centos6-build/system-setup.sh. You'll then probably need to update testing/docker/centos6-build/VERSION, testing/docker/centos6-build-upd/VERSION and testing/docker/centos6-build-upd/Dockerfile (that's what the last change to testing/docker/centos6-build/system-setup.sh required, but you should check with Dustin Mitchell whether that's still necessary, because things may have changed since the last change to that file) ::: taskcluster/ci/legacy/tasks/branches/base_jobs.yml:813 (Diff revision 4) > + linux64-binutils: > + task: tasks/builds/linux64_binutils.yml > + root: true > + when: > + file_patterns: > + - 'build/build-binutils/**' build/unix/build-binutils/**

Update centos6 build version from 0.1.6 to 0.1.7 Review commit: https://reviewboard.mozilla.org/r/65328/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/65328/

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64742/diff/4-5/

Comment on attachment 8772560 [details] Bug 1272629 - Add bison package installation to taskcluster docker setup script Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65328/diff/1-2/

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64742/diff/5-6/

https://reviewboard.mozilla.org/r/65328/#review62378 You'll want to r? dustin for this patch. ::: testing/docker/centos6-build/system-setup.sh:274 (Diff revision 2) > install libuuid-devel > install openssl-static > install cmake > install subversion > + > +# required for building binutits typo on binutils

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package https://reviewboard.mozilla.org/r/64742/#review62380 ::: build/unix/build-binutils/build-binutils.sh:4 (Diff revision 6) > +#!/bin/bash > + > +binutils_version=2.25.1 > +#bison_version=3.0.2 Please remove the commented parts building bison entirely.

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64742/diff/6-7/

Comment on attachment 8772560 [details] Bug 1272629 - Add bison package installation to taskcluster docker setup script Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65328/diff/2-3/

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64742/diff/7-8/

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package https://reviewboard.mozilla.org/r/64742/#review62384

Uploaded the resulting binutils package to tooltool. "size": 27710776, "digest": "3ab85f17cbb26a0d91eb55c6724323de32df298caecec40559b1d7cf275344a42b8ba6d6dd9cfdfde4307dcb2af07a82dae88aa0b00590efaf6c09cf457d4a24", From there, the next steps are: - Build a new gcc package without binutils - Add the binutils package to the tooltool manifests for all builds using gcc.tar.xz - Change the build/unix/mozconfig.* files to use binutils from the binutils package instead of gcc - Remove the gcc package from tooltool manifests for clang-based builds

Comment on attachment 8771645 [details] Bug 1272629 - Add taskcluster task to build binutils package https://reviewboard.mozilla.org/r/64742/#review62604 ::: build/unix/build-binutils/build-binutils.sh:17 (Diff revision 8) > +wget -c -P $TMPDIR ftp://ftp.gnu.org/gnu/binutils/binutils-${binutils_version}.tar.bz2 || exit 1 > +tar xjf $TMPDIR/binutils-${binutils_version}.tar.bz2 This is insecure. Please verify the SHA-256 of the downloaded archive.

(In reply to Gregory Szorc [:gps] from comment #27) > Comment on attachment 8771645 [details] > Bug 1272629 - Add taskcluster task to build binutils package > > https://reviewboard.mozilla.org/r/64742/#review62604 > > ::: build/unix/build-binutils/build-binutils.sh:17 > (Diff revision 8) > > +wget -c -P $TMPDIR ftp://ftp.gnu.org/gnu/binutils/binutils-${binutils_version}.tar.bz2 || exit 1 > > +tar xjf $TMPDIR/binutils-${binutils_version}.tar.bz2 > > This is insecure. > > Please verify the SHA-256 of the downloaded archive. This code is already in-tree. This can be done in a followup.

(In reply to Mike Hommey [:glandium] from comment #28) > This code is already in-tree. (in build-gcc.sh)

(and we have the same problem in build-gcc and build-clang, so this is something that ought to be addressed all at once, not as a one off for this new script, that is just split off build-gcc)

Note that you have both (gps and egoktas) bumped centos6-build to 0.1.7. Whoever lands second will need to change to 0.1.8 while rebasing.

Comment on attachment 8772560 [details] Bug 1272629 - Add bison package installation to taskcluster docker setup script https://reviewboard.mozilla.org/r/65328/#review62912

Pushed by mh@glandium.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/66c7af8b2654 Add bison package installation to taskcluster docker setup script. r=dustin https://hg.mozilla.org/integration/mozilla-inbound/rev/86d07e6bd5b7 Add taskcluster task to build binutils package. r=glandium

backed out since we have failing valgrind tests like https://treeherder.mozilla.org/logviewer.html#?job_id=32414351&repo=mozilla-inbound after this changes landed

Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d14e19de0a1e Backed out changeset 86d07e6bd5b7 https://hg.mozilla.org/integration/mozilla-inbound/rev/968c7be2c2a6 Backed out changeset 66c7af8b2654 for failing valgrind tests

The only way I can explain this is if taskcluster image building is busted, and/or the currently used image doesn't actually match the docker files in the tree. Because, really, I see no way adding bison to the docker image would trigger those valgrind errors.

(In reply to Carsten Book [:Tomcat] from comment #34) > backed out since we have failing valgrind tests like > https://treeherder.mozilla.org/logviewer.html#?job_id=32414351&repo=mozilla- > inbound after this changes landed Yes that is very strange, the only significant change to the os is that the 'bison' package is installed. I will try to see if 'bison' is causing the problem.

(without looking too closely) the other possibility is that something else has been updated in CentOS since the last build. One way to determine that may be to make a one-click loaner from both a passing and failing build, and compare the package versions.

As can be seen, there's a lot more than bison that has been changed... the joys of docker.

(In reply to Mike Hommey [:glandium] from comment #39) > Created attachment 8774935 [details] [diff] [review] > Packages differences between the build that succeeded with 0.1.6 and the > build that failed with 0.1.7 > > As can be seen, there's a lot more than bison that has been changed... the > joys of docker. This makes me sad. The base image builds "FROM centos:6." So when we build that image, it will download from Docker Hub whatever the latest version of the centos:6 image is (and it gets updated every few days). That is not deterministic. Then when we do a `yum` operation as part of building the image, we sync the latest version of the yum package database from a 3rd party server and install the latest versions of packages. The reason why so many packages changed and the reason Valgrind broke is because our image building process isn't deterministic over time. So any time someone changes the base image, we run the risk of pulling in unwanted package version bumps that could break things. It is a ticking time bomb. And it went off in this bug. You can work around the issue by using the existing centos6-build and centos6-build-upd image tags and add binutils in the desktop-build image. Unfortunately, my work in bug 1289249 to overhaul the desktop-build Docker image won't be so fortunate. Furthermore, my work will make rebuilding the desktop-build Docker image more frequent, which means higher chances of random breakage due to unwanted package updates. For better or worse, I guess I'll need to figure out package pinning. Gah, scope bloat.

There's no practical way to make builds of full linux installs like this deterministic (CentOS is bad, Ubuntu is worse!), because of those 3rd party package databases. Freezing the repos is, IMHO, impractical (we tried that with PuppetAgain and it's been a nightmare). We also need to balance the stability of a deterministic build against the need to keep the images up-to-date. Aside from the (admittedly minor) security issues with running out-of-date images, it can be very difficult to install new packages on a system that is using a years-old base image and repositories. Again, PuppetAgain has demonstrated this pretty clearly. For example, this bison install may have required either rebuilding bison against the frozen repository, or importing a lot more packages than just bison into a custom repository; and some of those additional packages may have been the cause of the valgrind issues. At any rate, with a frozen image there's a rapidly increasing disincentive to touch it as it becomes more and more out-of-date and the likelihood of bustage from an innocent modification grow without bound. I think the right place to get determinism is in using the same image for multiple builds. We can get the updates, as well as minimizing the number of extraneous changes pulled in when making changes like this, by rebuilding the docker image periodically (I had suggested weekly), in a distinct cset that can be backed out and investigated if it causes failures. We're not doing this yet, due to a shortage of round tuits, but I think it is the best way to balance the competing concerns here.

I filed bug 1289812 to track improving the system package management issue. Let's not rat hole here. I proposed a workaround in comment 40 for this bug. It is hacky. But it works and is likely far less effort than alternatives. I recommend you do that.

Installing the bison package through yum caused other packages to be updated and this caused some build problems. Shall we pull and compile bison at the time we are building binutils, instead of yum-installing it? In this way we can finish this bug I think. I believe that we do not necessarily need to yum-install bison.

I'll take a stab at fixing this.

Comment on attachment 8791453 [details] Bug 1272629 - Add bison package to desktop-build image; https://reviewboard.mozilla.org/r/78860/#review78346

Comment on attachment 8791454 [details] Bug 1272629 - Add taskcluster task to build binutils package; https://reviewboard.mozilla.org/r/78862/#review78350

Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9be03f78e363 Add bison package to desktop-build image; r=glandium https://hg.mozilla.org/integration/autoland/rev/b0d43f0d4e1e Add taskcluster task to build binutils package; r=glandium

https://hg.mozilla.org/mozilla-central/rev/9be03f78e363 https://hg.mozilla.org/mozilla-central/rev/b0d43f0d4e1e

(In reply to Mike Hommey [:glandium] from comment #26) > Uploaded the resulting binutils package to tooltool. > > "size": 27710776, > "digest": > "3ab85f17cbb26a0d91eb55c6724323de32df298caecec40559b1d7cf275344a42b8ba6d6dd9c > fdfde4307dcb2af07a82dae88aa0b00590efaf6c09cf457d4a24", > > From there, the next steps are: > - Build a new gcc package without binutils > - Add the binutils package to the tooltool manifests for all builds using > gcc.tar.xz > - Change the build/unix/mozconfig.* files to use binutils from the binutils > package instead of gcc > - Remove the gcc package from tooltool manifests for clang-based builds Hm I think we missed these parts. Shall we file a new bug for these steps?

Considering the title of this bug, and the fact it's not closed, you can continue here.

The leave-open keyword is there and there is no activity for 6 months. :kmoir, maybe it's time to close this bug?