[RFC] New configure option "--disable-ifunc"

Message ID "013e01d08d9a$80b32b10$82198130$@guseva"@samsung.com
State New, archived
Headers

Commit Message

Maria Guseva May 13, 2015, 4:33 p.m. UTC
  Hi all,

The proposed patch adds a new configure option in order to disable IFUNC
usage in Glibc.
Currently in Glibc configure there is an option to disable multiarch while
the IFUNC usage still may be enabled by default if linker supports it. 

I've tested the new option for ARM configuration. So I have to add pt-vfork
implementation in arm sysdeps - cause the common pt-vfork implementation
suggest IFUNC usage only. As I understood it was done in order to guarantee
the tail call in non-IFUNC case. Are there any reasons for this except
performance? If no I suggest to change the patch and move non-IFUNC
implementation in common source nptl/pt-vfork.c instead of copying aarch64
implementation in sysdeps folders. 

The current patch just includes the aarch64 implementation for arm.


Regards,
Maria
  

Comments

Joseph Myers May 13, 2015, 4:54 p.m. UTC | #1
On Wed, 13 May 2015, Maria Guseva wrote:

> The proposed patch adds a new configure option in order to disable IFUNC
> usage in Glibc.

What problem is this solving?  Each configure option adds to the 
combinatorial explosion of poorly-tested different ways to configure 
glibc, so there needs to be strong justification for adding a new option 
(and increasing the minimum supported version of build tools can often be 
preferred to having conditionals to support older tools, depending on how 
widespread the older tools are).

If a new option is added, it needs documenting in install.texi and INSTALL 
needs to be regenerated.
  
Richard Henderson May 13, 2015, 5:07 p.m. UTC | #2
On 05/13/2015 09:33 AM, Maria Guseva wrote:
> As I understood it was done in order to guarantee
> the tail call in non-IFUNC case. Are there any reasons for this except
> performance?


Yes.  If the return address for vfork is on the stack, it'll be corrupted and
the program will crash.


r~
  
Maria Guseva May 13, 2015, 5:22 p.m. UTC | #3
> On Wed, 13 May 2015, Maria Guseva wrote:

>> The proposed patch adds a new configure option in order to disable 
>> IFUNC usage in Glibc.

> What problem is this solving?  Each configure option adds to the
combinatorial explosion of poorly-tested different ways to configure >
glibc, so there needs to be strong justification for adding a new option
(and increasing the minimum supported version of build tools > can often be
preferred to having conditionals to support older tools, depending on how
widespread the older tools are).

We faced issues when Glibc was used with Prelink. As I know the latest
Prelink supports ifuncs but older versions are not working correctly with
it. We need to disable ifuncs in Glibc for such case.
  
Joseph Myers May 13, 2015, 5:25 p.m. UTC | #4
On Wed, 13 May 2015, Maria Guseva wrote:

> We faced issues when Glibc was used with Prelink. As I know the latest
> Prelink supports ifuncs but older versions are not working correctly with
> it. We need to disable ifuncs in Glibc for such case.

This seems like a case for simply saying that old prelink won't work with 
new glibc, rather than doubling the number of glibc configurations.
  
Carlos O'Donell May 13, 2015, 5:30 p.m. UTC | #5
On 05/13/2015 01:25 PM, Joseph Myers wrote:
> On Wed, 13 May 2015, Maria Guseva wrote:
> 
>> We faced issues when Glibc was used with Prelink. As I know the latest
>> Prelink supports ifuncs but older versions are not working correctly with
>> it. We need to disable ifuncs in Glibc for such case.
> 
> This seems like a case for simply saying that old prelink won't work with 
> new glibc, rather than doubling the number of glibc configurations.

Agreed.

Or at best making --disable-multi-arch disable all current uses of IFUNC,
and or fixing the configure.ac code that turns on IFUNC automatically if
multi-arch is set to default and the linker supports IFUNCs.

Cheers,
Carlos.
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 1e4d6f8..c025f3c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@ 
+2015-05-13 Maria Guseva  <m.guseva@samsung.com>
+
+       * configure.ac (--disable-ifunc): New configure option.
+       (use_ifunc): New AC_SUBST.
+       * configure: Regenerated.
+       * sysdeps/unix/sysv/linux/arm/pt-vfork.c: New file.
+
 2015-05-13  Szabolcs Nagy  <szabolcs.nagy@arm.com>

        * sysdeps/aarch64/tls-macros.h (TLS_GD): Add "cc" to the clobber
diff --git a/configure b/configure
index 97e549e..b63a736 100755
--- a/configure
+++ b/configure
@@ -662,6 +662,7 @@  sysdeps_add_ons
 sysnames
 submachine
 multi_arch
+use_ifunc
 base_machine
 add_on_subdirs
 add_ons
@@ -770,6 +771,7 @@  enable_maintainer_mode
 enable_kernel
 enable_all_warnings
 enable_werror
+enable_ifunc
 enable_multi_arch
 enable_nss_crypt
 enable_obsolete_rpc
@@ -1432,6 +1434,7 @@  Optional Features:
                           VERSION
   --enable-all-warnings   enable all useful warnings gcc can issue
   --disable-werror        do not build with -Werror
+  --disable-ifunc         do not use gnu_indirect_function
   --enable-multi-arch     enable single DSO with optimizations for multiple
                           architectures
   --enable-nss-crypt      enable libcrypt to use nss
@@ -3668,6 +3671,14 @@  fi



+# Check whether --enable-ifunc was given.
+if test "${enable_ifunc+set}" = set; then :
+  enableval=$enable_ifunc; use_ifunc=$enableval
+else
+  use_ifunc=default
+fi
+
+
 # Check whether --enable-multi-arch was given.
 if test "${enable_multi_arch+set}" = set; then :
   enableval=$enable_multi_arch; multi_arch=$enableval
@@ -4125,8 +4136,21 @@  $as_echo "yes" >&6; }
 fi

 if test x"$libc_cv_ld_gnu_indirect_function" != xyes; then
+  if test x"$use_ifunc" = xyes; then
+    as_fn_error $? "--enable-ifunc support requires assembler and linker
support" "$LINENO" 5
+  else
+    use_ifunc=no
+  fi
+fi
+
+if test "x$use_ifunc" = xdefault; then
+  use_ifunc=$libc_cv_ld_gnu_indirect_function
+fi
+
+
+if test x"$use_ifunc" != 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 requires ifunc support"
"$LINENO" 5
   else
     multi_arch=no
   fi
@@ -7226,7 +7250,7 @@  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"$use_ifunc" = xyes; then
   $as_echo "#define HAVE_IFUNC 1" >>confdefs.h

 fi
diff --git a/configure.ac b/configure.ac
index def655a..ac30620 100644
--- a/configure.ac
+++ b/configure.ac
@@ -269,6 +269,12 @@  AC_ARG_ENABLE([werror],
              [enable_werror=yes])
 AC_SUBST(enable_werror)

+AC_ARG_ENABLE([ifunc],
+             AC_HELP_STRING([--disable-ifunc],
+                            [do not use gnu_indirect_function]),
+             [use_ifunc=$enableval],
+             [use_ifunc=default])
+
 AC_ARG_ENABLE([multi-arch],
              AC_HELP_STRING([--enable-multi-arch],
                             [enable single DSO with optimizations for
multiple architectures]),
@@ -623,8 +629,21 @@  else
 fi

 if test x"$libc_cv_ld_gnu_indirect_function" != xyes; then
+  if test x"$use_ifunc" = xyes; then
+    AC_MSG_ERROR([--enable-ifunc support requires assembler and linker
support])
+  else
+    use_ifunc=no
+  fi
+fi
+
+if test "x$use_ifunc" = xdefault; then
+  use_ifunc=$libc_cv_ld_gnu_indirect_function
+fi
+AC_SUBST(use_ifunc)
+
+if test x"$use_ifunc" != 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 requires ifunc support])
   else
     multi_arch=no
   fi
@@ -2004,7 +2023,7 @@  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"$use_ifunc" = xyes; then
   AC_DEFINE(HAVE_IFUNC)
 fi

diff --git a/sysdeps/unix/sysv/linux/arm/pt-vfork.c
b/sysdeps/unix/sysv/linux/arm/pt-vfork.c
new file mode 100644
index 0000000..dfeda3b
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/arm/pt-vfork.c
@@ -0,0 +1,19 @@ 
+/* vfork ABI-compatibility entry points for libpthread.
+   Copyright (C) 2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <sysdeps/unix/sysv/linux/aarch64/pt-vfork.c>