On Mon, 2021-03-08 at 19:35 +0100, Jan Kiszka wrote: > On 08.03.21 18:02, Florian Bezdeka wrote: > > Introducing a new smokey plugin that can be extended for all kind of > > y2038 tests. > > And what does this version test? Some more words on how the new syscall > is stressed would be valuable here.
Yes. Will do. > > > > > Signed-off-by: Florian Bezdeka <[email protected]> > > --- > > configure.ac | 1 + > > testsuite/smokey/Makefile.am | 6 +- > > testsuite/smokey/y2038/Makefile.am | 10 ++ > > testsuite/smokey/y2038/syscall-tests.c | 170 +++++++++++++++++++++++++ > > 4 files changed, 185 insertions(+), 2 deletions(-) > > create mode 100644 testsuite/smokey/y2038/Makefile.am > > create mode 100644 testsuite/smokey/y2038/syscall-tests.c > > > > diff --git a/configure.ac b/configure.ac > > index abe538dbd..bd5fd5ba9 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -990,6 +990,7 @@ AC_CONFIG_FILES([ \ > > testsuite/smokey/net_common/Makefile \ > > testsuite/smokey/cpu-affinity/Makefile \ > > testsuite/smokey/gdb/Makefile \ > > + testsuite/smokey/y2038/Makefile \ > > testsuite/clocktest/Makefile \ > > testsuite/xeno-test/Makefile \ > > utils/Makefile \ > > diff --git a/testsuite/smokey/Makefile.am b/testsuite/smokey/Makefile.am > > index 02613c7dc..56c873026 100644 > > --- a/testsuite/smokey/Makefile.am > > +++ b/testsuite/smokey/Makefile.am > > @@ -38,7 +38,8 @@ COBALT_SUBDIRS = \ > > timerfd \ > > tsc \ > > vdso-access \ > > - xddp > > + xddp \ > > + y2038 > > > > > > > > > > > > > > > > > > MERCURY_SUBDIRS = \ > > memory-heapmem \ > > @@ -76,7 +77,8 @@ DIST_SUBDIRS = \ > > timerfd \ > > tsc \ > > vdso-access \ > > - xddp > > + xddp \ > > + y2038 > > > > > > > > > > > > > > > > > > if XENO_COBALT > > if CONFIG_XENO_LIBS_DLOPEN > > diff --git a/testsuite/smokey/y2038/Makefile.am > > b/testsuite/smokey/y2038/Makefile.am > > new file mode 100644 > > index 000000000..a97591aa2 > > --- /dev/null > > +++ b/testsuite/smokey/y2038/Makefile.am > > @@ -0,0 +1,10 @@ > > + > > +noinst_LIBRARIES = liby2038.a > > + > > +liby2038_a_SOURCES = syscall-tests.c > > + > > +liby2038_a_CPPFLAGS = \ > > + @XENO_USER_CFLAGS@ \ > > + -I$(top_srcdir) \ > > + -I$(top_srcdir)/include > > + > > diff --git a/testsuite/smokey/y2038/syscall-tests.c > > b/testsuite/smokey/y2038/syscall-tests.c > > new file mode 100644 > > index 000000000..0fc96374f > > --- /dev/null > > +++ b/testsuite/smokey/y2038/syscall-tests.c > > @@ -0,0 +1,170 @@ > > +/* > > + * y2038 tests > > + * > > + * Copyright (c) Siemens AG 2021 > > + * > > + * Authors: > > + * Florian Bezdeka <[email protected]> > > + * > > + * Released under the terms of GPLv2. > > + */ > > +#include <cobalt/uapi/syscall.h> > > +#include <smokey/smokey.h> > > +#include <semaphore.h> > > +#include <unistd.h> > > +#include <stdint.h> > > +#include <errno.h> > > + > > +smokey_test_plugin(y2038, SMOKEY_NOARGS, "Validate correct y2038 support"); > > + > > +/* > > + * libc independent data type representing a time64_t based struct timespec > > + */ > > +struct xn_timespec64 { > > + int64_t tv_sec; > > + int64_t tv_nsec; > > +}; > > + > > +#define NSEC_PER_SEC 1000000000 > > + > > +static void ts_normalise(struct xn_timespec64 *ts) > > +{ > > + while (ts->tv_nsec >= NSEC_PER_SEC) { > > + ts->tv_nsec += 1; > > + ts->tv_nsec -= NSEC_PER_SEC; > > + } > > + > > + while (ts->tv_nsec <= -NSEC_PER_SEC) { > > + ts->tv_sec -= 1; > > + ts->tv_nsec += NSEC_PER_SEC; > > + } > > + > > + if (ts->tv_nsec < 0) { > > + /* > > + * Negative nanoseconds isn't valid according to POSIX. > > + * Decrement tv_sec and roll tv_nsec over. > > + */ > > + ts->tv_sec -= 1; > > + ts->tv_nsec = (NSEC_PER_SEC + ts->tv_nsec); > > + } > > +} > > + > > +static inline void ts_add_ns(struct xn_timespec64 *ts, int ns) > > +{ > > + ts->tv_nsec += ns; > > + ts_normalise(ts); > > +} > > + > > +/** > > + * Compare two struct timespec instances > > + * > > + * @param a > > + * @param b > > + * @return True if a < b, false otherwise > > + */ > > +static inline bool ts_less(const struct xn_timespec64 *a, > > + const struct xn_timespec64 *b) > > +{ > > + if (a->tv_sec < b->tv_sec) > > + return true; > > + > > + if (a->tv_sec > b->tv_sec) > > + return false; > > + > > + /* a->tv_sec == b->tv_sec */ > > + > > + if (a->tv_nsec < b->tv_nsec) > > + return true; > > + > > + return false; > > +} > > + > > +static int test_sc_cobalt_sem_timedwait_time64(void) > > +{ > > + long ret; > > + sem_t sem; > > + int code = __xn_syscode(sc_cobalt_sem_timedwait_time64); > > + struct xn_timespec64 ts64, ts_wu; > > + struct timespec ts_nat; > > + > > + sem_init(&sem, 0, 0); > > + > > + /* > > + * TODO: Calling the syscall directly for now, as y2038 support is not > > + * yet available in glibc. Once this is available we should use our > > + * glibc wrappers provided by libcobalt. > > + */ > > Maybe wrap that, to make the invocations more readable? Maybe a macro > can do that in a way that will not affect the code once glibc is ready. Not sure if that is possible, but will think about that. We shouldn't limit our implementation to glibc only, there are other implementations like musl out there as well. The idea was to test the libcobalt libc wrappers here as well, but maybe that is not an good idea. Even glibc has the y2038 in the future it may not be activated by user code... So calling the syscall directly might be right thing to do here, and just remove the TODO comment. > > > + > > + /* Make sure we don't crash because of NULL pointers */ > > + ret = syscall(code, NULL, NULL); > > + if (ret == -1 && errno == ENOSYS) > > + return 0; // Not implemented, nothing to test, success > > + if (!smokey_assert(ret == -1) || !smokey_assert(errno == EINVAL)) > > + return errno; > > + > > + /* Timeout is never read by the kernel, so NULL should be OK */ > > + sem_post(&sem); > > + ret = syscall(code, &sem, NULL); > > + if (!smokey_assert(!ret)) > > + return errno; > > + > > + /* > > + * The semaphore is already exhausted, so calling again will validate > > + * the provided timeout now. Providing NULL has to deliver EFAULT > > + */ > > + ret = syscall(code, &sem, NULL); > > + if (!smokey_assert(ret == -1) || !smokey_assert(errno == EFAULT)) > > + return errno; > > + > > + /* > > + * The semaphore is still exhausted, calling again will validate the > > + * timeout, providing an invalid timeout has to deliver EINVAL > > + */ > > + ts64.tv_sec = -1; > > + ret = syscall(code, &sem, &ts64); > > + if (!smokey_assert(ret == -1) || !smokey_assert(errno == EINVAL)) > > + return errno; > > + > > + /* > > + * Providing a valid timeout, waiting for it to time out and check > > + * that we didn't come back to early. > > + */ > > + ret = clock_gettime(CLOCK_MONOTONIC, &ts_nat); > > + if (ret) > > + return errno; > > + > > + ts64.tv_sec = ts_nat.tv_sec; > > + ts64.tv_nsec = ts_nat.tv_nsec; > > + ts_add_ns(&ts64, 500000); > > + > > + ret = syscall(code, &sem, &ts64); > > + if (!smokey_assert(ret == -1) || !smokey_assert(errno == ETIMEDOUT)) > > + return errno; > > + > > + ret = clock_gettime(CLOCK_MONOTONIC, &ts_nat); > > + if (ret) > > + return errno; > > + > > + ts_wu.tv_sec = ts_nat.tv_sec; > > + ts_wu.tv_nsec = ts_nat.tv_nsec; > > + > > + if (ts_less(&ts_wu, &ts64)) > > + smokey_warning("sem_timedwait_time64 returned to early!\n" > > + "Expected wakeup at: %lld sec %lld nsec\n" > > + "Back at : %lld sec %lld nsec\n", > > + ts64.tv_sec, ts64.tv_nsec, ts_wu.tv_sec, > > + ts_wu.tv_nsec); > > + > > Does this already stress the 64-bit range of time_t? I don't spot that yet. > > > + return 0; > > +} > > + > > +static int run_y2038(struct smokey_test *t, int argc, char *const argv[]) > > +{ > > + int ret; > > + > > + ret = test_sc_cobalt_sem_timedwait_time64(); > > + if (ret) > > + return ret; > > + > > + return 0; > > +} > > > > Such a test is obviously key to approach the issue correctly. It's a > complex beast. > > Jan >
