diff mbox

[v2,01/16] linux: Fix vDSO macros build with time64 interfaces

Message ID 20191217214728.2886-1-adhemerval.zanella@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Netto Dec. 17, 2019, 9:47 p.m. UTC 
  Changes from previous version:

  - Reinstate syscall fallback on INLINE_VSYSCALL, it simplifies
    when the macro is used multiple times (as for clock_gettime
    and clock_getres).

--

As indicated on libc-help [1] the ec138c67cb commit broke 32-bit
builds when configured with --enable-kernel=5.1 or higher.  The
scenario 10 from [2] might also occur in this configuration and
INLINE_VSYSCALL will try to use the vDSO symbol and
HAVE_CLOCK_GETTIME64_VSYSCALL does not set HAVE_VSYSCALL prior its
usage.

Also, there is no easy way to just enable the code to use one
vDSO sysmbo since the macro INLINE_VSYSCALL is redefined if
HAVE_VSYSCALL is set.

Instead of adding more pre-processor handling and making the code
even more convoluted, this patch removes the requirement of defining
HAVE_VSYSCALL before including sysdep-vdso.h to enable vDSO usage.

The INLINE_VSYSCALL is now expected to be issued inside a
HAVE_*_VSYSCALL check, since it will try to use the internal vDSO
pointers.

Both clock_getres and clock_gettime vDSO code for time64_t were
removed since there is no vDSO setup code for the symbol (an
architecture can not set HAVE_CLOCK_GETTIME64_VSYSCALL).

Checked on i686-linux-gnu (default and with --enable-kernel=5.1),
x86_64-linux-gnu, aarch64-linux-gnu, and powerpc64le-linux-gnu.
I also checked against a build to mips64-linux-gnu and
sparc64-linux-gnu.

[1] https://sourceware.org/ml/libc-help/2019-12/msg00014.html
---
 .../unix/sysv/linux/aarch64/gettimeofday.c    |  4 --
 sysdeps/unix/sysv/linux/clock_getres.c        | 36 +++++++++++-------
 sysdeps/unix/sysv/linux/clock_gettime.c       | 38 +++++++++++--------
 sysdeps/unix/sysv/linux/getcpu.c              |  9 +----
 .../unix/sysv/linux/powerpc/gettimeofday.c    |  4 --
 sysdeps/unix/sysv/linux/powerpc/time.c        |  4 --
 sysdeps/unix/sysv/linux/sched_getcpu.c        | 15 +++-----
 sysdeps/unix/sysv/linux/sysdep-vdso.h         | 34 +----------------
 sysdeps/unix/sysv/linux/x86/gettimeofday.c    |  4 --
 sysdeps/unix/sysv/linux/x86/time.c            |  8 ++--
 10 files changed, 58 insertions(+), 98 deletions(-)

Comments

Siddhesh Poyarekar Jan. 2, 2020, 12:07 p.m. UTC | #1 
On 18/12/19 3:17 am, Adhemerval Zanella wrote:
> Changes from previous version:
> 
>   - Reinstate syscall fallback on INLINE_VSYSCALL, it simplifies
>     when the macro is used multiple times (as for clock_gettime
>     and clock_getres).
> 
> --
> 
> As indicated on libc-help [1] the ec138c67cb commit broke 32-bit
> builds when configured with --enable-kernel=5.1 or higher.  The
> scenario 10 from [2] might also occur in this configuration and

What is scenario 10 from [2]?

> INLINE_VSYSCALL will try to use the vDSO symbol and
> HAVE_CLOCK_GETTIME64_VSYSCALL does not set HAVE_VSYSCALL prior its
> usage.
> 
> Also, there is no easy way to just enable the code to use one
> vDSO sysmbo since the macro INLINE_VSYSCALL is redefined if

symbol

> HAVE_VSYSCALL is set.
> 
> Instead of adding more pre-processor handling and making the code
> even more convoluted, this patch removes the requirement of defining
> HAVE_VSYSCALL before including sysdep-vdso.h to enable vDSO usage.
> 
> The INLINE_VSYSCALL is now expected to be issued inside a
> HAVE_*_VSYSCALL check, since it will try to use the internal vDSO
> pointers.
> 
> Both clock_getres and clock_gettime vDSO code for time64_t were
> removed since there is no vDSO setup code for the symbol (an
> architecture can not set HAVE_CLOCK_GETTIME64_VSYSCALL).
> 
> Checked on i686-linux-gnu (default and with --enable-kernel=5.1),
> x86_64-linux-gnu, aarch64-linux-gnu, and powerpc64le-linux-gnu.
> I also checked against a build to mips64-linux-gnu and
> sparc64-linux-gnu.
> 
> [1] https://sourceware.org/ml/libc-help/2019-12/msg00014.html
> ---
>  .../unix/sysv/linux/aarch64/gettimeofday.c    |  4 --
>  sysdeps/unix/sysv/linux/clock_getres.c        | 36 +++++++++++-------
>  sysdeps/unix/sysv/linux/clock_gettime.c       | 38 +++++++++++--------
>  sysdeps/unix/sysv/linux/getcpu.c              |  9 +----
>  .../unix/sysv/linux/powerpc/gettimeofday.c    |  4 --
>  sysdeps/unix/sysv/linux/powerpc/time.c        |  4 --
>  sysdeps/unix/sysv/linux/sched_getcpu.c        | 15 +++-----
>  sysdeps/unix/sysv/linux/sysdep-vdso.h         | 34 +----------------
>  sysdeps/unix/sysv/linux/x86/gettimeofday.c    |  4 --
>  sysdeps/unix/sysv/linux/x86/time.c            |  8 ++--
>  10 files changed, 58 insertions(+), 98 deletions(-)
> 

The change looks OK with the above nits.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Adhemerval Zanella Netto Jan. 2, 2020, 12:55 p.m. UTC | #2 
On 02/01/2020 09:07, Siddhesh Poyarekar wrote:
> On 18/12/19 3:17 am, Adhemerval Zanella wrote:
>> Changes from previous version:
>>
>>   - Reinstate syscall fallback on INLINE_VSYSCALL, it simplifies
>>     when the macro is used multiple times (as for clock_gettime
>>     and clock_getres).
>>
>> --
>>
>> As indicated on libc-help [1] the ec138c67cb commit broke 32-bit
>> builds when configured with --enable-kernel=5.1 or higher.  The
>> scenario 10 from [2] might also occur in this configuration and
> 
> What is scenario 10 from [2]?

Oops, I forgot to add the link itself
https://sourceware.org/ml/libc-alpha/2019-12/msg00142.html. It is:

  10. Define __NR_clock_gettime64 and __NR_clock_gettime and provide
      a 32-bit vDSO.
      - i.e. sparc32, powerpc32

The architecture will define it has clock_gettime time32 vDSO
(HAVE_CLOCK_GETTIME_VSYSCALL) and the INLINE_VSYSCALL macro will
be defined to call the vDSO symbol.  However since the architecture
only defines it for time32 version, the macro fails because it does
not internally define the function pointer for time64.

I did not take in consideration this scenario is also valid for
32-bit architecture with --enable-kernel=5.1 (the case of the
build failure reported on libc-help), where __ASSUME_TIME64_SYSCALLS
is defined and thus it might call the time64 vDSO.

> 
>> INLINE_VSYSCALL will try to use the vDSO symbol and
>> HAVE_CLOCK_GETTIME64_VSYSCALL does not set HAVE_VSYSCALL prior its
>> usage.
>>
>> Also, there is no easy way to just enable the code to use one
>> vDSO sysmbo since the macro INLINE_VSYSCALL is redefined if
> 
> symbol

Ack.

> 
>> HAVE_VSYSCALL is set.
>>
>> Instead of adding more pre-processor handling and making the code
>> even more convoluted, this patch removes the requirement of defining
>> HAVE_VSYSCALL before including sysdep-vdso.h to enable vDSO usage.
>>
>> The INLINE_VSYSCALL is now expected to be issued inside a
>> HAVE_*_VSYSCALL check, since it will try to use the internal vDSO
>> pointers.
>>
>> Both clock_getres and clock_gettime vDSO code for time64_t were
>> removed since there is no vDSO setup code for the symbol (an
>> architecture can not set HAVE_CLOCK_GETTIME64_VSYSCALL).
>>
>> Checked on i686-linux-gnu (default and with --enable-kernel=5.1),
>> x86_64-linux-gnu, aarch64-linux-gnu, and powerpc64le-linux-gnu.
>> I also checked against a build to mips64-linux-gnu and
>> sparc64-linux-gnu.
>>
>> [1] https://sourceware.org/ml/libc-help/2019-12/msg00014.html
>> ---
>>  .../unix/sysv/linux/aarch64/gettimeofday.c    |  4 --
>>  sysdeps/unix/sysv/linux/clock_getres.c        | 36 +++++++++++-------
>>  sysdeps/unix/sysv/linux/clock_gettime.c       | 38 +++++++++++--------
>>  sysdeps/unix/sysv/linux/getcpu.c              |  9 +----
>>  .../unix/sysv/linux/powerpc/gettimeofday.c    |  4 --
>>  sysdeps/unix/sysv/linux/powerpc/time.c        |  4 --
>>  sysdeps/unix/sysv/linux/sched_getcpu.c        | 15 +++-----
>>  sysdeps/unix/sysv/linux/sysdep-vdso.h         | 34 +----------------
>>  sysdeps/unix/sysv/linux/x86/gettimeofday.c    |  4 --
>>  sysdeps/unix/sysv/linux/x86/time.c            |  8 ++--
>>  10 files changed, 58 insertions(+), 98 deletions(-)
>>
> 
> The change looks OK with the above nits.
> 
> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c b/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c
index 7e772e05ce..143c5c1507 100644
--- a/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/aarch64/gettimeofday.c
@@ -22,10 +22,6 @@ 
 
 #include <time.h>
 #include <sysdep.h>
-
-#ifdef HAVE_GETTIMEOFDAY_VSYSCALL
-# define HAVE_VSYSCALL
-#endif
 #include <sysdep-vdso.h>
 
 /* Used as a fallback in the ifunc resolver if VDSO is not available
diff --git a/sysdeps/unix/sysv/linux/clock_getres.c b/sysdeps/unix/sysv/linux/clock_getres.c
index 9497f78787..52671fa641 100644
--- a/sysdeps/unix/sysv/linux/clock_getres.c
+++ b/sysdeps/unix/sysv/linux/clock_getres.c
@@ -20,9 +20,6 @@ 
 #include <errno.h>
 #include <time.h>
 
-#ifdef HAVE_CLOCK_GETRES_VSYSCALL
-# define HAVE_VSYSCALL
-#endif
 #include <sysdep-vdso.h>
 #include <shlib-compat.h>
 #include <kernel-features.h>
@@ -32,23 +29,34 @@  int
 __clock_getres64 (clockid_t clock_id, struct __timespec64 *res)
 {
 #ifdef __ASSUME_TIME64_SYSCALLS
-# ifndef __NR_clock_getres_time64
-  return INLINE_VSYSCALL (clock_getres, 2, clock_id, res);
+  /* 64 bit ABIs or Newer 32-bit ABIs that only support 64-bit time_t.  */
+# ifdef __NR_clock_getres_time64
+  return INLINE_SYSCALL_CALL (clock_getres_time64, clock_id, res);
 # else
-  return INLINE_SYSCALL (clock_getres_time64, 2, clock_id, res);
+#  ifdef HAVE_CLOCK_GETRES_VSYSCALL
+  return INLINE_VSYSCALL (clock_getres, 2, clock_id, res);
+#  else
+  return INLINE_SYSCALL_CALL (clock_getres, clock_id, res);
+#  endif
 # endif
 #else
+  int r;
+  /* Old 32-bit ABI with possible 64-bit time_t support.  */
 # ifdef __NR_clock_getres_time64
-  int ret = INLINE_SYSCALL (clock_getres_time64, 2, clock_id, res);
-  if (ret == 0 || errno != ENOSYS)
-    return ret;
+  r = INLINE_SYSCALL_CALL (clock_getres_time64, clock_id, res);
+  if (r == 0 || errno != ENOSYS)
+    return r;
 # endif
+  /* Fallback code that uses 32-bit support.  */
   struct timespec ts32;
-  int retval = INLINE_VSYSCALL (clock_getres, 2, clock_id, &ts32);
-  if (! retval && res)
+# ifdef HAVE_CLOCK_GETRES_VSYSCALL
+  r = INLINE_VSYSCALL (clock_getres, 2, clock_id, &ts32);
+# else
+  r = INLINE_SYSCALL_CALL (clock_getres, clock_id, &ts32);
+# endif
+  if (r == 0)
     *res = valid_timespec_to_timespec64 (ts32);
-
-  return retval;
+  return r;
 #endif
 }
 
@@ -60,7 +68,7 @@  __clock_getres (clockid_t clock_id, struct timespec *res)
   int retval;
 
   retval = __clock_getres64 (clock_id, &ts64);
-  if (! retval && res)
+  if (retval == 0 && res != NULL)
     *res = valid_timespec64_to_timespec (ts64);
 
   return retval;
diff --git a/sysdeps/unix/sysv/linux/clock_gettime.c b/sysdeps/unix/sysv/linux/clock_gettime.c
index 875c4fe905..e895812704 100644
--- a/sysdeps/unix/sysv/linux/clock_gettime.c
+++ b/sysdeps/unix/sysv/linux/clock_gettime.c
@@ -21,10 +21,6 @@ 
 #include <errno.h>
 #include <time.h>
 #include "kernel-posix-cpu-timers.h"
-
-#ifdef HAVE_CLOCK_GETTIME_VSYSCALL
-# define HAVE_VSYSCALL
-#endif
 #include <sysdep-vdso.h>
 
 #include <shlib-compat.h>
@@ -34,22 +30,34 @@  int
 __clock_gettime64 (clockid_t clock_id, struct __timespec64 *tp)
 {
 #ifdef __ASSUME_TIME64_SYSCALLS
-# ifndef __NR_clock_gettime64
-#  define __NR_clock_gettime64   __NR_clock_gettime
-#  define __vdso_clock_gettime64 __vdso_clock_gettime
+  /* 64 bit ABIs or Newer 32-bit ABIs that only support 64-bit time_t.  */
+# ifdef __NR_clock_gettime64
+  return INLINE_SYSCALL_CALL (clock_gettime64, clock_id, tp);
+# else
+#  ifdef HAVE_CLOCK_GETTIME_VSYSCALL
+  return INLINE_VSYSCALL (clock_gettime, 2, clock_id, tp);
+#  else
+  return INLINE_SYSCALL_CALL (clock_gettime, clock_id, tp);
+#  endif
 # endif
-   return INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp);
 #else
-# if defined HAVE_CLOCK_GETTIME64_VSYSCALL
-  int ret64 = INLINE_VSYSCALL (clock_gettime64, 2, clock_id, tp);
-  if (ret64 == 0 || errno != ENOSYS)
-    return ret64;
+  int r;
+  /* Old 32-bit ABI with possible 64-bit time_t support.  */
+# ifdef __NR_clock_gettime64
+  r = INLINE_SYSCALL_CALL (clock_gettime64, clock_id, tp);
+  if (r == 0 || errno != ENOSYS)
+    return r;
 # endif
+  /* Fallback code that uses 32-bit support.  */
   struct timespec tp32;
-  int ret = INLINE_VSYSCALL (clock_gettime, 2, clock_id, &tp32);
-  if (ret == 0)
+# ifdef HAVE_CLOCK_GETTIME_VSYSCALL
+  r = INLINE_VSYSCALL (clock_gettime, 2, clock_id, &tp32);
+# else
+  r = INLINE_SYSCALL_CALL (clock_gettime, clock_id, &tp32);
+# endif
+  if (r == 0)
     *tp = valid_timespec_to_timespec64 (tp32);
-  return ret;
+  return r;
 #endif
 }
 
diff --git a/sysdeps/unix/sysv/linux/getcpu.c b/sysdeps/unix/sysv/linux/getcpu.c
index fdd27203af..efb4bb7627 100644
--- a/sysdeps/unix/sysv/linux/getcpu.c
+++ b/sysdeps/unix/sysv/linux/getcpu.c
@@ -18,20 +18,15 @@ 
 #include <errno.h>
 #include <sched.h>
 #include <sysdep.h>
-
-#ifdef HAVE_GETCPU_VSYSCALL
-# define HAVE_VSYSCALL
-#endif
 #include <sysdep-vdso.h>
 
 int
 __getcpu (unsigned int *cpu, unsigned int *node)
 {
-#ifdef __NR_getcpu
+#ifdef HAVE_GETCPU_VSYSCALL
   return INLINE_VSYSCALL (getcpu, 3, cpu, node, NULL);
 #else
-  __set_errno (ENOSYS);
-  return -1;
+  return INLINE_SYSCALL_CALL (getcpu, cpu, node, NULL);
 #endif
 }
 weak_alias (__getcpu, getcpu)
diff --git a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
index 18d8f7cb7a..c40793fe64 100644
--- a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
@@ -17,10 +17,6 @@ 
 
 #include <time.h>
 #include <sysdep.h>
-
-#ifdef HAVE_GETTIMEOFDAY_VSYSCALL
-# define HAVE_VSYSCALL
-#endif
 #include <sysdep-vdso.h>
 
 static int
diff --git a/sysdeps/unix/sysv/linux/powerpc/time.c b/sysdeps/unix/sysv/linux/powerpc/time.c
index 80a4c73416..f2b4c76be8 100644
--- a/sysdeps/unix/sysv/linux/powerpc/time.c
+++ b/sysdeps/unix/sysv/linux/powerpc/time.c
@@ -18,10 +18,6 @@ 
 
 #include <time.h>
 #include <sysdep.h>
-
-#ifdef HAVE_TIME_VSYSCALL
-# define HAVE_VSYSCALL
-#endif
 #include <sysdep-vdso.h>
 
 static time_t
diff --git a/sysdeps/unix/sysv/linux/sched_getcpu.c b/sysdeps/unix/sysv/linux/sched_getcpu.c
index 65dd9fdda7..68ba7f3734 100644
--- a/sysdeps/unix/sysv/linux/sched_getcpu.c
+++ b/sysdeps/unix/sysv/linux/sched_getcpu.c
@@ -18,22 +18,17 @@ 
 #include <errno.h>
 #include <sched.h>
 #include <sysdep.h>
-
-#ifdef HAVE_GETCPU_VSYSCALL
-# define HAVE_VSYSCALL
-#endif
 #include <sysdep-vdso.h>
 
 int
 sched_getcpu (void)
 {
-#ifdef __NR_getcpu
   unsigned int cpu;
-  int r = INLINE_VSYSCALL (getcpu, 3, &cpu, NULL, NULL);
-
-  return r == -1 ? r : cpu;
+  int r = -1;
+#ifdef HAVE_GETCPU_VSYSCALL
+  r = INLINE_VSYSCALL (getcpu, 3, &cpu, NULL, NULL);
 #else
-  __set_errno (ENOSYS);
-  return -1;
+  r = INLINE_SYSCALL_CALL (getcpu, &cpu, NULL, NULL);
 #endif
+  return r == -1 ? r : cpu;
 }
diff --git a/sysdeps/unix/sysv/linux/sysdep-vdso.h b/sysdeps/unix/sysv/linux/sysdep-vdso.h
index cf614fbf8b..76a211e39b 100644
--- a/sysdeps/unix/sysv/linux/sysdep-vdso.h
+++ b/sysdeps/unix/sysv/linux/sysdep-vdso.h
@@ -20,17 +20,14 @@ 
 # define SYSDEP_VDSO_LINUX_H
 
 #include <dl-vdso.h>
+#include <libc-vdso.h>
 
 #ifndef INTERNAL_VSYSCALL_CALL
 # define INTERNAL_VSYSCALL_CALL(funcptr, err, nr, args...)		      \
      funcptr (args)
 #endif
 
-#ifdef HAVE_VSYSCALL
-
-# include <libc-vdso.h>
-
-# define INLINE_VSYSCALL(name, nr, args...)				      \
+#define INLINE_VSYSCALL(name, nr, args...)				      \
   ({									      \
     __label__ out;							      \
     __label__ iserr;							      \
@@ -59,31 +56,4 @@ 
     sc_ret;								      \
   })
 
-# define INTERNAL_VSYSCALL(name, err, nr, args...)			      \
-  ({									      \
-    __label__ out;							      \
-    long v_ret;								      \
-									      \
-    __typeof (__vdso_##name) vdsop = __vdso_##name;			      \
-    PTR_DEMANGLE (vdsop);						      \
-    if (vdsop != NULL)							      \
-      {									      \
-	v_ret = INTERNAL_VSYSCALL_CALL (vdsop, err, nr, ##args);	      \
-	if (!INTERNAL_SYSCALL_ERROR_P (v_ret, err)			      \
-	    || INTERNAL_SYSCALL_ERRNO (v_ret, err) != ENOSYS)		      \
-	  goto out;							      \
-      }									      \
-    v_ret = INTERNAL_SYSCALL (name, err, nr, ##args);			      \
-  out:									      \
-    v_ret;								      \
-  })
-#else
-
-# define INLINE_VSYSCALL(name, nr, args...) \
-   INLINE_SYSCALL (name, nr, ##args)
-# define INTERNAL_VSYSCALL(name, err, nr, args...) \
-   INTERNAL_SYSCALL (name, err, nr, ##args)
-
-#endif /* USE_VSYSCALL && defined HAVE_VSYSCALL */
-
 #endif /* SYSDEP_VDSO_LINUX_H  */
diff --git a/sysdeps/unix/sysv/linux/x86/gettimeofday.c b/sysdeps/unix/sysv/linux/x86/gettimeofday.c
index 190127d31e..8015c40210 100644
--- a/sysdeps/unix/sysv/linux/x86/gettimeofday.c
+++ b/sysdeps/unix/sysv/linux/x86/gettimeofday.c
@@ -18,10 +18,6 @@ 
 
 #include <time.h>
 #include <sysdep.h>
-
-#ifdef HAVE_GETTIMEOFDAY_VSYSCALL
-# define HAVE_VSYSCALL
-#endif
 #include <sysdep-vdso.h>
 
 static int
diff --git a/sysdeps/unix/sysv/linux/x86/time.c b/sysdeps/unix/sysv/linux/x86/time.c
index 4a03c46d21..85a9fc8a08 100644
--- a/sysdeps/unix/sysv/linux/x86/time.c
+++ b/sysdeps/unix/sysv/linux/x86/time.c
@@ -18,16 +18,16 @@ 
 
 #include <time.h>
 #include <sysdep.h>
-
-#ifdef HAVE_TIME_VSYSCALL
-# define HAVE_VSYSCALL
-#endif
 #include <sysdep-vdso.h>
 
 static time_t
 time_vsyscall (time_t *t)
 {
+#ifdef HAVE_TIME_VSYSCALL
   return INLINE_VSYSCALL (time, 1, t);
+#else
+  return INLINE_SYSCALL_CALL (time, t);
+#endif
 }
 
 #ifdef SHARED