Message ID | 20191217214728.2886-1-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | [v2,01/16] linux: Fix vDSO macros build with time64 interfaces | expand |
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>
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 --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