Bug 1067245

Our systemd 209 upload blocks on this btw.

Created attachment 865343 [details]
enable ifunc

gcc isn't enabling ifunc support by default on armv7hl or aarch64, since it's not set in gcc/config.gcc, and we're not passing a flag to configure in the spec file to do it. The attached patch should fix that.

Sounds easier to --enable-gnu-indirect-function in the .spec file for the selected architectures (also ppc/ppc64 don't enable it by default).

Note that e.g. on ppc/ppc64 the ifunc support in binutils is very new, e.g. in glibc around 2013-12, so not sure if it is even in RHEL7 or latest Fedora.

Anyway, requiring ifunc in systemd is way too premature, most architectures don't implement ifunc and that is hardly going to change any time soon.
You can surely use it if available, but requiring it?  Eh.

(In reply to Jakub Jelinek from comment #3)
> Anyway, requiring ifunc in systemd is way too premature, most architectures
> don't implement ifunc and that is hardly going to change any time soon.
> You can surely use it if available, but requiring it?  Eh.

It's only for compat libs until stuff is re-compiled and the compat stuff goes
away. Pkgconfig files are changed, a simple re-build should switch over to the
new lib.

Any better idea/way than ifunc to wrap the symbols of a lib with a new name
in a stub .so with the old name?

If it is in the same lib, just __attribute__((alias ("..."))), or just have wrappers like:
type old_symbol (arguments)
{
  return new_symbol (arguments);
}
if not in the same library or alias can't be used.  If it is a compat temporary thing, it is not worth optimizing, right?

The new lib is separate, libsystemd.so. The old libs are like:
  libsystemd-daemon.so
  libsystemd-journal.so
  ...

Right, no need to optimize the use of the old libs, no. A re-compilation
will find only the new lib anyway.

The symbols names are identical in the old and new lib. Both, the old and the
new lib, have symbol versions though, the new lib all only the new version.

(In reply to Jakub Jelinek from comment #5)
> If it is in the same lib, just __attribute__((alias ("..."))), or just have
> wrappers like:
> type old_symbol (arguments)
> {
>   return new_symbol (arguments);
> }
We have hundreds of symbols, and we'd rather not have to write wrappers for
each one. With ifunc's, they could be generated from a list of symbols by a simple macro...

(In reply to Jakub Jelinek from comment #5)
> If it is in the same lib, just __attribute__((alias ("..."))), or just have
> wrappers like:
> type old_symbol (arguments)
> {
>   return new_symbol (arguments);
> }
> if not in the same library or alias can't be used.  If it is a compat
> temporary thing, it is not worth optimizing, right?

So, this doesn't really work for us since there are quite a number of varargs involved, and we do not supply alternatives that take va_list as alternatives for them (on purpose in many cases). Thus, doing manual invocations is nasty, because we actually need to convert the varargs manually into some object before being able to invoke the real function, which is a lot of work we'd rather not do for stuff that is compat.

I've reviewed test results on ppc64/ppc/armv7hl/aarch64, and everything involved looks fine... Jakub's committed turning this on on those platforms in the next gcc build, so things should work happily going forward.

--Kyle

While this was pushed for 4.8.x it seems to have been missed in the rebase to 4.9, any chance we could get this re-enabled in 4.9?

Weird, the 4.9 gcc.spec contains:
%ifarch %{ix86} x86_64 ppc ppc64 ppc64le ppc64p7 s390 s390x %{arm} aarch64
%global attr_ifunc 1
%else
%global attr_ifunc 0
%endif
...
%if 0%{?fedora} >= 21 || 0%{?rhel} >= 7
%if %{attr_ifunc}
        --enable-gnu-indirect-function \
%endif
%endif

Well we're getting this for builds:

/tmp/cc4zjjvp.ltrans0.ltrans.o: In function `sd_booted':
cc4zjjvp.ltrans0.o:(.text+0x140): multiple definition of `sd_booted'
libsystemd_internal_la-sd-daemon.o (symbol from plugin):(.text+0x0): first defined here
/tmp/cc4zjjvp.ltrans0.ltrans.o: In function `sd_notifyf':
cc4zjjvp.ltrans0.o:(.text+0x360): multiple definition of `sd_notifyf'
libsystemd_internal_la-sd-daemon.o (symbol from plugin):(.text+0x0): first defined here
/tmp/cc4zjjvp.ltrans0.ltrans.o: In function `sd_is_mq':
cc4zjjvp.ltrans0.o:(.text+0x43c): multiple definition of `sd_is_mq'
libsystemd_internal_la-sd-daemon.o (symbol from plugin):(.text+0x0): first defined here
/tmp/cc4zjjvp.ltrans0.ltrans.o: In function `sd_is_socket_unix':
cc4zjjvp.ltrans0.o:(.text+0x554): multiple definition of `sd_is_socket_unix'
libsystemd_internal_la-sd-daemon.o (symbol from plugin):(.text+0x0): first defined here
/tmp/cc4zjjvp.ltrans0.ltrans.o: In function `sd_is_socket_inet':
cc4zjjvp.ltrans0.o:(.text+0x698): multiple definition of `sd_is_socket_inet'
libsystemd_internal_la-sd-daemon.o (symbol from plugin):(.text+0x0): first defined here
/tmp/cc4zjjvp.ltrans0.ltrans.o: In function `sd_is_special':
cc4zjjvp.ltrans0.o:(.text+0x8a8): multiple definition of `sd_is_special'
libsystemd_internal_la-sd-daemon.o (symbol from plugin):(.text+0x0): first defined here
/tmp/cc4zjjvp.ltrans0.ltrans.o: In function `sd_is_fifo':
cc4zjjvp.ltrans0.o:(.text+0x9f8): multiple definition of `sd_is_fifo'
libsystemd_internal_la-sd-daemon.o (symbol from plugin):(.text+0x0): first defined here

which I believe is indicative of not having ifunc enabled

http://arm.koji.fedoraproject.org/koji/taskinfo?taskID=2282311

That build was using the last gcc build, it was suppose to wait for the 4.9.0 final to land, retesting against that now

Much more likely just some LTO related bug, be it in the compiler, linker plugin or in the linker.  Try removing -flto?

(In reply to Jakub Jelinek from comment #14)
> Much more likely just some LTO related bug, be it in the compiler, linker
> plugin or in the linker.  Try removing -flto?

I can confirm that fixes the systemd build issues

Closing this as resolved as ifunc is fixed. The LTO issue is general and seen on x86 as well so I've opened RHBZ 1091611 to track that issue directly