diff mbox

[3/8] i386, x86: Use libc_ifunc macro for time, gettimeofday.

Message ID 1466682952-6301-3-git-send-email-stli@linux.vnet.ibm.com
State Superseded
Headers show

Commit Message

Stefan Liebler June 23, 2016, 11:55 a.m. UTC
This patch uses the libc_ifunc macro to create already existing ifunc functions
time and gettimeofday on intel. This way, the libc_hidden_def macro can be used
instead of the libc_ifunc_hidden_def one which was only used here. Thus the
macro is removed from libc-symbols.h.
On i386, the __GI_* symbols do not target the ifunc symbol and thus the
redirection construct has to be applied here.

ChangeLog:

	* sysdeps/unix/sysv/linux/x86/gettimeofday.c (__gettimeofday):
	Use libc_ifunc macro. Use libc_hidden_def instead of
	libc_ifunc_hidden_def.
	* sysdeps/unix/sysv/linux/x86/time.c (time): Likewise.
	* sysdeps/unix/sysv/linux/i386/gettimeofday.c (__gettimeofday):
	Redirect ifunced function in header and create a copy of the prototype
	for using as ifunc function. Add appropiate aliases to the real symbol
	names.
	* sysdeps/unix/sysv/linux/i386/time.c (time): Likewise.
	* include/libc-symbols.h
	(libc_ifunc_hidden_def, libc_ifunc_hidden_def1): Delete macro.
---
 include/libc-symbols.h                      | 15 ---------------
 sysdeps/unix/sysv/linux/i386/gettimeofday.c | 19 ++++++++++++++++---
 sysdeps/unix/sysv/linux/i386/time.c         | 15 ++++++++++++---
 sysdeps/unix/sysv/linux/x86/gettimeofday.c  | 20 ++++++--------------
 sysdeps/unix/sysv/linux/x86/time.c          | 18 +++++-------------
 5 files changed, 39 insertions(+), 48 deletions(-)

Comments

Florian Weimer July 4, 2016, 8:54 a.m. UTC | #1
On 06/23/2016 01:55 PM, Stefan Liebler wrote:
> This patch uses the libc_ifunc macro to create already existing ifunc functions
> time and gettimeofday on intel. This way, the libc_hidden_def macro can be used
> instead of the libc_ifunc_hidden_def one which was only used here. Thus the
> macro is removed from libc-symbols.h.
> On i386, the __GI_* symbols do not target the ifunc symbol and thus the
> redirection construct has to be applied here.

I'm not sure if I understand what is going on here. 
sysdeps/unix/sysv/linux/x86/gettimeofday.c is compiled straight for 
x86_64, but wrapped on i386, right?

I wonder if it possible to avoid some of the preprocessor magic by 
introducing a separate x86_64 file.

Thanks,
Florian
Adhemerval Zanella July 4, 2016, 1:40 p.m. UTC | #2
On 04/07/2016 05:54, Florian Weimer wrote:
> On 06/23/2016 01:55 PM, Stefan Liebler wrote:
>> This patch uses the libc_ifunc macro to create already existing ifunc functions
>> time and gettimeofday on intel. This way, the libc_hidden_def macro can be used
>> instead of the libc_ifunc_hidden_def one which was only used here. Thus the
>> macro is removed from libc-symbols.h.
>> On i386, the __GI_* symbols do not target the ifunc symbol and thus the
>> redirection construct has to be applied here.
> 
> I'm not sure if I understand what is going on here. sysdeps/unix/sysv/linux/x86/gettimeofday.c is compiled straight for x86_64, but wrapped on i386, right?
> 

If I recall correctly i686 has the same issue as powerpc32, where ifunc for local
hidden symbols fails to resolve where called by another function library function.
This examples shows the issue:

$ cat test.c
#define libc_ifunc_hidden_def1(local, name)   \
    __asm__ (".globl " #local "\n\t"         \
             ".hidden " #local "\n\t"        \
             #local " = " #name);

#define libc_ifunc_hidden_def(name) \
  libc_ifunc_hidden_def1 (__GI_##name, name)

int foo (void) __attribute__ ((ifunc ("foo_ifunc")));

static int global = 1;

static int
f1 (void)
{
  return 0;
}

static int
f2 (void)
{
  return 1;
}

void *
foo_ifunc (void)
{
  return global == 1 ? f1 : f2;
}

libc_ifunc_hidden_def (foo)
$ cat test2.c 
#define __hidden_proto_hiddenattr(attrs...) \
  __attribute__ ((visibility ("hidden"), ##attrs))
#define hidden_proto(name, attrs...) \
  __hidden_proto (name, , __GI_##name, ##attrs)
#define __hidden_proto(name, thread, internal, attrs...)           \
  extern thread __typeof (name) name __asm__ (#internal) \
  __hidden_proto_hiddenattr (attrs);

int foo (void);
hidden_proto (foo)

int bar (void)
{
  return foo ();
}
$ cat main.c 
int bar (void);

int main ()
{
  return bar ();
}
$ gcc -Wall -fpic test.c -c -m32
$ gcc -Wall -fpic test2.c -c -m32
$ gcc -shared test.o test2.o -o libtest.so -m32
$ gcc -Wall main.c -c -m32
$ gcc main.o -o main -L. -ltest -m32
$ LD_LIBRARY_PATH=. ./main 
Segmentation fault (core dumped)

I am using binutils 2.26 from Ubuntu 16.04 and I am not sure if it has been fixed
in more recent versions.

> I wonder if it possible to avoid some of the preprocessor magic by introducing a separate x86_64 file.

I am not sure if it would be gain, the i386/gettimeofday.c is doing exactly what
it does not support (libc_ifunc_hidden_def symbol being called from other library
calls).  I think it worth a comment though.

Another possible cleanup could be consolidate powerpc and x86 gettimeofday
gettimeofay.
Florian Weimer July 6, 2016, 12:19 p.m. UTC | #3
On 07/04/2016 03:40 PM, Adhemerval Zanella wrote:
> If I recall correctly i686 has the same issue as powerpc32, where ifunc for local
> hidden symbols fails to resolve where called by another function library function.
> This examples shows the issue:

Thanks.  I believe this is a missing diagnostic in binutils.  The static 
link should fail due to incompatible relocations.  It is not possible to 
call an IFUNC (which needs an indirect call) while also avoiding the PLT 
on x86 (which results in a direct call).

Have you reported the lack of a diagnostic as a binutils RFE/bug?

Thanks,
Florian
diff mbox

Patch

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index a77308a..c651b24 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -730,21 +730,6 @@  for linking")
 #define libm_ifunc_init()
 #define libm_ifunc(name, expr) __ifunc (name, expr, void, libm_ifunc_init)
 
-#ifdef HAVE_ASM_SET_DIRECTIVE
-# define libc_ifunc_hidden_def1(local, name)				\
-    __asm__ (".globl " #local "\n\t"					\
-	     ".hidden " #local "\n\t"					\
-	     ".set " #local ", " #name);
-#else
-# define libc_ifunc_hidden_def1(local, name)				\
-    __asm__ (".globl " #local "\n\t"					\
-	     ".hidden " #local "\n\t"					\
-	     #local " = " #name);
-#endif
-
-#define libc_ifunc_hidden_def(name) \
-  libc_ifunc_hidden_def1 (__GI_##name, name)
-
 /* Add the compiler optimization to inhibit loop transformation to library
    calls.  This is used to avoid recursive calls in memset and memmove
    default implementations.  */
diff --git a/sysdeps/unix/sysv/linux/i386/gettimeofday.c b/sysdeps/unix/sysv/linux/i386/gettimeofday.c
index 965bb81..fc6cfdd 100644
--- a/sysdeps/unix/sysv/linux/i386/gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/i386/gettimeofday.c
@@ -16,14 +16,27 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#ifdef SHARED
+# define __gettimeofday __redirect___gettimeofday
+#endif
+
 #include <sys/time.h>
 
 #ifdef SHARED
+# undef __gettimeofday
 
-# undef libc_ifunc_hidden_def
-# define libc_ifunc_hidden_def(name)  \
-  libc_ifunc_hidden_def1 (__GI_##name, __gettimeofday_syscall)
+extern __typeof (__redirect___gettimeofday) __libc___gettimeofday;
+# define __gettimeofday __libc___gettimeofday
 
+# undef libc_hidden_def
+# define libc_hidden_def(name) \
+  __hidden_ver1 (__gettimeofday_syscall, __GI___gettimeofday, \
+	       __gettimeofday_syscall);
 #endif
 
 #include <sysdeps/unix/sysv/linux/x86/gettimeofday.c>
+
+#ifdef SHARED
+# undef __gettimeofday
+strong_alias (__libc___gettimeofday, __gettimeofday)
+#endif
diff --git a/sysdeps/unix/sysv/linux/i386/time.c b/sysdeps/unix/sysv/linux/i386/time.c
index 62b78b2..bacbf84 100644
--- a/sysdeps/unix/sysv/linux/i386/time.c
+++ b/sysdeps/unix/sysv/linux/i386/time.c
@@ -17,10 +17,19 @@ 
    <http://www.gnu.org/licenses/>.  */
 
 #ifdef SHARED
+# define time __redirect_time
+#endif
+
+#include <time.h>
+
+#ifdef SHARED
+# undef time
+
+extern __typeof (__redirect_time) time;
 
-# undef libc_ifunc_hidden_def
-# define libc_ifunc_hidden_def(name)  \
-  libc_ifunc_hidden_def1 (__GI_##name, __time_syscall)
+# undef libc_hidden_def
+# define libc_hidden_def(name)  \
+  __hidden_ver1 (__time_syscall, __GI_time, __time_syscall);
 #endif
 
 #include <sysdeps/unix/sysv/linux/x86/time.c>
diff --git a/sysdeps/unix/sysv/linux/x86/gettimeofday.c b/sysdeps/unix/sysv/linux/x86/gettimeofday.c
index 36f7c26..02f8d6e 100644
--- a/sysdeps/unix/sysv/linux/x86/gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/x86/gettimeofday.c
@@ -29,20 +29,12 @@  __gettimeofday_syscall (struct timeval *tv, struct timezone *tz)
   return INLINE_SYSCALL (gettimeofday, 2, tv, tz);
 }
 
-void *gettimeofday_ifunc (void) __asm__ ("__gettimeofday");
-
-void *
-gettimeofday_ifunc (void)
-{
-  PREPARE_VERSION_KNOWN (linux26, LINUX_2_6);
-
-  /* If the vDSO is not available we fall back to syscall.  */
-  return (_dl_vdso_vsym ("__vdso_gettimeofday", &linux26)
-	  ?: (void*) (&__gettimeofday_syscall));
-}
-asm (".type __gettimeofday, %gnu_indirect_function");
-
-libc_ifunc_hidden_def(__gettimeofday)
+# undef INIT_ARCH
+# define INIT_ARCH() PREPARE_VERSION_KNOWN (linux26, LINUX_2_6)
+/* If the vDSO is not available we fall back to syscall.  */
+libc_ifunc (__gettimeofday, (_dl_vdso_vsym ("__vdso_gettimeofday", &linux26)
+			     ?: &__gettimeofday_syscall))
+libc_hidden_def (__gettimeofday)
 
 #else
 
diff --git a/sysdeps/unix/sysv/linux/x86/time.c b/sysdeps/unix/sysv/linux/x86/time.c
index f5f7f91..957d586 100644
--- a/sysdeps/unix/sysv/linux/x86/time.c
+++ b/sysdeps/unix/sysv/linux/x86/time.c
@@ -30,20 +30,12 @@  __time_syscall (time_t *t)
   return INTERNAL_SYSCALL (time, err, 1, t);
 }
 
-void *time_ifunc (void) __asm__ ("time");
-
-void *
-time_ifunc (void)
-{
-  PREPARE_VERSION_KNOWN (linux26, LINUX_2_6);
-
+#undef INIT_ARCH
+#define INIT_ARCH() PREPARE_VERSION_KNOWN (linux26, LINUX_2_6);
 /* If the vDSO is not available we fall back on the syscall.  */
-  return _dl_vdso_vsym ("__vdso_time", &linux26)
-			?: (void*) &__time_syscall;
-}
-asm (".type time, %gnu_indirect_function");
-
-libc_ifunc_hidden_def(time)
+libc_ifunc (time, (_dl_vdso_vsym ("__vdso_time", &linux26)
+		   ?:  &__time_syscall))
+libc_hidden_def (time)
 
 #else