913442 - Build and deploy binutils 2.23.1 for testing

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: CIDuty, task)

Description

[Ryan VanderMeulen [:RyanVM]]

Many of the PGO compilation crashes we've seen since upgrading to gcc 4.7.3 are in program as. It was brought up that we use an ancient release of binutils which might be causing problems.

We would like to test the current 2.23.1 release to see if it improves the crash situation.

Comment 1

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

ryanvm: before we go deploy something across all machines to see if it improves the situation (or makes things worse - in production), maybe it would be best to start with getting you a loaner, to see what you find when experimenting there?

Comment 2

[Ryan VanderMeulen [:RyanVM]]

I believe Trev volunteered to work on this on IRC.

I guess this isn't something we can test on Try like gcc updates?

Comment 3

[Trevor Saunders (:tbsaunde)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #2)
> I believe Trev volunteered to work on this on IRC.
> 
> I guess this isn't something we can test on Try like gcc updates?

My plan is a tooltool package, so try should be possible

Comment 4

[Trevor Saunders (:tbsaunde)]

Attachment: wip replacement for build-gcc.py

The main problem I'm aware of with this is that the built  binutils / gc don't use the glibc unless LD_LIBRARY_PATH is set, and instead use the system one, I'm not sure if we care about building glibc and if we do if there's a way to use a non default library path at build time or if mozconfigs that use this tool chain should just set LD_LIBRARY_PATH.

All other comments welcome of course! :)

Comment 5

[Mike Hommey [:glandium]]

I don't care about glibc, as long as your system one is as old as the one on build slaves.

Comment 6

[Trevor Saunders (:tbsaunde)]

(In reply to Mike Hommey [:glandium] from comment #5)
> I don't care about glibc, as long as your system one is as old as the one on
> build slaves.

I'll just rm that bit and build in a centos 6 chroot then seems a lot simpler.

Comment 7

[Trevor Saunders (:tbsaunde)]

It turns out that gcc gcc 4.7's build system really doesn't like building the newer mpfr we've been using, at some point mpfr moved mpfr.h from topsrcdir of their tree to src/ and gcc doesn't know how to tell mpc to look there, so I think our options are build each lib on its own then point gcc at them like the spec file did, or we can use the contrib/download_prerequisites script  as is which  seems simpler, and maybe better since those library versions are presumably what are expected to be used.

Comment 8

[Trevor Saunders (:tbsaunde)]

Attachment: bug 913442 - rewrite build-gcc.py

I went with just running download_prerequisites as is for now, if
the'res a good reason to go for building all the libs on there own that
shouldn't be too hard.

I checked that a x86_64 build with this toolchain works locally

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 803505 [details] [diff] [review]
bug 913442 - rewrite build-gcc.py

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

::: build/unix/build-gcc/build-gcc.sh
@@ +6,5 @@
> +
> +root_dir=$(mktemp -d)
> +cd $root_dir
> +
> +wget ftp://ftp.gnu.org/gnu/binutils/binutils-$binutils_version.tar.bz2 || exit 1

might be worth keeping the tarball around somewhere (in $TMPDIR?) for subsequent attempts (not requiring to re-download).

@@ +7,5 @@
> +root_dir=$(mktemp -d)
> +cd $root_dir
> +
> +wget ftp://ftp.gnu.org/gnu/binutils/binutils-$binutils_version.tar.bz2 || exit 1
> +tar xf binutils-$binutils_version.tar.bz2

tar jxf

@@ +11,5 @@
> +tar xf binutils-$binutils_version.tar.bz2
> +mkdir binutils-objdir
> +cd binutils-objdir
> +../binutils-$binutils_version/configure --prefix $root_dir/tools/gcc/ --enable-gold  --enable-plugins --disable-nls|| exit 1
> +make -j12 || exit 1

probably better not to hardcode a -jN value.

@@ +16,5 @@
> +make install -j12 || exit 1
> +cd ..
> +
> +wget ftp://ftp.gnu.org/gnu/gcc/gcc-$gcc_version/gcc-$gcc_version.tar.bz2 || exit 1
> +tar xf gcc-$gcc_version.tar.bz2

same remarks as for binutils

@@ +22,5 @@
> +
> +./contrib/download_prerequisites
> +
> +# gcc 4.7 doesn't dump a stack on ICE so hack that in
> +git apply $gcc_bt_patch || exit 1

Use patch instead of git.

@@ +27,5 @@
> +
> +cd ..
> +mkdir gcc-objdir
> +cd gcc-objdir
> +../gcc-$gcc_version/configure --prefix=$root_dir/tools/gcc --enable-languages=c,c++  --disable-nls --disable-lto || exit 1

might as well leave lto on. Note there are flags we use in the rpm spec that would need to be taken here (don't remember exactly which ones off hand).

Comment 10

[Trevor Saunders (:tbsaunde)]

> > +
> > +root_dir=$(mktemp -d)
> > +cd $root_dir
> > +
> > +wget ftp://ftp.gnu.org/gnu/binutils/binutils-$binutils_version.tar.bz2 || exit 1
> 
> might be worth keeping the tarball around somewhere (in $TMPDIR?) for
> subsequent attempts (not requiring to re-download).

shouldn't be too hard so why not

> @@ +7,5 @@
> > +root_dir=$(mktemp -d)
> > +cd $root_dir
> > +
> > +wget ftp://ftp.gnu.org/gnu/binutils/binutils-$binutils_version.tar.bz2 || exit 1
> > +tar xf binutils-$binutils_version.tar.bz2
> 
> tar jxf

just to be explicit, or does that actually do something useful?

> @@ +22,5 @@
> > +
> > +./contrib/download_prerequisites
> > +
> > +# gcc 4.7 doesn't dump a stack on ICE so hack that in
> > +git apply $gcc_bt_patch || exit 1
> 
> Use patch instead of git.

yeah, that really is the right thing to do (I used git apply because I can never remember the args patch wants)

> @@ +27,5 @@
> > +
> > +cd ..
> > +mkdir gcc-objdir
> > +cd gcc-objdir
> > +../gcc-$gcc_version/configure --prefix=$root_dir/tools/gcc --enable-languages=c,c++  --disable-nls --disable-lto || exit 1
> 
> might as well leave lto on. Note there are flags we use in the rpm spec that
> would need to be taken here (don't remember exactly which ones off hand).

yeah, I guess we don't need to optimize for size that much.

Comment 11

[Mike Hommey [:glandium]]

(In reply to Trevor Saunders (:tbsaunde) from comment #10)
> > tar jxf
> 
> just to be explicit, or does that actually do something useful?

tar xf is not going to do any decompression. It needs to be tar axf or jxf.

Comment 12

[Trevor Saunders (:tbsaunde)]

> tar xf is not going to do any decompression. It needs to be tar axf or jxf.

hm, tar xf foo.tar.bz2 certainly works for me, as in results in it produces a directory containing the contents of the tarball.

Comment 13

[Trevor Saunders (:tbsaunde)]

Attachment: bug 913442 - rewrite build-gcc.py

I checked that this passes the same args to gcc's configure as the spec file other than the --with-<lib> which aren't needed in this setup

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 803549 [details] [diff] [review]
bug 913442 - rewrite build-gcc.py

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

::: build/unix/build-gcc/build-gcc.sh
@@ +16,5 @@
> +  ln -s $TMPDIR/binutils-$binutils_version.tar.bz2 .
> +else
> +  wget ftp://ftp.gnu.org/gnu/binutils/binutils-$binutils_version.tar.bz2 || exit 1
> +  cp binutils-$binutils_version.tar.bz2 $TMPDIR
> +fi

how about: 
wget -c -P $TMPDIR ftp://ftp.gnu.org/gnu/binutils/binutils-$binutils_version.tar.bz2 || exit 1
if test -O $TMPDIR/binutils-$binutils_version.tar.bz2; then
  ln -s $TMPDIR/binutils-$binutils_version.tar.bz2 .
fi

(and in fact, the symlink is probably not necessary at all)

@@ +49,5 @@
> +make $make_flags || exit 1
> +make $make_flags install || exit 1
> +
> +cd $root_dir/tools
> +tar caf $root_dir/gcc.tar.xz gcc/

It might be better to use --prefix=/tools/gcc and then make install DESTDIR=$root_dir for both. That'd avoid having the temporary directory name in the final binaries. That being said, it would be even better if we built gcc/binutils as relocatable, except i have no idea how to. Maybe Nathan does.

Comment 15

[Trevor Saunders (:tbsaunde)]

Attachment: bug 913442 - rewrite build-gcc.py with comments fixed

this seems to work so unless someone has more comments I'll check it in and ask for a tarball to be uploaded soon maybe tomorrow?

Comment 16

[Nathan Froyd [:froydnj]]

Comment on attachment 807462 [details] [diff] [review]
bug 913442 - rewrite build-gcc.py with comments fixed

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

::: build/unix/build-gcc/build-gcc.sh
@@ +15,5 @@
> +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
> +mkdir binutils-objdir
> +cd binutils-objdir
> +../binutils-$binutils_version/configure --prefix /tools/gcc/ --enable-gold  --enable-plugins --disable-nls || exit 1

You may want to stick the GCC version in the prefix, somehow?  I guess the intent here is for tooltool packages, so maybe it doesn't matter that much.

@@ +27,5 @@
> +
> +./contrib/download_prerequisites
> +
> +# gcc 4.7 doesn't dump a stack on ICE so hack that in
> +patch -p1 < $gcc_bt_patch || exit 1

I agree stacks are useful; I don't know why our GCC needs to provide stacks.  Reasoning here?

@@ +32,5 @@
> +
> +cd ..
> +mkdir gcc-objdir
> +cd gcc-objdir
> +../gcc-$gcc_version/configure --prefix=/tools/gcc --enable-languages=c,c++  --disable-nls --disable-gnu-unique-object --enable-__cxa_atexit || exit 1

These configure options should be as close as possible to the ones used in the RPM spec file.  (Maybe they are, haven't looked, just putting in two cents.)  --with-arch-32=i686 or somesuch is probably a good idea.

Any reason you're not statically linking against gmp/mpfr/gmp here, compiling new packages for those too?  --without-system-zlib might also be a good idea; just trying to minimize dependencies so these binaries work on a wide range of systems.

I think specifying the same prefix for binutils and gcc will force gcc to use the newly installed binutils, but you may want to pass an explicit --with-{as,ld} just to make sure.  Explicit is better than implicit, yadda yadda.

Comment 17

[Trevor Saunders (:tbsaunde)]

(In reply to Nathan Froyd (:froydnj) (AFK 16-20 September) from comment #16)
> Comment on attachment 807462 [details] [diff] [review]
> bug 913442 - rewrite build-gcc.py with comments fixed
> 
> Review of attachment 807462 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: build/unix/build-gcc/build-gcc.sh
> @@ +15,5 @@
> > +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
> > +mkdir binutils-objdir
> > +cd binutils-objdir
> > +../binutils-$binutils_version/configure --prefix /tools/gcc/ --enable-gold  --enable-plugins --disable-nls || exit 1
> 
> You may want to stick the GCC version in the prefix, somehow?  I guess the
> intent here is for tooltool packages, so maybe it doesn't matter that much.

yeah, for tooltool packages I think it will just cause us grief by needing to take care of it in setup.sh

> @@ +27,5 @@
> > +
> > +./contrib/download_prerequisites
> > +
> > +# gcc 4.7 doesn't dump a stack on ICE so hack that in
> > +patch -p1 < $gcc_bt_patch || exit 1
> 
> I agree stacks are useful; I don't know why our GCC needs to provide stacks.
> Reasoning here?

We seem to  still see some ICEs in gcc itself see bug 910105 its pretty rare, but is there a reason to not do this given its pretty trivial?

> @@ +32,5 @@
> > +
> > +cd ..
> > +mkdir gcc-objdir
> > +cd gcc-objdir
> > +../gcc-$gcc_version/configure --prefix=/tools/gcc --enable-languages=c,c++  --disable-nls --disable-gnu-unique-object --enable-__cxa_atexit || exit 1
> 
> These configure options should be as close as possible to the ones used in

They are

> the RPM spec file.  (Maybe they are, haven't looked, just putting in two
> cents.)  --with-arch-32=i686 or somesuch is probably a good idea.

we pass -march=pentiumpro so I guess that would make the most sense?

> Any reason you're not statically linking against gmp/mpfr/gmp here,
> compiling new packages for those too?  --without-system-zlib might also be a
> good idea; just trying to minimize dependencies so these binaries work on a
> wide range of systems.

I'm pretty sure when you dump gmp / mpfr / mpc in the gcc tree gcc's build system magically does that for you, when I ldd  cc1 / c11plus from this package they don't link against any of them anyway.

> I think specifying the same prefix for binutils and gcc will force gcc to
> use the newly installed binutils, but you may want to pass an explicit
> --with-{as,ld} just to make sure.  Explicit is better than implicit, yadda
> yadda.

or we could just add $TOPSRCDIR/gcc to $PATH in the mozconfigs which would be explicit in a place more people will look.

Comment 18

[Trevor Saunders (:tbsaunde)]

landed the script as https://hg.mozilla.org/integration/mozilla-inbound/rev/6dccf6a79376 I'll ask relng to upload the resulting binary to tooltool tomorrow.

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/6dccf6a79376

Comment 20

[Trevor Saunders (:tbsaunde)]

Armenzg could you please upload people.mozilla.org/~tsaunders/gcc.tar.xz and people.mozilla.org/~tsaunders/setup.sh.gcc to tooltool?

Comment 21

[Armen [:armenzg]]

OK.

Comment 22

[Armen [:armenzg]]

Done:
[[e-mail]3 ~]$ ls -l  /mnt/netapp/relengweb/tooltool/pvt/build/sha512/${SHA512}
-rw-r--r-- 1 armenzg armenzg 79726288 Nov  1 11:06 /mnt/netapp/relengweb/tooltool/pvt/build/sha512/fbbb2f0d63d8681d834f7ae0dca88e3f1f22bca4d6cfb6596243c565904485430022161f93f418cb98dfec389c42b9d22b1268c908e72d09a52c34ecfc537311

Comment 23

[Nick Thomas [:nthomas] (UTC+12)]

Added the missing setup.sh.gcc file:

$ ls -l  /mnt/netapp/relengweb/tooltool/pvt/build/sha512/${SHA512}
-rw-r--r-- 1 nthomas nthomas 40 Nov  1 09:07 /mnt/netapp/relengweb/tooltool/pvt/build/sha512/459b332864aece4742cd1a6886e56cf3f202e5c27bb481cfae6145ce3e2e52fb34d1448788c6618e58a26a64e415341895326d293e0d2968e56efc0ae990acd0

Comment 24

[Mike Hommey [:glandium]]

According to comment 22 and comment 23, this is fixed.