Message ID | 20190918211603.8444-4-lukma@denx.de |
---|---|
State | New |
Headers | show |
Series | y2038: Linux: Introduce __clock_settime64 function | expand |
On Wed, 18 Sep 2019, Lukasz Majewski wrote: > Tests: > - The code has been tested with x86_64/x86 (native compilation): > make PARALLELMFLAGS="-j8" && make xcheck PARALLELMFLAGS="-j8" > > - Run specific tests on ARM/x86 32bit systems (qemu): > https://github.com/lmajewski/meta-y2038 > and run tests: > https://github.com/lmajewski/y2038-tests/commits/master > on kernels with and without 64 bit time support. Could you give more details of the configurations (including kernel headers version, --enable-kernel version, kernel version used at runtime) for which you have built glibc and run the full glibc testsuite? I suspect this code is pretty close to being ready to go in - but such patches have plenty of scope for mistakes (such as were in earlier RV32 patch versions) such as typos, transposed parameters, etc., that are hard for humans to spot reliably and easy for compilers and testsuites to spot, so it's important to know that sufficient automated tests have been run to catch any such mistakes. I gave a list in <https://sourceware.org/ml/libc-alpha/2019-08/msg00234.html> of five configurations I think are relevant to cover the different cases in patches such as this ("new" = 5.1 or later; Florian's changes to provide syscall tables in glibc would allow "#ifdef __NR_clock_settime64" to be removed and eliminate case (b)): (a) one that has always had 64-bit time, e.g. x86_64; (b) one with 32-bit time and old kernel headers (any kernel version at runtime); (c) one with 32-bit time and new kernel headers, old kernel at runtime; (d) one with 32-bit time and new kernel headers, new kernel at runtime but no --enable-kernel; (e) one with 32-bit time and new kernel at runtime and new --enable-kernel. If some of these are problematic to test, you can ask for help testing them. For *this particular* patch you might not need to test both (c) and (d), because they are identical as far as compilation is concerned and the testsuite doesn't really cover execution of clock_settime anyway. But it's in the nature of the Y2038 changes - involving lots of conditional code - that it's necessary to test in several different configurations to cover the conditionals adequately. (For avoidance of doubt, it is *not* necessary to run build-many-glibcs.py for this patch, and nor would running build-many-glibcs.py be sufficient since it doesn't do execution testing or cover the kernel headers version and --enable-kernel variants.)
Hi Joseph, > On Wed, 18 Sep 2019, Lukasz Majewski wrote: > > > Tests: > > - The code has been tested with x86_64/x86 (native compilation): > > make PARALLELMFLAGS="-j8" && make xcheck PARALLELMFLAGS="-j8" > > > > - Run specific tests on ARM/x86 32bit systems (qemu): > > https://github.com/lmajewski/meta-y2038 > > and run tests: > > https://github.com/lmajewski/y2038-tests/commits/master > > on kernels with and without 64 bit time support. > > Could you give more details of the configurations (including kernel > headers version, --enable-kernel version, kernel version used at > runtime) for which you have built glibc and run the full glibc > testsuite? I suspect this code is pretty close to being ready to go > in - but such patches have plenty of scope for mistakes (such as were > in earlier RV32 patch versions) such as typos, transposed parameters, > etc., that are hard for humans to spot reliably and easy for > compilers and testsuites to spot, so it's important to know that > sufficient automated tests have been run to catch any such mistakes. > So I tested this on ARM32 QEMU BSP build with meta-y2038 [1]. I. Build testing: - Machine's MACHINE=qemux86-64-x32 MACHINE=qemux86-64 MACHINE=qemux86 MACHINE=qemuarm - Using glib's test-wrapper='/opt/Y2038/glibc/src/scripts/cross-test-ssh.sh for qemu ARM. - ../src/scripts/build-many-glibcs.py II. Runtime testing - with BSP [1]: runqemu -d y2038arm nographic - PREFERRED_VERSION_linux-y2038 = "5.1%" && Y2038_GLIBC_MIN_KERNEL_VERSION="5.1.0" --enable-kernel=${Y2038_GLIBC_MIN_KERNEL_VERSION} Linux y2038arm 5.1.21-y2038-4a9b1eb8bc3ba4ad8b3b1aa3317cf8d4a3aaad83 (Support __ASSUME_TIME64_SYSCALLS defined) - Only PREFERRED_VERSION_linux-y2038 = "5.1%" - the minimal kernel version is default one for current mainline glibc (The __ASSUME_TIME64_SYSCALLS not defined, but kernel supports __clock_settime64 syscalls). - PREFERRED_VERSION_linux-lts = "4.19%" with default minimal kernel version for contemporary glibc (SHA1: 87accae3978c77c1a50d19ea8e3da3f0248d2612) This kernel doesn't support __clock_settime64 syscalls, so we fallback to clock_settime. Above tests are performed with Y2038 redirection applied [2] as well as without (so the __TIMESIZE != 64 execution path is checked as well). III. Syscalls unit tests - test_y2038 program [3] installed on BSP. IV. For x86_64 I do run the - as I can do it on my HOST PC make PARALLELMFLAGS="-j8" && make xcheck PARALLELMFLAGS="-j8" (before and after the patchset). > I gave a list in > <https://sourceware.org/ml/libc-alpha/2019-08/msg00234.html> of five > configurations I think are relevant to cover the different cases in > patches such as this ("new" = 5.1 or later; Florian's changes to > provide syscall tables in glibc would allow "#ifdef > __NR_clock_settime64" to be removed and eliminate case (b)): > > (a) one that has always had 64-bit time, e.g. x86_64; > Point IV. > (b) one with 32-bit time and old kernel headers (any kernel version > at runtime); Point II. > > (c) one with 32-bit time and new kernel headers, old kernel at > runtime; > > (d) one with 32-bit time and new kernel headers, new kernel at > runtime but no --enable-kernel; > > (e) one with 32-bit time and new kernel at runtime and new > --enable-kernel. Point II. > I will double check if the above points are in sync with points a) to e). > If some of these are problematic to test, you can ask for help > testing them. For *this particular* patch you might not need to test > both (c) and (d), because they are identical as far as compilation is > concerned and the testsuite doesn't really cover execution of > clock_settime anyway. But it's in the nature of the Y2038 changes - > involving lots of conditional code - that it's necessary to test in > several different configurations to cover the conditionals adequately. QEMU (and OE/Yocto) helps here ... > > (For avoidance of doubt, it is *not* necessary to run > build-many-glibcs.py for this patch, and nor would running > build-many-glibcs.py be sufficient since it doesn't do execution > testing or cover the kernel headers version and --enable-kernel > variants.) > The build-many-glibcs helps with checking if there aren't some odd build breaks. It is one of many test approaches for testing the Y2038 problem. Note: [1] - https://github.com/lmajewski/meta-y2038 [2] - https://github.com/lmajewski/y2038_glibc/commit/1229b54508d0bb130a017a5b5591209167255665 [3] - https://github.com/lmajewski/y2038-tests/commits/master Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Joseph, > Hi Joseph, > > > On Wed, 18 Sep 2019, Lukasz Majewski wrote: > > > > > Tests: > > > - The code has been tested with x86_64/x86 (native compilation): > > > make PARALLELMFLAGS="-j8" && make xcheck PARALLELMFLAGS="-j8" > > > > > > - Run specific tests on ARM/x86 32bit systems (qemu): > > > https://github.com/lmajewski/meta-y2038 > > > and run tests: > > > https://github.com/lmajewski/y2038-tests/commits/master > > > on kernels with and without 64 bit time support. > > > > Could you give more details of the configurations (including kernel > > headers version, --enable-kernel version, kernel version used at > > runtime) for which you have built glibc and run the full glibc > > testsuite? I suspect this code is pretty close to being ready to go > > in - but such patches have plenty of scope for mistakes (such as > > were in earlier RV32 patch versions) such as typos, transposed > > parameters, etc., that are hard for humans to spot reliably and > > easy for compilers and testsuites to spot, so it's important to > > know that sufficient automated tests have been run to catch any > > such mistakes. > > So I tested this on ARM32 QEMU BSP build with meta-y2038 [1]. > > I. Build testing: > - Machine's > MACHINE=qemux86-64-x32 > MACHINE=qemux86-64 > MACHINE=qemux86 > MACHINE=qemuarm > > - Using glib's > test-wrapper='/opt/Y2038/glibc/src/scripts/cross-test-ssh.sh > > for qemu ARM. > > - ../src/scripts/build-many-glibcs.py > > > II. Runtime testing - with BSP [1]: > runqemu -d y2038arm nographic > > - PREFERRED_VERSION_linux-y2038 = "5.1%" && > Y2038_GLIBC_MIN_KERNEL_VERSION="5.1.0" > --enable-kernel=${Y2038_GLIBC_MIN_KERNEL_VERSION} > > Linux y2038arm 5.1.21-y2038-4a9b1eb8bc3ba4ad8b3b1aa3317cf8d4a3aaad83 > > (Support __ASSUME_TIME64_SYSCALLS defined) > > - Only PREFERRED_VERSION_linux-y2038 = "5.1%" - the minimal kernel > version is default one for current mainline glibc > > (The __ASSUME_TIME64_SYSCALLS not defined, but kernel supports > __clock_settime64 syscalls). > > - PREFERRED_VERSION_linux-lts = "4.19%" > with default minimal kernel version for contemporary glibc > (SHA1: 87accae3978c77c1a50d19ea8e3da3f0248d2612) > > This kernel doesn't support __clock_settime64 syscalls, so we > fallback to clock_settime. > > Above tests are performed with Y2038 redirection applied [2] as well > as without (so the __TIMESIZE != 64 execution path is checked as > well). > > III. Syscalls unit tests - test_y2038 program [3] installed on BSP. > > IV. For x86_64 I do run the - as I can do it on my HOST PC > > make PARALLELMFLAGS="-j8" && make xcheck PARALLELMFLAGS="-j8" > > (before and after the patchset). > > > I gave a list in > > <https://sourceware.org/ml/libc-alpha/2019-08/msg00234.html> of > > five configurations I think are relevant to cover the different > > cases in patches such as this ("new" = 5.1 or later; Florian's > > changes to provide syscall tables in glibc would allow "#ifdef > > __NR_clock_settime64" to be removed and eliminate case (b)): > > > > (a) one that has always had 64-bit time, e.g. x86_64; > > > > Point IV. For this point I've only used make && make xcheck > > > (b) one with 32-bit time and old kernel headers (any kernel version > > at runtime); > > Point II. > > > > > (c) one with 32-bit time and new kernel headers, old kernel at > > runtime; > > > > (d) one with 32-bit time and new kernel headers, new kernel at > > runtime but no --enable-kernel; Point c) and d) test scenario as described: https://github.com/lmajewski/meta-y2038 > > > > (e) one with 32-bit time and new kernel at runtime and new > > --enable-kernel. > > Point II. > > > > > I will double check if the above points are in sync with points a) to > e). > > > If some of these are problematic to test, you can ask for help > > testing them. For *this particular* patch you might not need to > > test both (c) and (d), because they are identical as far as > > compilation is concerned and the testsuite doesn't really cover > > execution of clock_settime anyway. But it's in the nature of the > > Y2038 changes - involving lots of conditional code - that it's > > necessary to test in several different configurations to cover the > > conditionals adequately. > > QEMU (and OE/Yocto) helps here ... > > > > > (For avoidance of doubt, it is *not* necessary to run > > build-many-glibcs.py for this patch, and nor would running > > build-many-glibcs.py be sufficient since it doesn't do execution > > testing or cover the kernel headers version and --enable-kernel > > variants.) > > > > The build-many-glibcs helps with checking if there aren't some odd > build breaks. It is one of many test approaches for testing the Y2038 > problem. > > Note: > > [1] - https://github.com/lmajewski/meta-y2038 > [2] - > https://github.com/lmajewski/y2038_glibc/commit/1229b54508d0bb130a017a5b5591209167255665 > [3] - https://github.com/lmajewski/y2038-tests/commits/master > > Best regards, > > Lukasz Majewski > > -- > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: > lukma@denx.de Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff --git a/include/time.h b/include/time.h index 64b5468115..91f6280eb4 100644 --- a/include/time.h +++ b/include/time.h @@ -124,6 +124,14 @@ extern __time64_t __timegm64 (struct tm *__tp) __THROW; libc_hidden_proto (__timegm64) #endif +#if __TIMESIZE == 64 +# define __clock_settime64 __clock_settime +#else +extern int __clock_settime64 (clockid_t clock_id, + const struct __timespec64 *tp); +libc_hidden_proto (__clock_settime64) +#endif + /* Compute the `struct tm' representation of T, offset OFFSET seconds east of UTC, and store year, yday, mon, mday, wday, hour, min, sec into *TP. diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c index f76178e0f6..1295bb5603 100644 --- a/sysdeps/unix/sysv/linux/clock_settime.c +++ b/sysdeps/unix/sysv/linux/clock_settime.c @@ -20,11 +20,9 @@ #include <time.h> #include <shlib-compat.h> -#include "kernel-posix-cpu-timers.h" - /* Set CLOCK to value TP. */ int -__clock_settime (clockid_t clock_id, const struct timespec *tp) +__clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp) { /* Make sure the time cvalue is OK. */ if (tp->tv_nsec < 0 || tp->tv_nsec >= 1000000000) @@ -33,8 +31,40 @@ __clock_settime (clockid_t clock_id, const struct timespec *tp) return -1; } - return INLINE_SYSCALL_CALL (clock_settime, clock_id, tp); +#ifdef __ASSUME_TIME64_SYSCALLS +# ifndef __NR_clock_settime64 +# define __NR_clock_settime64 __NR_clock_settime +# endif + return INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp); +#else +# ifdef __NR_clock_settime64 + int ret = INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp); + if (ret == 0 || errno != ENOSYS) + return ret; +# endif + if (! in_time_t_range (tp->tv_sec)) + { + __set_errno (EOVERFLOW); + return -1; + } + + struct timespec ts32; + valid_timespec64_to_timespec (tp, &ts32); + return INLINE_SYSCALL_CALL (clock_settime, clock_id, &ts32); +#endif } + +#if __TIMESIZE != 64 +int +__clock_settime (clockid_t clock_id, const struct timespec *tp) +{ + struct __timespec64 ts64; + + valid_timespec_to_timespec64 (tp, &ts64); + return __clock_settime64 (clock_id, &ts64); +} +#endif + libc_hidden_def (__clock_settime) versioned_symbol (libc, __clock_settime, clock_settime, GLIBC_2_17);