| Message ID | 1470667145-18563-1-git-send-email-stli@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 08/08/2016 09:38 AM, Stefan Liebler wrote: > This patch adds a configure check to test if gcc supports attribute ifunc. > The support can either be enabled in <gcc-src>/gcc/config.gcc for one > architecture in general by setting default_gnu_indirect_function variable to yes > or by configuring gcc with --enable-gnu-indirect-function. > > The next patch rewrites libc_ifunc macro to use gcc attribute ifunc instead > of inline assembly to generate the IFUNC symbols due to false debuginfo. > > If gcc does not support attribute ifunc and glibc is configured with > --enable-multi-arch then configure will abort with an error message. > If --enable-multi-arch is not given then configure will silently > disable multi-arch support. > > ChangeLog: > > * configure.ac: Add check if gcc supports attribute ifunc feature. > * configure: Regenerated. One more hiccup after trying out this patch. pt-vfork.c mandates ifunc on most (all?) targets using nptl. It seems configure needs to mandate this support on any target using nptl.
On Mon, 8 Aug 2016, Stefan Liebler wrote: > This patch adds a configure check to test if gcc supports attribute ifunc. > The support can either be enabled in <gcc-src>/gcc/config.gcc for one > architecture in general by setting default_gnu_indirect_function variable to yes > or by configuring gcc with --enable-gnu-indirect-function. > > The next patch rewrites libc_ifunc macro to use gcc attribute ifunc instead > of inline assembly to generate the IFUNC symbols due to false debuginfo. > > If gcc does not support attribute ifunc and glibc is configured with > --enable-multi-arch then configure will abort with an error message. > If --enable-multi-arch is not given then configure will silently > disable multi-arch support. I'm concerned about requiring non-default configure options. Various architectures support IFUNC in binutils and may have done so for a long time, but without the option being enabled in GCC by default. There should be a much more detailed analysis of which architectures have IFUNC support in binutils, when it was added, and correspondingly, whether it's enabled by default in GCC for those architectures and, if so, when such enabling was added. In any case, new compiler requirements must be documented in install.texi with INSTALL regenerated. And should have appropriate NEWS entries as well.
On Mon, 8 Aug 2016, Paul E. Murphy wrote: > One more hiccup after trying out this patch. pt-vfork.c mandates ifunc > on most (all?) targets using nptl. It seems configure needs to mandate > this support on any target using nptl. No it doesn't. Many architectures have their own pt-vfork.S.
On Mo, Aug 08 2016, "Paul E. Murphy" <murphyp@linux.vnet.ibm.com> wrote: > One more hiccup after trying out this patch. pt-vfork.c mandates ifunc > on most (all?) targets using nptl. Only if there is no architecture override (some architecures can just use a tail call). Andreas.
On 08/08/2016 06:40 PM, Joseph Myers wrote: > On Mon, 8 Aug 2016, Stefan Liebler wrote: > >> This patch adds a configure check to test if gcc supports attribute ifunc. >> The support can either be enabled in <gcc-src>/gcc/config.gcc for one >> architecture in general by setting default_gnu_indirect_function variable to yes >> or by configuring gcc with --enable-gnu-indirect-function. >> >> The next patch rewrites libc_ifunc macro to use gcc attribute ifunc instead >> of inline assembly to generate the IFUNC symbols due to false debuginfo. >> >> If gcc does not support attribute ifunc and glibc is configured with >> --enable-multi-arch then configure will abort with an error message. >> If --enable-multi-arch is not given then configure will silently >> disable multi-arch support. > > I'm concerned about requiring non-default configure options. Various > architectures support IFUNC in binutils and may have done so for a long > time, but without the option being enabled in GCC by default. There > should be a much more detailed analysis of which architectures have IFUNC > support in binutils, when it was added, and correspondingly, whether it's > enabled by default in GCC for those architectures and, if so, when such > enabling was added. Is this really necessary? Can't we rely on architecture maintainers to do this? Moving from assembler hacks to a cleaner, GCC-provided interface is a desirable cleanup. Stefan took up this work because we ask him to, and now his little project went sideways *again*. This doesn't look fair to me. We should probably go back to Stefan's original approach (sorry, Stefan). > In any case, new compiler requirements must be documented in install.texi > with INSTALL regenerated. And should have appropriate NEWS entries as > well. Agreed on this part. Thanks, Florian
On 08/08/2016 06:08 PM, Paul E. Murphy wrote: > > > On 08/08/2016 09:38 AM, Stefan Liebler wrote: >> This patch adds a configure check to test if gcc supports attribute ifunc. >> The support can either be enabled in <gcc-src>/gcc/config.gcc for one >> architecture in general by setting default_gnu_indirect_function variable to yes >> or by configuring gcc with --enable-gnu-indirect-function. >> >> The next patch rewrites libc_ifunc macro to use gcc attribute ifunc instead >> of inline assembly to generate the IFUNC symbols due to false debuginfo. >> >> If gcc does not support attribute ifunc and glibc is configured with >> --enable-multi-arch then configure will abort with an error message. >> If --enable-multi-arch is not given then configure will silently >> disable multi-arch support. >> >> ChangeLog: >> >> * configure.ac: Add check if gcc supports attribute ifunc feature. >> * configure: Regenerated. > > > One more hiccup after trying out this patch. pt-vfork.c mandates ifunc > on most (all?) targets using nptl. It seems configure needs to mandate > this support on any target using nptl. > > According to the comment in nptl/pt-vfork.c: /* Note! If the architecture doesn't support IFUNC, then we need an alternate target-specific mechanism to implement this. So we just assume IFUNC here and require that the target override this file if necessary. If the architecture can assume all supported versions of gcc will produce a tail-call to __libc_vfork, consider including the version in sysdeps/unix/sysv/linux/aarch64/pt-vfork.c. */ The following targets have an own version of pt-vfork.[cS]: ./sysdeps/unix/sysv/linux/sh/pt-vfork.S ./sysdeps/unix/sysv/linux/hppa/pt-vfork.S ./sysdeps/unix/sysv/linux/microblaze/pt-vfork.S ./sysdeps/unix/sysv/linux/ia64/pt-vfork.S ./sysdeps/unix/sysv/linux/aarch64/pt-vfork.c ./sysdeps/unix/sysv/linux/tile/pt-vfork.c ./sysdeps/unix/sysv/linux/mips/pt-vfork.S ./sysdeps/unix/sysv/linux/s390/pt-vfork.S ./sysdeps/unix/sysv/linux/sparc/pt-vfork.S ./sysdeps/unix/sysv/linux/m68k/pt-vfork.c ./sysdeps/unix/sysv/linux/alpha/pt-vfork.S
On 08/09/2016 01:56 PM, Florian Weimer wrote: > On 08/08/2016 06:40 PM, Joseph Myers wrote: >> On Mon, 8 Aug 2016, Stefan Liebler wrote: >> >>> This patch adds a configure check to test if gcc supports attribute >>> ifunc. >>> The support can either be enabled in <gcc-src>/gcc/config.gcc for one >>> architecture in general by setting default_gnu_indirect_function >>> variable to yes >>> or by configuring gcc with --enable-gnu-indirect-function. >>> >>> The next patch rewrites libc_ifunc macro to use gcc attribute ifunc >>> instead >>> of inline assembly to generate the IFUNC symbols due to false debuginfo. >>> >>> If gcc does not support attribute ifunc and glibc is configured with >>> --enable-multi-arch then configure will abort with an error message. >>> If --enable-multi-arch is not given then configure will silently >>> disable multi-arch support. >> >> I'm concerned about requiring non-default configure options. Various >> architectures support IFUNC in binutils and may have done so for a long >> time, but without the option being enabled in GCC by default. There >> should be a much more detailed analysis of which architectures have IFUNC >> support in binutils, when it was added, and correspondingly, whether it's >> enabled by default in GCC for those architectures and, if so, when such >> enabling was added. Here are some information so far: In <gcc-src>/gcc/config.gcc IFUNC-support is only enabled by default for intel and s390 targets. See the following commits: -"Don't enable IFUNC by default for Android and uclibc" 2014-11-14 precedes gcc-5_1_0-release (https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e6cdd6b1755033e8f416efaa4334d1294c0a43c6) Don't set default_gnu_indirect_function variable=yes for targets *-*-*android*|*-*-*uclibc*. -"* config.gcc: Enable ifunc attribute by default on s390 and s390x." 2012-07-05 precedes gcc-4_8_0-release, gcc-4_9_0-release, gcc-5_1_0-release (https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=fcddec9f2159d14c98924e4ccd2dabffa6bd4a0e) default_gnu_indirect_function=yes for targets s390x-*-linux* and s390-*-linux*. -"* config.gcc (i[34567]86-*-linux*): Set default_gnu_indirect_function to yes." 2011-07-22 precedes alpha-v0.1, gcc-4_7_0-release, gcc-4_8_0-release, gcc-4_9_0-release, gcc-5_1_0-release (https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=2c6c4996dd9e4b6cc96639b270954405c79f648d) default_gnu_indirect_function=yes for targets x86_64-*-linux* -"* configure.ac: Add --enable-indirect-function option." 2010-09-29 precedes alpha-v0.1, gcc-4_6_0-release, gcc-4_7_0-release, gcc-4_8_0-release, gcc-4_9_0-release, gcc-5_1_0-release (https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=1c6e0788f1a7e6b577c101d5c3ee9a8cb01ac4d5) default_gnu_indirect_function=yes for targets i[34567]86-*-linux* and default_gnu_indirect_function=glibc-2011 for targets x86_64-*-linux* Each distro can activate gcc ifunc support with gcc configure option --enable-gnu-indirect-function. According to the gcc.specs file from fedora 21 gcc-4.9.2-1.fc21.src.rpm this is done for fedora >= 21 and rhel >= 7: %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 The debian/ubuntu/opensuse gcc source packages rely on the default behaviour and don't use --enable-gnu-indirect-function. > > Is this really necessary? Can't we rely on architecture maintainers to > do this? > > Moving from assembler hacks to a cleaner, GCC-provided interface is a > desirable cleanup. Stefan took up this work because we ask him to, and > now his little project went sideways *again*. This doesn't look fair to > me. We should probably go back to Stefan's original approach (sorry, > Stefan). One way is to rely on the distro maintainers which picks up the glibc release with this patchset that they also use a gcc with ifunc support. I think the better approach is the previous way to test gcc ifunc support and use it if available and provide the old behaviour as fallback (see the former version of the patchset). This way we don't break an architecture which is currently using binutils-only-ifunc support. Afterwards we can add a NEWS entry, update the recommended gcc in INSTALL, send a note to the distribution maintainers. Perhaps more gccs support ifunc in the future and we can remove the fallback. > >> In any case, new compiler requirements must be documented in install.texi >> with INSTALL regenerated. And should have appropriate NEWS entries as >> well. > > Agreed on this part. > > Thanks, > Florian >
On Tue, 9 Aug 2016, Florian Weimer wrote: > > I'm concerned about requiring non-default configure options. Various > > architectures support IFUNC in binutils and may have done so for a long > > time, but without the option being enabled in GCC by default. There > > should be a much more detailed analysis of which architectures have IFUNC > > support in binutils, when it was added, and correspondingly, whether it's > > enabled by default in GCC for those architectures and, if so, when such > > enabling was added. > > Is this really necessary? Can't we rely on architecture maintainers to do > this? If you're proposing increasing build requirements, you need to give the community a proper understanding of what the proposed change is. In this case, the impacts depend on the architecture, as some architectures (e.g. AArch64, ARM, PowerPC, SPARC) have IFUNC support but do not enable it by default in GCC, while on some other architectures, a nondefault GCC configure option would be needed to disable the support.
Hi, as information: I filed the following bugzilla "Bug 20478 - libc_ifunc macro and similar usages leads to false debug-information." (https://sourceware.org/bugzilla/show_bug.cgi?id=20478) How to proceed with this patchset? My suggestion is to check for gcc-attribute-ifunc support while configuring glibc. If it is available (currently on intel, s390 and on fedora/rhel for power and arm) the gcc-attribute-ifunc will be used for ifunc-macros to get rid of the false debug-information. If it is not available the current approach for ifunc-macros will be used instead. This way the debug-information is not correct, but it does not break current configurations without the gcc-support. Furthermore the "assembler hacks" for ifunc-handling are not scattered in multiple files but are used only indirect via ifunc-macros and can simply removed in libc-symbols.h in future. If glibc is configured with --enable-multi-arch then configure will dump a warning that gcc does not support attribute ifunc and it can be enabled by configuring gcc with --enable-gnu-indirect-function. I'll add a note in INSTALL/NEWS file that a gcc with ifunc support is recommended for multi-arch glibcs and how it can be enabled. After the patchset is committed I can write a note to the Distribution Maintainers listed in https://sourceware.org/glibc/wiki/MAINTAINERS#Distribution_Maintainers to enable ifunc in their gccs. Furthermore I can file a bug against gcc to enable ifunc support in <gcc-src>/gcc/config.gcc on architectures which support ifunc in binutils. If we have consensus about this approach, I'll prepare/post a further version of the patchset. Bye Stefan
On 08/17/2016 12:37 PM, Stefan Liebler wrote: > My suggestion is to check for gcc-attribute-ifunc support while > configuring glibc. > > If it is available (currently on intel, s390 and on fedora/rhel for > power and arm) the gcc-attribute-ifunc will be used for ifunc-macros to > get rid of the false debug-information. > > If it is not available the current approach for ifunc-macros will be > used instead. This way the debug-information is not correct, but it does > not break current configurations without the gcc-support. > Furthermore the "assembler hacks" for ifunc-handling are not scattered > in multiple files but are used only indirect via ifunc-macros and can > simply removed in libc-symbols.h in future. > If glibc is configured with --enable-multi-arch then configure will dump > a warning that gcc does not support attribute ifunc and it can be > enabled by configuring gcc with --enable-gnu-indirect-function. This looks like a good plan to me. Thanks, Florian
On Wed, 17 Aug 2016, Florian Weimer wrote: > On 08/17/2016 12:37 PM, Stefan Liebler wrote: > > My suggestion is to check for gcc-attribute-ifunc support while > > configuring glibc. > > > > If it is available (currently on intel, s390 and on fedora/rhel for > > power and arm) the gcc-attribute-ifunc will be used for ifunc-macros to > > get rid of the false debug-information. > > > > If it is not available the current approach for ifunc-macros will be > > used instead. This way the debug-information is not correct, but it does > > not break current configurations without the gcc-support. > > Furthermore the "assembler hacks" for ifunc-handling are not scattered > > in multiple files but are used only indirect via ifunc-macros and can > > simply removed in libc-symbols.h in future. > > If glibc is configured with --enable-multi-arch then configure will dump > > a warning that gcc does not support attribute ifunc and it can be > > enabled by configuring gcc with --enable-gnu-indirect-function. > > This looks like a good plan to me. Likewise.
On 08/17/2016 12:37 PM, Stefan Liebler wrote: > If we have consensus about this approach, I'll prepare/post a further > version of the patchset. > > Bye > Stefan > After the answers from Florian and Joseph I've posted a further version here: "[PATCH v3 1/9] Add configure check to test if gcc supports attribute ifunc." https://www.sourceware.org/ml/libc-alpha/2016-08/msg00752.html
diff --git a/configure b/configure index 17625e1..aedada3 100755 --- a/configure +++ b/configure @@ -3914,9 +3914,40 @@ fi { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_ld_gnu_indirect_function" >&5 $as_echo "$libc_cv_ld_gnu_indirect_function" >&6; } -if test x"$libc_cv_ld_gnu_indirect_function" != xyes; then +# Check if gcc supports attribute ifunc as it is used in libc_ifunc macro. +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for gcc attribute ifunc support" >&5 +$as_echo_n "checking for gcc attribute ifunc support... " >&6; } +if ${libc_cv_gcc_indirect_function+:} false; then : + $as_echo_n "(cached) " >&6 +else + cat > conftest.c <<EOF +extern int func (int); +int used_func (int a) +{ + return a; +} +static void *resolver () +{ + return &used_func; +} +extern __typeof (func) func __attribute__ ((ifunc ("resolver"))); +EOF +libc_cv_gcc_indirect_function=no +if ${CC-cc} -c conftest.c -o conftest.o 1>&5 \ + 2>&5 ; then + if $READELF -s conftest.o | grep IFUNC >/dev/null 2>&5; then + libc_cv_gcc_indirect_function=yes + fi +fi +rm -f conftest* +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_gcc_indirect_function" >&5 +$as_echo "$libc_cv_gcc_indirect_function" >&6; } + +if test x"$libc_cv_ld_gnu_indirect_function" != xyes \ + || test x"$libc_cv_gcc_indirect_function" != xyes; then if test x"$multi_arch" = xyes; then - as_fn_error $? "--enable-multi-arch support requires assembler and linker support" "$LINENO" 5 + as_fn_error $? "--enable-multi-arch support require gcc, assembler and linker indirect-function support" "$LINENO" 5 else multi_arch=no fi @@ -6499,7 +6530,8 @@ fi # A sysdeps configure fragment can reset this if IFUNC is not actually # usable even though the assembler knows how to generate the symbol type. -if test x"$libc_cv_ld_gnu_indirect_function" = xyes; then +if test x"$libc_cv_ld_gnu_indirect_function" = xyes \ + && test x"$libc_cv_gcc_indirect_function" = xyes; then $as_echo "#define HAVE_IFUNC 1" >>confdefs.h fi diff --git a/configure.ac b/configure.ac index 33bcd62..0961151 100644 --- a/configure.ac +++ b/configure.ac @@ -634,9 +634,34 @@ if ${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS \ fi rm -f conftest*]) -if test x"$libc_cv_ld_gnu_indirect_function" != xyes; then +# Check if gcc supports attribute ifunc as it is used in libc_ifunc macro. +AC_CACHE_CHECK([for gcc attribute ifunc support], + libc_cv_gcc_indirect_function, [dnl +cat > conftest.c <<EOF +extern int func (int); +int used_func (int a) +{ + return a; +} +static void *resolver () +{ + return &used_func; +} +extern __typeof (func) func __attribute__ ((ifunc ("resolver"))); +EOF +libc_cv_gcc_indirect_function=no +if ${CC-cc} -c conftest.c -o conftest.o 1>&AS_MESSAGE_LOG_FD \ + 2>&AS_MESSAGE_LOG_FD ; then + if $READELF -s conftest.o | grep IFUNC >/dev/null 2>&AS_MESSAGE_LOG_FD; then + libc_cv_gcc_indirect_function=yes + fi +fi +rm -f conftest*]) + +if test x"$libc_cv_ld_gnu_indirect_function" != xyes \ + || test x"$libc_cv_gcc_indirect_function" != xyes; then if test x"$multi_arch" = xyes; then - AC_MSG_ERROR([--enable-multi-arch support requires assembler and linker support]) + AC_MSG_ERROR([--enable-multi-arch support require gcc, assembler and linker indirect-function support]) else multi_arch=no fi @@ -1766,7 +1791,8 @@ AC_SUBST(libc_cv_gcc_unwind_find_fde) # A sysdeps configure fragment can reset this if IFUNC is not actually # usable even though the assembler knows how to generate the symbol type. -if test x"$libc_cv_ld_gnu_indirect_function" = xyes; then +if test x"$libc_cv_ld_gnu_indirect_function" = xyes \ + && test x"$libc_cv_gcc_indirect_function" = xyes; then AC_DEFINE(HAVE_IFUNC) fi