1032055 - Build failure with -flto

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Octoploid]

With gcc-4.9 and gold I get with -flto:
...
test "$(nm -g libxul.so | grep _NSModule$ | sort | sed -n 's/^.* _*\([^ ]*\)$/\1/;1p;$p' | xargs echo)" != "start_kPStaticModules_NSModule end_kPStaticModules_NSModule" && echo "NSModules are not ordered appropriately" && exit 1 || exit 0 ; test "$(readelf -l libxul.so | awk 'libxul.so == "LOAD" { t += 1 } END { print t }')" -le 1 && echo "Only one PT_LOAD segment" && exit 1 || exit 0
NSModules are not ordered appropriately

markus@x4 build % nm -g libxul.so | grep _NSModule
00000000034e9720 D application_NSModule
00000000034e9748 D Apprunner_NSModule
00000000034e9700 D appshell_NSModule
00000000034e9730 D BOOT_NSModule
00000000034e96f8 D Browser_Embedding_Module_NSModule
00000000034e9760 D CommandLineModule_NSModule
00000000034e9768 D DiskSpaceWatcherModule_NSModule
00000000034e96e8 D docshell_provider_NSModule
00000000034e96f0 D embedcomponents_NSModule
00000000034e9628 D end_kPStaticModules_NSModule
00000000034e97c0 D identity_NSModule
00000000034e97a0 D jsctypes_NSModule
00000000034e97e8 D jsdebugger_NSModule
00000000034e97b8 D jsinspector_NSModule
00000000034e9780 D jsperf_NSModule
00000000034e9790 D jsreflect_NSModule
00000000034e9728 D mozSpellCheckerModule_NSModule
00000000034e9670 D mozStorageModule_NSModule
00000000034e9648 D necko_NSModule
00000000034e9650 D nsAuthModule_NSModule
00000000034e97d0 D nsAutoConfigModule_NSModule
00000000034e9658 D nsChardetModule_NSModule
00000000034e96d8 D nsComposerModule_NSModule
00000000034e96c8 D nsContentProcessWidgetModule_NSModule
00000000034e9678 D nsCookieModule_NSModule
00000000034e9770 D nsFileViewModule_NSModule
00000000034e9698 D nsGfxModule_NSModule
00000000034e97f0 D nsGIOModule_NSModule
00000000034e9640 D nsI18nModule_NSModule
00000000034e96a8 D nsIconDecoderModule_NSModule
00000000034e96a0 D nsImageLib2Module_NSModule
00000000034e9660 D nsJarModule_NSModule
00000000034e96e0 D nsLayoutModule_NSModule
00000000034e9778 D nsMediaSnifferModule_NSModule
00000000034e9690 D nsParserModule_NSModule
00000000034e9680 D nsPermissionsModule_NSModule
00000000034e9788 D nsPlacesModule_NSModule
00000000034e96b8 D nsPluginModule_NSModule
00000000034e9630 D nsPrefModule_NSModule
00000000034e9710 D nsProfilerModule_NSModule
00000000034e9688 D nsRDFModule_NSModule
00000000034e97d8 D nsServicesCryptoModule_NSModule
00000000034e9738 D NSS_NSModule
00000000034e9798 D nsTelemetryModule_NSModule
00000000034e9750 D nsToolkitCompsModule_NSModule
00000000034e96d0 D nsTransactionManagerModule_NSModule
00000000034e9638 D nsUConvModule_NSModule
00000000034e9708 D nsUniversalCharDetModule_NSModule
00000000034e97c8 D nsUnixProxyModule_NSModule
00000000034e96c0 D nsWidgetGtk2Module_NSModule
00000000034e9718 D nsWindowDataSourceModule_NSModule
00000000034e96b0 D peerconnection_NSModule
00000000034e9740 D PKI_NSModule
00000000034e9758 D RemoteServiceModule_NSModule
00000000034e97b0 D satchel_NSModule
00000000034e9620 D start_kPStaticModules_NSModule
00000000034e97e0 D StartupCacheModule_NSModule
00000000034e97a8 D tkAutoCompleteModule_NSModule
00000000034e9668 D ZipWriterModule_NSModule
markus@x4 build % nm -g libxul.so | grep _NSModule$ | sort | sed -n 's/^.* _*\([^ ]*\)$/\1/;1p;$p'
start_kPStaticModules_NSModule
nsGIOModule_NSModule

Comment 1

[Mike Hommey [:glandium]]

You'll have to provide more info as to how you're building, because if I try to build with -fto, I have failures way before reaching libxul.

Comment 2

[Octoploid]

For gcc-4.9 you must use the gcc-ar, etc. wrappers, because we use slim-lto-objects by default now.
See: https://gcc.gnu.org/wiki/LinkTimeOptimizationFAQ#ar.2C_nm_and_ranlib

With this in place, just adding "-flto" to the CFLAGS and CXXFLAGS should be enough to reproduce.

Comment 3

[Trevor Saunders (:tbsaunde)]

(In reply to Mike Hommey [:glandium] from comment #1)
> You'll have to provide more info as to how you're building, because if I try
> to build with -fto, I have failures way before reaching libxul.


and the build finishes for me with wrappers around ar/nm/ranlib, and disabling elf hack (one of its tests fails)

Comment 4

[Octoploid]

(In reply to Trevor Saunders (:tbsaunde) from comment #3)
> (In reply to Mike Hommey [:glandium] from comment #1)
> > You'll have to provide more info as to how you're building, because if I try
> > to build with -fto, I have failures way before reaching libxul.
> 
> 
> and the build finishes for me with wrappers around ar/nm/ranlib, and
> disabling elf hack (one of its tests fails)

OK. Time to post my .mozconfig:

. $topsrcdir/browser/config/mozconfig

ac_add_options --prefix=/usr
ac_add_options --libdir=/usr/lib

ac_add_options --with-system-jpeg --with-pthreads --with-system-zlib
ac_add_options --with-system-bz2 --with-system-hunspell
ac_add_options --with-system-libevent
ac_add_options --with-system-lcms 
ac_add_options --with-system-png
ac_add_options --with-system-icu
ac_add_options --with-system-libvpx
ac_add_options --with-system-ffmpeg
ac_add_options --enable-system-ffi
ac_add_options --enable-system-pixman 
ac_add_options --enable-system-sqlite
ac_add_options --disable-gstreamer --disable-content-sandbox

ac_add_options --disable-installer --disable-updater
ac_add_options --enable-official-branding
ac_add_options --enable-startup-notification
ac_add_options --disable-necko-wifi --disable-accessibility
ac_add_options --disable-elf-hack
ac_add_options --disable-sync
ac_add_options --disable-pulseaudio 

ac_add_options --disable-jemalloc
ac_add_options --enable-optimize=-O3 --disable-debug --disable-tests
ac_add_options --disable-debug-symbols
ac_add_options --disable-strip --disable-install-strip

ac_add_options --enable-default-toolkit=cairo-gtk2 --enable-system-cairo

ac_add_options --disable-crashreporter --disable-parental-controls
ac_add_options --disable-safe-browsing

mk_add_options MOZ_OBJDIR=/var/tmp/moz-build-dir
mk_add_options MOZ_MAKE_FLAGS="-j4"

export CFLAGS="-flto=4 "
export CXXFLAGS="-flto=4 "

export LDFLAGS="-Wl,--hash-style=gnu,--as-needed,--gc-sections,--icf=all,--icf-iterations=3"

export MOZ_DEBUG_SYMBOLS=0

Comment 5

[Octoploid]

Also happens when I comment out the LDFLAGS export.

Comment 6

[John Schoenick [:johns]]

Possibly related, my clang LTO builds broke last week as well, with a ld.gold error:
> error: read-only segment has dynamic relocations

I haven't tried to bisect this yet, since the linker takes 20+GiB of ram and the better part of an hour, but I can try if it would be useful.

Comment 7

[Mike Hommey [:glandium]]

> ac_add_options --with-system-icu

I guess that's how you get away with the errors I've got.

Comment 8

[Trevor Saunders (:tbsaunde)]

(In reply to Mike Hommey [:glandium] from comment #7)
> > ac_add_options --with-system-icu
> 
> I guess that's how you get away with the errors I've got.

I'd guess not, my sucessful build in comment 3 was CFLAGS=-flto=12 CXXFLAGS=-flto=12 LDFLAGS=-flto=12 configure --disable-gstreamer --disable-pulseaudio --disable-elf-hack with gcc debian 4.9.0-9

Comment 9

[Octoploid]

 % ld -v
GNU gold (GNU Binutils 2.24.51.20140508) 1.11

Comment 10

[Trevor Saunders (:tbsaunde)]

errr, I spoke too soon, I was using a tree that was older than I thought, building with inbound from this morning breaks for in the same way.

Comment 11

[Octoploid]

With clang (trunk) I get:
x86_64-pc-linux-gnu/bin/ld: error: read-only segment has dynamic relocations
clang-3.5: error: linker command failed with exit code 1 (use -v to see invocation)
/var/tmp/mozilla-central/config/rules.mk:867: recipe for target 'libxul.so' failed
make[5]: *** [libxul.so] Error 1

No issues with Mike's patch reverted.

Comment 12

[John Schoenick [:johns]]

Bug 938437 is the culprit for clang lto bustage as well. Building with clang 3.4.2 and mozconfig:

> export MOZ_DEBUG_SYMBOLS=1
> ac_add_options --enable-debug-symbols="-ggdb"
>
> export MOZ_PSEUDO_DERECURSE=1
> 
> ac_add_options --enable-application=browser
> ac_add_options --enable-optimize
> ac_add_options --disable-debug
> ac_add_options --enable-tests
> ac_add_options --enable-profiling
> ac_add_options --disable-install-strip
>
> # -march=native is a workaround for "LLVM ERROR: Cannot select: intrinsic", due
> # to the linker not being aware of available cpu features.
> # See: http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-February/070047.html
> 
> ac_add_options --enable-optimize=-O3
> export LDFLAGS="$LDFLAGS -flto -march=native"
> export CFLAGS="$CFLAGS -flto"
> export CXXFLAGS="$CXXFLAGS -flto"
> export CC="clang"
> export CXX="clang++"
> export CFLAGS="$CFLAGS -fcolor-diagnostics -Wno-inline-new-delete"
> export CXXFLAGS="$CXXFLAGS -fcolor-diagnostics -Wno-inline-new-delete"

Comment 13

[Trevor Saunders (:tbsaunde)]

(In reply to Octoploid from comment #11)
> With clang (trunk) I get:
> x86_64-pc-linux-gnu/bin/ld: error: read-only segment has dynamic relocations
> clang-3.5: error: linker command failed with exit code 1 (use -v to see
> invocation)
> /var/tmp/mozilla-central/config/rules.mk:867: recipe for target 'libxul.so'
> failed
> make[5]: *** [libxul.so] Error 1
> 
> No issues with Mike's patch reverted.

yeah, not entirely suprising, I bet clang is also deciding that since it blew away the idea of translation units it doesn't have to care about preserving the order of global objects between .o files.  And that totally breaks Mike's code which assumes we can stick a bunch of stuff in a section and then rely on order of things passed to the link command to determin the order of things in the library.  Would you mind seeing -fno-toplevel-reorder 'fixes" things with gcc, and clang if it has that option.

Comment 14

[Octoploid]

For gcc Marxin has already tested a -fno-toplevel-reorder LTO build and it doesn't work.
He's getting link errors due to a GCC extension that allows label addresses to be streamed to different LTRANS files, e.g.:
 /tmp/ccvWENvK.ltrans9.ltrans.o:ccvWENvK.ltrans9.o:_ZZL9InterpretP9JSContextRN2js8RunStateEE9addresses.lto_priv.65093: error: undefined reference to '.L261'
I don't think clang supports that option.

Comment 15

[Trevor Saunders (:tbsaunde)]

(In reply to Octoploid from comment #14)
> For gcc Marxin has already tested a -fno-toplevel-reorder LTO build and it
> doesn't work.
> He's getting link errors due to a GCC extension that allows label addresses
> to be streamed to different LTRANS files, e.g.:
>  /tmp/ccvWENvK.ltrans9.ltrans.o:ccvWENvK.ltrans9.o:
> _ZZL9InterpretP9JSContextRN2js8RunStateEE9addresses.lto_priv.65093: error:
> undefined reference to '.L261'
> I don't think clang supports that option.

Then I guess somebody really needs to get the linker script we use with bfd ld to work with gold. Which means working around http://mxr.mozilla.org/mozilla-central/source/toolkit/library/libxul.mk#254 which sounds like loads of fun.

Comment 16

[[e-mail]]

Don't build StaticXULComponentsStart.cpp StaticXULComponentsEnd.cpp with lto.

Comment 17

[Trevor Saunders (:tbsaunde)]

Attachment: lto.patch

I've been meaning to try this for a while, but it turns out not LTOing the start and end files is enough to make things work.

Comment 18

[Octoploid]

Comment on attachment 8470193 [details] [diff] [review]
lto.patch

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

There's a + missing in toolkit/library/moz.build, it should read:
+SOURCES['StaticXULComponentsStart.cpp'].flags += ['-fno-lto']

Comment 19

[Octoploid]

The patch is working fine for me. I've tested gcc-4.9 and gcc-5 LTO builds.

Comment 20

[John Schoenick [:johns]]

The patch also fixes clang-3.4 LTO

Comment 21

[Mike Hommey [:glandium]]

Comment on attachment 8470193 [details] [diff] [review]
lto.patch

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

It would be better to have a generic way to exclude flags, but meh, that can wait.

Comment 22

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3ad9f60f1972

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Backed out for B2G ICS bustage. Presumably GCC 4.4 issues?
https://hg.mozilla.org/integration/mozilla-inbound/rev/da9147abd7b7

https://tbpl.mozilla.org/php/getParsedLog.php?id=46086684&tree=Mozilla-Inbound

Comment 24

[Trevor Saunders (:tbsaunde)]

Attachment: prevent lto from incorrectly reordering component constants

this is a pretty hacky way of avoiding the issue with b2g builds, but it works and I'm not sure we really need to export compiler version to moz.build today.

Comment 25

[Mike Hommey [:glandium]]

Comment on attachment 8478671 [details] [diff] [review]
prevent lto from incorrectly reordering component constants

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

::: toolkit/library/StaticXULComponentsEnd/moz.build
@@ +8,5 @@
>  
> +# XXX gcc 4.4 doesn't like -fno-lto, and we don't have an easy way to check gcc
> +# version here, so just check for b2g.
> +if not CONFIG['MOZ_B2G']:
> +  SOURCES['StaticXULComponentsEnd.cpp'].flags += ['-fno-lto']

How about making '-flto' in CONFIG['OS_CXXFLAGS'] the condition?

Comment 26

[Trevor Saunders (:tbsaunde)]

Attachment: lto.patch

its a little more than that, but we actually can do that you need to care about -flto=n, and OS_CXXFLAGS is a string so you need to split it up.

Comment 27

[Mike Hommey [:glandium]]

Comment on attachment 8479323 [details] [diff] [review]
lto.patch

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

::: toolkit/library/StaticXULComponentsEnd/moz.build
@@ +12,5 @@
> +for flag in CONFIG['OS_CXXFLAGS'].split():
> +    if flag.startswith('-flto'):
> +        lto = True
> +
> +if lto:

That can be written:
if any(f.startswith('-flto') for f in CONFIG['OS_CXXFLAGS'].split()):

although, in practice, since CONFIG['OS_CXXFLAGS'] is a string, '-flto' in CONFIG['OS_CXXFLAGS'] is a good approximation (although it would trigger when OS_CXXFLAGS is something like "foo-fltobar", but how likely is that?)

Comment 28

[Trevor Saunders (:tbsaunde)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9de6514b994

Comment 29

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b9de6514b994