[systemd-devel] [PATCH v2] build-sys: remove --gc-sections to fix debugging

[Peter Wu]

The `--gc-sections` linker option triggers a bug in the gold linker[1]
(binutils 2.24 or older). This results in a bogus .eh_frame section
making debugging harder: gdb backtraces stop at a library built by
systemd and libunwind simply segfaults because it does not check for
garbage values.

Workaround that bug by removing `-Wl,--gc-sections`. Also remove
`-fdata-sections` and `-ffunction-sections` as suggested by Gustavo
Sverzut Barbieri. There is no benefit in creating separate sections for
each function or data item if unused sections are not removed.

The additional disk space saved by this option is marginal anyway (less
than 1%). To illustrate this, see this `du -ks` on the installed files
(where `-Wl,--gc-sections` is removed):

83548 broken-binutils-without-gc-sections/install
83432 broken-binutils-with-gc-sections/install
25796 broken-binutils-without-gc-sections/install-strip
25752 broken-binutils-with-gc-sections/install-strip

84024 binutils-master-without-gc-sections/install
83988 binutils-master-with-gc-sections/install
26384 binutils-master-without-gc-sections/install-strip
26380 binutils-master-with-gc-sections/install-strip

[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=17639

https://bugs.freedesktop.org/show_bug.cgi?id=86666
---
v2: removed -ffunction-sections and -fdata-sections too, mention
that binutils fixed the bug.
---
configure.ac | 3 ---
1 file changed, 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index bd3cc0e..baba072 100644
--- a/configure.ac
+++ b/configure.ac
@@ -191,8 +191,6 @@ CC_CHECK_FLAGS_APPEND([with_cflags], [CFLAGS], [\
-fdiagnostics-show-option \
-fno-strict-aliasing \
-fvisibility=hidden \
- -ffunction-sections \
- -fdata-sections \
-fstack-protector \
-fstack-protector-strong \
-fPIE \
@@ -219,7 +217,6 @@ AC_SUBST([OUR_CPPFLAGS], "$with_cppflags $sanitizer_cppflags")
CC_CHECK_FLAGS_APPEND([with_ldflags], [LDFLAGS], [\
-Wl,--as-needed \
-Wl,--no-undefined \
- -Wl,--gc-sections \
-Wl,-z,relro \
-Wl,-z,now \
-pie \

[Cristian Rodríguez]

Post by Peter Wu
[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=17639
https://bugs.freedesktop.org/show_bug.cgi?id=86666
---
v2: removed -ffunction-sections and -fdata-sections too, mention
that binutils fixed the bug.
---

-1 Just fix the linker and move along... not our bug IMHO.

[Zbigniew Jędrzejewski-Szmek]

Post by Cristian Rodríguez

Post by Peter Wu
[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=17639
https://bugs.freedesktop.org/show_bug.cgi?id=86666
---
v2: removed -ffunction-sections and -fdata-sections too, mention
that binutils fixed the bug.
---

-1 Just fix the linker and move along... not our bug IMHO.

Hm, with this patch the build fails for me with

../src/shared/ptyfwd.c:117: error: undefined reference to 'sd_event_source_unref'
../src/shared/ptyfwd.c:120: error: undefined reference to 'sd_event_exit'
../src/shared/ptyfwd.c:127: error: undefined reference to 'sd_event_source_unref'
../src/shared/ptyfwd.c:134: error: undefined reference to 'sd_event_exit'
../src/shared/ptyfwd.c:151: error: undefined reference to 'sd_event_source_unref'
../src/shared/ptyfwd.c:154: error: undefined reference to 'sd_event_exit'
../src/shared/ptyfwd.c:180: error: undefined reference to 'sd_event_source_unref'
../src/shared/ptyfwd.c:183: error: undefined reference to 'sd_event_exit'
../src/shared/ptyfwd.c:374: error: undefined reference to 'sd_event_unref'
../src/shared/ptyfwd.c:298: error: undefined reference to 'sd_event_ref'
../src/shared/ptyfwd.c:300: error: undefined reference to 'sd_event_default'
../src/shared/ptyfwd.c:345: error: undefined reference to 'sd_event_add_io'
../src/shared/ptyfwd.c:349: error: undefined reference to 'sd_event_add_io'
../src/shared/ptyfwd.c:353: error: undefined reference to 'sd_event_add_io'
../src/shared/ptyfwd.c:360: error: undefined reference to 'sd_event_add_signal'
../src/shared/pty.c:436: error: undefined reference to 'sd_event_source_set_io_events'
../src/shared/pty.c:468: error: undefined reference to 'sd_event_add_io'
../src/shared/pty.c:477: error: undefined reference to 'sd_event_source_set_prepare'
../src/shared/pty.c:483: error: undefined reference to 'sd_event_add_child'
collect2: error: ld returned 1 exit status
Makefile:10333: recipe for target 'login.la' failed

(and many repeats for other libraries).

$ rpm -q gcc binutils
gcc-4.9.2-1.fc21.x86_64
binutils-2.24-21.fc21.x86_64

../configure CFLAGS='-g -Og -ftrapv' CPPFLAGS=-DVALDGRIND=1 --disable-compat-libs --disable-kdbus --sysconfdir=/etc --localstatedir=/var --libdir=/usr/lib64 --enable-lz4 --enable-xz --enable-terminal

I think I'm with Christian here... let sleeping dogs lie.

Zbyszek

[Peter Wu]

Post by Zbigniew Jędrzejewski-Szmek

Post by Cristian Rodríguez

Post by Peter Wu
[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=17639
https://bugs.freedesktop.org/show_bug.cgi?id=86666
---
v2: removed -ffunction-sections and -fdata-sections too, mention
that binutils fixed the bug.
---

-1 Just fix the linker and move along... not our bug IMHO.

Now that this bug is fixed, there is less need for this patch, but the
options themselves do not seem to bring great benefit so might as well
remove it. See also below.

Post by Zbigniew Jędrzejewski-Szmek
Hm, with this patch the build fails for me with
../src/shared/ptyfwd.c:117: error: undefined reference to 'sd_event_source_unref'
../src/shared/ptyfwd.c:120: error: undefined reference to 'sd_event_exit'

[..]

Post by Zbigniew Jędrzejewski-Szmek
(and many repeats for other libraries).
$ rpm -q gcc binutils
gcc-4.9.2-1.fc21.x86_64
binutils-2.24-21.fc21.x86_64
../configure CFLAGS='-g -Og -ftrapv' CPPFLAGS=-DVALDGRIND=1 --disable-compat-libs --disable-kdbus --sysconfdir=/etc --localstatedir=/var --libdir=/usr/lib64 --enable-lz4 --enable-xz --enable-terminal

Reproduced. I built with --enable-compat-libs. Isn't a real bug being
hidden by the -Wl,--gc-sections option? id128.so uses the
sd_event_source_unref symbol which is provided by libsystemd-internal,
but id128 only links to libsystemd-shared. When linking with
-Wl,--gc-sections, I get:

$ readelf -s .libs/id128.so | grep sd_event_source_unref
1913: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND sd_event_source_unref

Post by Zbigniew Jędrzejewski-Szmek
I think I'm with Christian here... let sleeping dogs lie.

Now that a bug is released in binutils (I did not expect such a quick
response, kudos to them!), I can live with this patch not being applied.
But the reasoning is wrong, something was actually broken and you cannot
just ignore that.

As an aside, none of the files in /usr/{lib,bin} on my system were
compiled with this fatal combination of flags, except for systemd. It is
apparently not a common option.

I was also thinking about other adopters of systemd, like Debian. They
just disable gold[1] which would overcome the issue without this patch.
Though if they upgrade systemd in the future they will either have to
disable compat libs or otherwise run in the problem described at
https://bugs.freedesktop.org/show_bug.cgi?id=86666. Hmm, strange folks,
they use --enable-compat-libs but then remove[2] the libraries...?

[Michael Biebl]

Post by Peter Wu
https://bugs.freedesktop.org/show_bug.cgi?id=86666. Hmm, strange folks,
they use --enable-compat-libs but then remove[2] the libraries...?

We don't remove the libraries, we remove .so symlinks, since they are useless.

[Peter Wu]

Post by Michael Biebl

Post by Peter Wu
https://bugs.freedesktop.org/show_bug.cgi?id=86666. Hmm, strange folks,
they use --enable-compat-libs but then remove[2] the libraries...?

We don't remove the libraries, we remove .so symlinks, since they are useless.

Ah, to prevent building with -lsystemd-X while allowing existing
programs to run I guess.

On topic, I think that --gc-section hides those errors as the code do
not reference directly and --gc-sections strip them before it get to
linking. This is the case for python-systemd. No other part of the code
uses id128.so, _daemon.so, _journal.so, login.so or _reader.so.

With --without-python --disable-compat-libs, everything still compiles.
It could likely be fixed for --with-python by adding systemd-shared to
id128_la_LIBADD (etc.)?