Bug 33466

For the record, this is
<http://lists.freedesktop.org/archives/dbus/2010-December/013847.html>.

Created attachment 42822 [details] [review]
Allow --gc-sections to be disabled, again to work around broken toolchains

As with Bug #16621, I've redone the patch to make it explicit that this is a workaround for broken toolchains.

Created attachment 43614 [details] [review]
Add support for --disable-gc-sections for broken toolchains (v2)

Also use AC_LINK_IFELSE rather than reinventing it as a shell function.
This was the last user of ld_supports_flag, so, delete it.
    
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=33466

(As seen here, this depends on the first of the current patches on Bug #16621 - it could be disentangled if necessary)

(In reply to comment #3)
> Created an attachment (id=43614) [details]
> Add support for --disable-gc-sections for broken toolchains (v2)

Looks OK, though I find myself wondering whether Havoc or whoever did the change originally benchmarked it; the GCC manual says:

     Only use these options when there are significant benefits from
     doing so.  When you specify these options, the assembler and
     linker will create larger object and executable files and will
     also be slower.  You will not be able to use `gprof' on all
     systems if you specify this option and you may have problems with
     debugging if you specify both this option and `-g'.

(In reply to comment #4)
> Looks OK

Fixed in git for 1.4.10, 1.5.2

> though I find myself wondering whether Havoc or whoever did the
> change originally benchmarked it

Bug retitled and left open

The version of GNU ld currently in Debian unstable seems be one of these broken toolchains - it segfaults while linking dbus-daemon on mips, mipsel, armel. Yay, rarely-used options...

Using current Debian unstable to build dbus-1.4 on x86-64, I get:

                bin    lib   libexec
gc              428K   1.2M  48K
no gc           480K   924K  292K
no gc, ld -O1   484K   928K  292K
no gc, cc -Os   424K   848K  252K

(All with --disable-tests, --disable-verbose-mode, make install-strip, sizes measured with du -sh.)

The size increase in lib with --gc-sections is mostly the static library.

The size increase in bin without --gc-sections is mostly dbus-daemon (the ancillary tools remain small, because they do so little already).

The only thing in libexec is the dbus-daemon-launch-helper. Its size goes down dramatically with --gc-sections, but it's so small anyway that I'm not convinced I care enough to keep support for --gc-sections in configure.ac.

People who particularly care about size optimization should probably experiment with different flags themselves anyway, and if armel and mips are among the platforms where --gc-sections doesn't work, I think that demonstrates that embedded users who use general-purpose toolchains don't really care that much about ±300K.

I note with amusement that our dbus-daemon and dbus-daemon-launch-helper are probably only as fat as they are because they (effectively) statically link libdbus; people who really care about size optimizations should probably look into linking them dynamically, although that'd require some combination of moving a lot of libdbus' private API to be public and/or moving some of the tools' functionality to be public API in libdbus.

Thanks for posting numbers!

As far as dbus-daemon duplicating libdbus; I think the most interesting thing to do would be to try http://gcc.gnu.org/wiki/LinkTimeOptimization

(Apparently gcc has a hacky version of this called "-combine" we could use right now too, though the disadvantage is it doesn't support incremental rebuilds, so it has to be a "packaging" option)

(In reply to comment #7)
> As far as dbus-daemon duplicating libdbus; I think the most interesting thing
> to do would be to try http://gcc.gnu.org/wiki/LinkTimeOptimization

If you want to try it you're welcome to do so, but I'm not planning to.

I've written two alternative patches for this: one changes the default to not use -Wl,--gc-sections, and the other removes that feature completely.

Rationale for leaving --enable-gc-sections in: we currently only use it in bus/ where it's likely to be most useful; if we took it out completely, the only way to enable it would be to put it in LDFLAGS at configure time, which would mean it affected the entire tree.

Rationale for not: like the "hardening" flags on Bug #16621, this is the sort of obscure option where the maintainers of a particular toolchain/arch/OS combination are likely to know the advantages and disadvantages much better than we do - and if they don't, the right answer is probably "don't use it".

I'm currently leaning towards removing it completely, but I could be convinced either way.

Created attachment 47591 [details] [review]
[patch] Remove support for -Wl,--gc-sections altogether

Packagers should only enable this flag if they have confirmed that it
actually works on their toolchain (it's the sort of rarely used feature
that frequently regresses on obscure architectures/OSs without anyone
noticing), and also confirmed that it is actually a significant size win
for their configuration.

Created attachment 47592 [details] [review]
[alternative patch] Turn off -ffunction-sections -fdata-sections  -Wl,--gc-sections by default

It's an obscure feature that is often broken in toolchains for less common
architectures/OSs, and the size reduction turns out not to be particularly
significant in practice, so it seems better to make it opt-in.

Removing this feature altogether would result in a less targeted application
of -Wl,--gc-sections when used: at the moment -Wl,--gc-sections is only
applied within the bus directory (the only place it's likely to be useful)
whereas if added to LDFLAGS by a packager, it would affect the entire package.

Review of attachment 47592 [details] [review]:

Looks good.

Review of attachment 47592 [details] [review]:

Looks good.

(In reply to comment #9)
> Created an attachment (id=47591) [details]
> [patch] Remove support for -Wl,--gc-sections altogether

I applied this one, based on discussion with Colin on IRC.

Fixed in git for 1.4.12, 1.5.4.