[v2] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437)

  Although the kernel ABI uses 64-bit (so it can re-use the syscall
logic from x86_64), POSIX states the type to be 'long int'.  It
requires to explicit signal extend the value on syscalls that
take the value to avoid requiring bumping the minimum kernel version
(ea2ce8f3514e ("time: Fix get_timespec64() for y2038 safe compat
interfaces") got merged into v4.18, futex and recvmsg followed in v5.1,
but v5.6 is considered 'complete').

The fix uses 'long long int' array to represent 'struct timespec',
since it allows a simpler code than define a kernel specific timespec.

There is no need to compat symbol, the now high-bits are not used
to represent the nanoseconds and for some syscalls it only issues
an error (which hardly indicates the need of compat symbol).

Checked on x86_64-linux-gnu-x32 with kernel 4.4.0 and 5.13.
---
Changes from v1:
 * Remove the usage of zero width bitfields.
---
 conform/data/signal.h-data                  |  3 +-
 conform/data/sys/select.h-data              |  3 +-
 conform/data/sys/stat.h-data                |  3 +-
 conform/data/time.h-data                    |  3 +-
 nptl/futex-internal.c                       | 10 +++++--
 sysdeps/unix/sysv/linux/clock_nanosleep.c   |  3 +-
 sysdeps/unix/sysv/linux/clock_settime.c     |  3 +-
 sysdeps/unix/sysv/linux/mq_timedreceive.c   |  8 +++++-
 sysdeps/unix/sysv/linux/mq_timedsend.c      |  8 +++++-
 sysdeps/unix/sysv/linux/ppoll.c             | 13 ++++-----
 sysdeps/unix/sysv/linux/pselect.c           | 19 ++++++-------
 sysdeps/unix/sysv/linux/recvmmsg.c          |  8 +++++-
 sysdeps/unix/sysv/linux/select.c            | 31 ++++++++++-----------
 sysdeps/unix/sysv/linux/semtimedop.c        | 10 +++++--
 sysdeps/unix/sysv/linux/sigtimedwait.c      | 10 +++++--
 sysdeps/unix/sysv/linux/timer_settime.c     |  7 +++--
 sysdeps/unix/sysv/linux/timerfd_settime.c   |  6 +++-
 sysdeps/unix/sysv/linux/utimensat.c         | 19 +++++++++----
 sysdeps/unix/sysv/linux/x86_64/x32/Makefile |  2 +-
 time/bits/types/struct_timespec.h           | 24 ++++++----------
 20 files changed, 116 insertions(+), 77 deletions(-)
  

On Fri, Dec 10, 2021 at 3:03 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Although the kernel ABI uses 64-bit (so it can re-use the syscall
> logic from x86_64), POSIX states the type to be 'long int'.  It
> requires to explicit signal extend the value on syscalls that
> take the value to avoid requiring bumping the minimum kernel version
> (ea2ce8f3514e ("time: Fix get_timespec64() for y2038 safe compat
> interfaces") got merged into v4.18, futex and recvmsg followed in v5.1,
> but v5.6 is considered 'complete').
>
> The fix uses 'long long int' array to represent 'struct timespec',
> since it allows a simpler code than define a kernel specific timespec.
>
> There is no need to compat symbol, the now high-bits are not used
> to represent the nanoseconds and for some syscalls it only issues
> an error (which hardly indicates the need of compat symbol).
>
> Checked on x86_64-linux-gnu-x32 with kernel 4.4.0 and 5.13.

Isn't it an ABI change for x32?

> ---
> Changes from v1:
>  * Remove the usage of zero width bitfields.
> ---
>  conform/data/signal.h-data                  |  3 +-
>  conform/data/sys/select.h-data              |  3 +-
>  conform/data/sys/stat.h-data                |  3 +-
>  conform/data/time.h-data                    |  3 +-
>  nptl/futex-internal.c                       | 10 +++++--
>  sysdeps/unix/sysv/linux/clock_nanosleep.c   |  3 +-
>  sysdeps/unix/sysv/linux/clock_settime.c     |  3 +-
>  sysdeps/unix/sysv/linux/mq_timedreceive.c   |  8 +++++-
>  sysdeps/unix/sysv/linux/mq_timedsend.c      |  8 +++++-
>  sysdeps/unix/sysv/linux/ppoll.c             | 13 ++++-----
>  sysdeps/unix/sysv/linux/pselect.c           | 19 ++++++-------
>  sysdeps/unix/sysv/linux/recvmmsg.c          |  8 +++++-
>  sysdeps/unix/sysv/linux/select.c            | 31 ++++++++++-----------
>  sysdeps/unix/sysv/linux/semtimedop.c        | 10 +++++--
>  sysdeps/unix/sysv/linux/sigtimedwait.c      | 10 +++++--
>  sysdeps/unix/sysv/linux/timer_settime.c     |  7 +++--
>  sysdeps/unix/sysv/linux/timerfd_settime.c   |  6 +++-
>  sysdeps/unix/sysv/linux/utimensat.c         | 19 +++++++++----
>  sysdeps/unix/sysv/linux/x86_64/x32/Makefile |  2 +-
>  time/bits/types/struct_timespec.h           | 24 ++++++----------
>  20 files changed, 116 insertions(+), 77 deletions(-)
>
> diff --git a/conform/data/signal.h-data b/conform/data/signal.h-data
> index 674e5793db..d987affa28 100644
> --- a/conform/data/signal.h-data
> +++ b/conform/data/signal.h-data
> @@ -32,8 +32,7 @@ xfail[powerpc32-linux]-element ucontext_t mcontext_t uc_mcontext
>
>  type {struct timespec}
>  element {struct timespec} __time_t tv_sec
> -// Bug 16437: tv_nsec has wrong type.
> -xfail[x86_64-x32-linux]-element {struct timespec} long tv_nsec
> +element {struct timespec} long tv_nsec
>  #endif
>
>  #if defined POSIX || defined UNIX98 || defined XOPEN2K || defined XOPEN2K8 || defined POSIX2008
> diff --git a/conform/data/sys/select.h-data b/conform/data/sys/select.h-data
> index 44d63ebd2d..16e66b1527 100644
> --- a/conform/data/sys/select.h-data
> +++ b/conform/data/sys/select.h-data
> @@ -10,8 +10,7 @@ type sigset_t
>
>  type {struct timespec}
>  element {struct timespec} time_t tv_sec
> -// Bug 16437: tv_nsec has wrong type.
> -xfail[x86_64-x32-linux]-element {struct timespec} long tv_nsec
> +element {struct timespec} long tv_nsec
>
>  type fd_set
>  #if defined XPG4 || defined XPG42 || defined UNIX98
> diff --git a/conform/data/sys/stat.h-data b/conform/data/sys/stat.h-data
> index 03be4814ec..4eb1434ab1 100644
> --- a/conform/data/sys/stat.h-data
> +++ b/conform/data/sys/stat.h-data
> @@ -63,8 +63,7 @@ element {struct stat} blkcnt_t st_blocks
>  # if defined XOPEN2K8 || defined POSIX2008
>  type {struct timespec}
>  element {struct timespec} time_t tv_sec
> -// Bug 16437: tv_nsec has wrong type.
> -xfail[x86_64-x32-linux]-element {struct timespec} long tv_nsec
> +element {struct timespec} long tv_nsec
>  # endif
>
>  #if !defined POSIX && !defined POSIX2008
> diff --git a/conform/data/time.h-data b/conform/data/time.h-data
> index 9c1c19596e..d4cb6514f5 100644
> --- a/conform/data/time.h-data
> +++ b/conform/data/time.h-data
> @@ -9,8 +9,7 @@ macro-int-constant TIME_UTC > 0
>  type {struct timespec}
>
>  element {struct timespec} time_t tv_sec
> -// Bug 16437: tv_nsec has wrong type.
> -xfail[x86_64-x32-linux]-element {struct timespec} long tv_nsec
> +element {struct timespec} long tv_nsec
>  #endif
>
>  type size_t
> diff --git a/nptl/futex-internal.c b/nptl/futex-internal.c
> index 58605b2fca..050f38884b 100644
> --- a/nptl/futex-internal.c
> +++ b/nptl/futex-internal.c
> @@ -53,13 +53,19 @@ __futex_abstimed_wait_common64 (unsigned int* futex_word,
>                                  const struct __timespec64* abstime,
>                                  int private, bool cancel)
>  {
> +  long long int ts[2];
> +  if (abstime != NULL)
> +    {
> +      ts[0] = abstime->tv_sec;
> +      ts[1] = abstime->tv_nsec;
> +    }
>    if (cancel)
>      return INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op, expected,
> -                                   abstime, NULL /* Unused.  */,
> +                                   abstime != NULL ? ts : NULL, NULL,
>                                     FUTEX_BITSET_MATCH_ANY);
>    else
>      return INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op, expected,
> -                                 abstime, NULL /* Ununsed.  */,
> +                                 abstime != NULL ? ts : NULL, NULL,
>                                   FUTEX_BITSET_MATCH_ANY);
>  }
>
> diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c
> index 74e2407575..09ae954360 100644
> --- a/sysdeps/unix/sysv/linux/clock_nanosleep.c
> +++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c
> @@ -45,7 +45,8 @@ __clock_nanosleep_time64 (clockid_t clock_id, int flags,
>
>    int r;
>  #ifdef __ASSUME_TIME64_SYSCALLS
> -  r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, clock_id, flags, req,
> +  long long int ts[2] = { req->tv_sec, req->tv_nsec };
> +  r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, clock_id, flags, &ts,
>                                rem);
>  #else
>    if (!in_time_t_range (req->tv_sec))
> diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
> index 598d72b8b1..c57ae06311 100644
> --- a/sysdeps/unix/sysv/linux/clock_settime.c
> +++ b/sysdeps/unix/sysv/linux/clock_settime.c
> @@ -35,7 +35,8 @@ __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp)
>  #ifndef __NR_clock_settime64
>  # define __NR_clock_settime64 __NR_clock_settime
>  #endif
> -  int ret = INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp);
> +  long long int ts[2] = { tp->tv_sec, tp->tv_nsec };
> +  int ret = INLINE_SYSCALL_CALL (clock_settime64, clock_id, ts);
>
>  #ifndef __ASSUME_TIME64_SYSCALLS
>    if (ret == 0 || errno != ENOSYS)
> diff --git a/sysdeps/unix/sysv/linux/mq_timedreceive.c b/sysdeps/unix/sysv/linux/mq_timedreceive.c
> index 7f3a112d7f..be7dc56c8b 100644
> --- a/sysdeps/unix/sysv/linux/mq_timedreceive.c
> +++ b/sysdeps/unix/sysv/linux/mq_timedreceive.c
> @@ -32,8 +32,14 @@ ___mq_timedreceive_time64 (mqd_t mqdes, char *__restrict msg_ptr, size_t msg_len
>  #endif
>
>  #ifdef __ASSUME_TIME64_SYSCALLS
> +  long long int ts[2];
> +  if (abs_timeout != NULL)
> +    {
> +      ts[0] = abs_timeout->tv_sec;
> +      ts[1] = abs_timeout->tv_nsec;
> +    }
>    return SYSCALL_CANCEL (mq_timedreceive_time64, mqdes, msg_ptr, msg_len,
> -                        msg_prio, abs_timeout);
> +                        msg_prio, abs_timeout != NULL ? ts : NULL);
>  #else
>    bool need_time64 = abs_timeout != NULL
>                      && !in_time_t_range (abs_timeout->tv_sec);
> diff --git a/sysdeps/unix/sysv/linux/mq_timedsend.c b/sysdeps/unix/sysv/linux/mq_timedsend.c
> index f93f865721..dcbb1a00d5 100644
> --- a/sysdeps/unix/sysv/linux/mq_timedsend.c
> +++ b/sysdeps/unix/sysv/linux/mq_timedsend.c
> @@ -32,8 +32,14 @@ ___mq_timedsend_time64 (mqd_t mqdes, const char *msg_ptr, size_t msg_len,
>  # endif
>
>  #ifdef __ASSUME_TIME64_SYSCALLS
> +  long long int ts[2];
> +  if (abs_timeout != NULL)
> +    {
> +      ts[0] = abs_timeout->tv_sec;
> +      ts[1] = abs_timeout->tv_nsec;
> +    }
>    return SYSCALL_CANCEL (mq_timedsend_time64, mqdes, msg_ptr, msg_len,
> -                        msg_prio, abs_timeout);
> +                        msg_prio, abs_timeout != NULL ? ts : NULL);
>  #else
>    bool need_time64 = abs_timeout != NULL
>                      && !in_time_t_range (abs_timeout->tv_sec);
> diff --git a/sysdeps/unix/sysv/linux/ppoll.c b/sysdeps/unix/sysv/linux/ppoll.c
> index 465dea623d..d01379ba2d 100644
> --- a/sysdeps/unix/sysv/linux/ppoll.c
> +++ b/sysdeps/unix/sysv/linux/ppoll.c
> @@ -27,11 +27,11 @@ __ppoll64 (struct pollfd *fds, nfds_t nfds, const struct __timespec64 *timeout,
>  {
>    /* The Linux kernel can in some situations update the timeout value.
>       We do not want that so use a local variable.  */
> -  struct __timespec64 tval;
> +  long long int ts[2];
>    if (timeout != NULL)
>      {
> -      tval = *timeout;
> -      timeout = &tval;
> +      ts[0] = timeout->tv_sec;
> +      ts[1] = timeout->tv_nsec;
>      }
>
>  #ifndef __NR_ppoll_time64
> @@ -39,15 +39,14 @@ __ppoll64 (struct pollfd *fds, nfds_t nfds, const struct __timespec64 *timeout,
>  #endif
>
>  #ifdef __ASSUME_TIME64_SYSCALLS
> -  return SYSCALL_CANCEL (ppoll_time64, fds, nfds, timeout, sigmask,
> -                        __NSIG_BYTES);
> +  return SYSCALL_CANCEL (ppoll_time64, fds, nfds, timeout != NULL ? ts : NULL,
> +                        sigmask, __NSIG_BYTES);
>  #else
>    int ret;
>    bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec);
>    if (need_time64)
>      {
> -      ret = SYSCALL_CANCEL (ppoll_time64, fds, nfds, timeout, sigmask,
> -                           __NSIG_BYTES);
> +      ret = SYSCALL_CANCEL (ppoll_time64, fds, nfds, ts, sigmask, __NSIG_BYTES);
>        if (ret == 0 || errno != ENOSYS)
>         return ret;
>        __set_errno (EOVERFLOW);
> diff --git a/sysdeps/unix/sysv/linux/pselect.c b/sysdeps/unix/sysv/linux/pselect.c
> index f58c01f25a..8cf22c8aa4 100644
> --- a/sysdeps/unix/sysv/linux/pselect.c
> +++ b/sysdeps/unix/sysv/linux/pselect.c
> @@ -31,23 +31,22 @@ pselect64_syscall (int nfds, fd_set *readfds, fd_set *writefds,
>      {
>        (uintptr_t) sigmask, __NSIG_BYTES
>      };
> +  /* The Linux kernel can in some situations update the timeout value.
> +     We do not want that so use a local variable.  */
> +  long long int ts[2];
> +  if (timeout != NULL)
> +    {
> +      ts[0] = timeout->tv_sec;
> +      ts[1] = timeout->tv_nsec;
> +    }
>    return SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
> -                        timeout, data);
> +                        timeout != NULL ? ts : NULL, data);
>  }
>
>  int
>  __pselect64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>              const struct __timespec64 *timeout, const sigset_t *sigmask)
>  {
> -  /* The Linux kernel can in some situations update the timeout value.
> -     We do not want that so use a local variable.  */
> -  struct __timespec64 tval;
> -  if (timeout != NULL)
> -    {
> -      tval = *timeout;
> -      timeout = &tval;
> -    }
> -
>    /* Note: the system call expects 7 values but on most architectures
>       we can only pass in 6 directly.  If there is an architecture with
>       support for more parameters a new version of this file needs to
> diff --git a/sysdeps/unix/sysv/linux/recvmmsg.c b/sysdeps/unix/sysv/linux/recvmmsg.c
> index c214321e94..8d6cd238eb 100644
> --- a/sysdeps/unix/sysv/linux/recvmmsg.c
> +++ b/sysdeps/unix/sysv/linux/recvmmsg.c
> @@ -26,8 +26,14 @@ __recvmmsg64 (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags,
>  #ifndef __NR_recvmmsg_time64
>  # define __NR_recvmmsg_time64 __NR_recvmmsg
>  #endif
> +  long long int ts[2];
> +  if (timeout != NULL)
> +    {
> +      ts[0] = timeout->tv_sec;
> +      ts[1] = timeout->tv_nsec;
> +    }
>    int r = SYSCALL_CANCEL (recvmmsg_time64, fd, vmessages, vlen, flags,
> -                         timeout);
> +                         timeout != NULL ? ts : NULL);
>  #ifndef __ASSUME_TIME64_SYSCALLS
>    if (r >= 0 || errno != ENOSYS)
>      return r;
> diff --git a/sysdeps/unix/sysv/linux/select.c b/sysdeps/unix/sysv/linux/select.c
> index da25b4b4cf..dd47238572 100644
> --- a/sysdeps/unix/sysv/linux/select.c
> +++ b/sysdeps/unix/sysv/linux/select.c
> @@ -53,13 +53,7 @@ __select64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>        ns = us * NSEC_PER_USEC;
>      }
>
> -  struct __timespec64 ts64, *pts64 = NULL;
> -   if (timeout != NULL)
> -     {
> -       ts64.tv_sec = s;
> -       ts64.tv_nsec = ns;
> -       pts64 = &ts64;
> -     }
> +  long long int ts[2] = { s, ns };
>
>  #ifndef __NR_pselect6_time64
>  # define __NR_pselect6_time64 __NR_pselect6
> @@ -67,19 +61,23 @@ __select64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>
>  #ifdef __ASSUME_TIME64_SYSCALLS
>    int r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
> -                         pts64, NULL);
> +                         timeout != NULL ? ts : NULL, NULL);
>    if (timeout != NULL)
> -    TIMESPEC_TO_TIMEVAL (timeout, pts64);
> +    {
> +      timeout->tv_sec = ts[0];
> +      timeout->tv_usec = ts[1] / 1000;
> +    }
>    return r;
>  #else
>    bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec);
>    if (need_time64)
>      {
>        int r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds,
> -                             exceptfds, pts64, NULL);
> +                             exceptfds, ts, NULL);
>        if ((r >= 0 || errno != ENOSYS) && timeout != NULL)
>         {
> -         TIMESPEC_TO_TIMEVAL (timeout, &ts64);
> +         timeout->tv_sec = ts[0];
> +         timeout->tv_usec = ts[1] / 1000;
>         }
>        else
>         __set_errno (EOVERFLOW);
> @@ -88,10 +86,10 @@ __select64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>
>  # ifdef __ASSUME_PSELECT
>    struct timespec ts32, *pts32 = NULL;
> -  if (pts64 != NULL)
> +  if (timeout != NULL)
>      {
> -      ts32.tv_sec = pts64->tv_sec;
> -      ts32.tv_nsec = pts64->tv_nsec;
> +      ts32.tv_sec = ts[0];
> +      ts32.tv_nsec = ts[1];
>        pts32 = &ts32;
>      }
>
> @@ -102,9 +100,10 @@ __select64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>    return r;
>  # else
>    struct timeval tv32, *ptv32 = NULL;
> -  if (pts64 != NULL)
> +  if (timeout != NULL)
>      {
> -      tv32 = valid_timespec64_to_timeval (*pts64);
> +      tv32.tv_sec = ts[0];
> +      tv32.tv_usec = ts[1];
>        ptv32 = &tv32;
>      }
>
> diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c
> index 43577535ef..5816577a9f 100644
> --- a/sysdeps/unix/sysv/linux/semtimedop.c
> +++ b/sysdeps/unix/sysv/linux/semtimedop.c
> @@ -22,7 +22,7 @@
>
>  static int
>  semtimedop_syscall (int semid, struct sembuf *sops, size_t nsops,
> -                   const struct __timespec64 *timeout)
> +                   const void *timeout)
>  {
>  #ifdef __NR_semtimedop_time64
>    return INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, timeout);
> @@ -40,7 +40,13 @@ __semtimedop64 (int semid, struct sembuf *sops, size_t nsops,
>                 const struct __timespec64 *timeout)
>  {
>  #ifdef __ASSUME_TIME64_SYSCALLS
> -  return semtimedop_syscall (semid, sops, nsops, timeout);
> +  long long int ts[2];
> +  if (timeout != NULL)
> +    {
> +      ts[0] = timeout->tv_sec;
> +      ts[1] = timeout->tv_nsec;
> +    }
> +  return semtimedop_syscall (semid, sops, nsops, timeout != NULL ? ts : NULL);
>  #else
>    bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec);
>    if (need_time64)
> diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c
> index 607381a0a7..f08cc688db 100644
> --- a/sysdeps/unix/sysv/linux/sigtimedwait.c
> +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c
> @@ -28,8 +28,14 @@ __sigtimedwait64 (const sigset_t *set, siginfo_t *info,
>
>    int result;
>  #ifdef __ASSUME_TIME64_SYSCALLS
> -  result = SYSCALL_CANCEL (rt_sigtimedwait_time64, set, info, timeout,
> -                          __NSIG_BYTES);
> +  long long int ts[2];
> +  if (timeout != NULL)
> +    {
> +      ts[0] = timeout->tv_sec;
> +      ts[1] = timeout->tv_nsec;
> +    }
> +  result = SYSCALL_CANCEL (rt_sigtimedwait_time64, set, info,
> +                          timeout != NULL ? ts : NULL, __NSIG_BYTES);
>  #else
>    bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec);
>    if (need_time64)
> diff --git a/sysdeps/unix/sysv/linux/timer_settime.c b/sysdeps/unix/sysv/linux/timer_settime.c
> index 24706e45d8..86199ebac5 100644
> --- a/sysdeps/unix/sysv/linux/timer_settime.c
> +++ b/sysdeps/unix/sysv/linux/timer_settime.c
> @@ -35,8 +35,11 @@ ___timer_settime64 (timer_t timerid, int flags,
>  #  ifndef __NR_timer_settime64
>  #   define __NR_timer_settime64 __NR_timer_settime
>  #  endif
> -  return INLINE_SYSCALL_CALL (timer_settime64, ktimerid, flags, value,
> -                              ovalue);
> +  long long int ts[4] = { value->it_interval.tv_sec,
> +                         value->it_interval.tv_nsec,
> +                         value->it_value.tv_sec,
> +                         value->it_value.tv_nsec };
> +  return INLINE_SYSCALL_CALL (timer_settime64, ktimerid, flags, ts, ovalue);
>  # else
>  #  ifdef __NR_timer_settime64
>    int ret = INLINE_SYSCALL_CALL (timer_settime64, ktimerid, flags, value,
> diff --git a/sysdeps/unix/sysv/linux/timerfd_settime.c b/sysdeps/unix/sysv/linux/timerfd_settime.c
> index 0069e1374a..709d54d37c 100644
> --- a/sysdeps/unix/sysv/linux/timerfd_settime.c
> +++ b/sysdeps/unix/sysv/linux/timerfd_settime.c
> @@ -31,7 +31,11 @@ __timerfd_settime64 (int fd, int flags, const struct __itimerspec64 *value,
>  #endif
>
>  #ifdef __ASSUME_TIME64_SYSCALLS
> -  return INLINE_SYSCALL_CALL (timerfd_settime64, fd, flags, value, ovalue);
> +  long long int ts[4] = { value->it_interval.tv_sec,
> +                         value->it_interval.tv_nsec,
> +                         value->it_value.tv_sec,
> +                         value->it_value.tv_nsec };
> +  return INLINE_SYSCALL_CALL (timerfd_settime64, fd, flags, ts, ovalue);
>  #else
>    bool need_time64 = !in_time_t_range (value->it_value.tv_sec)
>                      || !in_time_t_range (value->it_interval.tv_sec);
> diff --git a/sysdeps/unix/sysv/linux/utimensat.c b/sysdeps/unix/sysv/linux/utimensat.c
> index e9061d2323..d64fb46899 100644
> --- a/sysdeps/unix/sysv/linux/utimensat.c
> +++ b/sysdeps/unix/sysv/linux/utimensat.c
> @@ -22,6 +22,10 @@
>  #include <time.h>
>  #include <kernel-features.h>
>
> +/* For UTIME_NOW and UTIME_OMIT the value of tv_sec field is ignored.  */
> +# define TS_SPECIAL(ts) \
> +  ((ts).tv_nsec == UTIME_NOW || (ts).tv_nsec == UTIME_OMIT)
> +
>  /* Helper function defined for easy reusage of the code which calls utimensat
>     and utimensat_time64 syscall.  */
>  int
> @@ -33,12 +37,17 @@ __utimensat64_helper (int fd, const char *file,
>  #endif
>
>  #ifdef __ASSUME_TIME64_SYSCALLS
> -  return INLINE_SYSCALL_CALL (utimensat_time64, fd, file, &tsp64[0], flags);
> +  long long int ts[4];
> +  if (tsp64 != NULL)
> +    {
> +      ts[0] = !TS_SPECIAL(tsp64[0]) ? tsp64[0].tv_sec : 0;
> +      ts[1] = tsp64[0].tv_nsec;
> +      ts[2] = !TS_SPECIAL(tsp64[1]) ? tsp64[1].tv_sec : 0;
> +      ts[3] = tsp64[1].tv_nsec;
> +    }
> +  return INLINE_SYSCALL_CALL (utimensat_time64, fd, file,
> +                             tsp64 != NULL ? ts : NULL, flags);
>  #else
> -  /* For UTIME_NOW and UTIME_OMIT the value of tv_sec field is ignored.  */
> -# define TS_SPECIAL(ts) \
> -  ((ts).tv_nsec == UTIME_NOW || (ts).tv_nsec == UTIME_OMIT)
> -
>    bool need_time64 = tsp64 != NULL
>                      && ((!TS_SPECIAL (tsp64[0])
>                           && !in_time_t_range (tsp64[0].tv_sec))
> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile
> index 16b768d8ba..74b3d30e2f 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile
> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile
> @@ -6,6 +6,6 @@ sysdep_routines += arch_prctl
>  endif
>
>  ifeq ($(subdir),conform)
> -# For bugs 16437 and 21279.
> +# For bug 21279.
>  conformtest-xfail-conds += x86_64-x32-linux
>  endif
> diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h
> index 489e81136d..923f8579e5 100644
> --- a/time/bits/types/struct_timespec.h
> +++ b/time/bits/types/struct_timespec.h
> @@ -10,23 +10,15 @@
>     has nanoseconds instead of microseconds.  */
>  struct timespec
>  {
> -#ifdef __USE_TIME_BITS64
> -  __time64_t tv_sec;           /* Seconds.  */
> -#else
> -  __time_t tv_sec;             /* Seconds.  */
> +  time_t tv_sec;               /* Seconds.  */
> +#if __WORDSIZE == 32 && (__TIMESIZE == 64 || defined __USE_TIME_BITS64) \
> +  && __BYTE_ORDER == __BIG_ENDIAN
> +  int: 32;                     /* Padding.  */
>  #endif
> -#if __WORDSIZE == 64 \
> -  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
> -  || (__TIMESIZE == 32 && !defined __USE_TIME_BITS64)
> -  __syscall_slong_t tv_nsec;   /* Nanoseconds.  */
> -#else
> -# if __BYTE_ORDER == __BIG_ENDIAN
> -  int: 32;           /* Padding.  */
> -  long int tv_nsec;  /* Nanoseconds.  */
> -# else
> -  long int tv_nsec;  /* Nanoseconds.  */
> -  int: 32;           /* Padding.  */
> -# endif
> +  long int tv_nsec;            /* Nanoseconds.  */
> +#if __WORDSIZE == 32 && (__TIMESIZE == 64 || defined __USE_TIME_BITS64) \
> +  && __BYTE_ORDER == __LITTLE_ENDIAN
> +  int: 32;                     /* Padding.  */
>  #endif
>  };
>
> --
> 2.32.0
>
  

* H. J. Lu via Libc-alpha:

> On Fri, Dec 10, 2021 at 3:03 AM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> Although the kernel ABI uses 64-bit (so it can re-use the syscall
>> logic from x86_64), POSIX states the type to be 'long int'.  It
>> requires to explicit signal extend the value on syscalls that
>> take the value to avoid requiring bumping the minimum kernel version
>> (ea2ce8f3514e ("time: Fix get_timespec64() for y2038 safe compat
>> interfaces") got merged into v4.18, futex and recvmsg followed in v5.1,
>> but v5.6 is considered 'complete').
>>
>> The fix uses 'long long int' array to represent 'struct timespec',
>> since it allows a simpler code than define a kernel specific timespec.
>>
>> There is no need to compat symbol, the now high-bits are not used
>> to represent the nanoseconds and for some syscalls it only issues
>> an error (which hardly indicates the need of compat symbol).
>>
>> Checked on x86_64-linux-gnu-x32 with kernel 4.4.0 and 5.13.
>
> Isn't it an ABI change for x32?

I agree.  I said so repeatedly.

Thanks,
Florian
  

On 10/12/2021 12:36, H.J. Lu wrote:
> On Fri, Dec 10, 2021 at 3:03 AM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> Although the kernel ABI uses 64-bit (so it can re-use the syscall
>> logic from x86_64), POSIX states the type to be 'long int'.  It
>> requires to explicit signal extend the value on syscalls that
>> take the value to avoid requiring bumping the minimum kernel version
>> (ea2ce8f3514e ("time: Fix get_timespec64() for y2038 safe compat
>> interfaces") got merged into v4.18, futex and recvmsg followed in v5.1,
>> but v5.6 is considered 'complete').
>>
>> The fix uses 'long long int' array to represent 'struct timespec',
>> since it allows a simpler code than define a kernel specific timespec.
>>
>> There is no need to compat symbol, the now high-bits are not used
>> to represent the nanoseconds and for some syscalls it only issues
>> an error (which hardly indicates the need of compat symbol).
>>
>> Checked on x86_64-linux-gnu-x32 with kernel 4.4.0 and 5.13.
> 
> Isn't it an ABI change for x32?

Strictly speaking yes, although regarding kernel iteraction it won't
matter (kernel will zero the higher bits and glibc will also for
set functions).  It might be a problem is callers operates on timespec
on libraries built with different libcs (assuming compiler might not
initialize higher bits).

If later is a blocker, I can drop the patch and close BZ#16437 as
WONTFIX.

> 
>> ---
>> Changes from v1:
>>  * Remove the usage of zero width bitfields.
>> ---
>>  conform/data/signal.h-data                  |  3 +-
>>  conform/data/sys/select.h-data              |  3 +-
>>  conform/data/sys/stat.h-data                |  3 +-
>>  conform/data/time.h-data                    |  3 +-
>>  nptl/futex-internal.c                       | 10 +++++--
>>  sysdeps/unix/sysv/linux/clock_nanosleep.c   |  3 +-
>>  sysdeps/unix/sysv/linux/clock_settime.c     |  3 +-
>>  sysdeps/unix/sysv/linux/mq_timedreceive.c   |  8 +++++-
>>  sysdeps/unix/sysv/linux/mq_timedsend.c      |  8 +++++-
>>  sysdeps/unix/sysv/linux/ppoll.c             | 13 ++++-----
>>  sysdeps/unix/sysv/linux/pselect.c           | 19 ++++++-------
>>  sysdeps/unix/sysv/linux/recvmmsg.c          |  8 +++++-
>>  sysdeps/unix/sysv/linux/select.c            | 31 ++++++++++-----------
>>  sysdeps/unix/sysv/linux/semtimedop.c        | 10 +++++--
>>  sysdeps/unix/sysv/linux/sigtimedwait.c      | 10 +++++--
>>  sysdeps/unix/sysv/linux/timer_settime.c     |  7 +++--
>>  sysdeps/unix/sysv/linux/timerfd_settime.c   |  6 +++-
>>  sysdeps/unix/sysv/linux/utimensat.c         | 19 +++++++++----
>>  sysdeps/unix/sysv/linux/x86_64/x32/Makefile |  2 +-
>>  time/bits/types/struct_timespec.h           | 24 ++++++----------
>>  20 files changed, 116 insertions(+), 77 deletions(-)
>>
>> diff --git a/conform/data/signal.h-data b/conform/data/signal.h-data
>> index 674e5793db..d987affa28 100644
>> --- a/conform/data/signal.h-data
>> +++ b/conform/data/signal.h-data
>> @@ -32,8 +32,7 @@ xfail[powerpc32-linux]-element ucontext_t mcontext_t uc_mcontext
>>
>>  type {struct timespec}
>>  element {struct timespec} __time_t tv_sec
>> -// Bug 16437: tv_nsec has wrong type.
>> -xfail[x86_64-x32-linux]-element {struct timespec} long tv_nsec
>> +element {struct timespec} long tv_nsec
>>  #endif
>>
>>  #if defined POSIX || defined UNIX98 || defined XOPEN2K || defined XOPEN2K8 || defined POSIX2008
>> diff --git a/conform/data/sys/select.h-data b/conform/data/sys/select.h-data
>> index 44d63ebd2d..16e66b1527 100644
>> --- a/conform/data/sys/select.h-data
>> +++ b/conform/data/sys/select.h-data
>> @@ -10,8 +10,7 @@ type sigset_t
>>
>>  type {struct timespec}
>>  element {struct timespec} time_t tv_sec
>> -// Bug 16437: tv_nsec has wrong type.
>> -xfail[x86_64-x32-linux]-element {struct timespec} long tv_nsec
>> +element {struct timespec} long tv_nsec
>>
>>  type fd_set
>>  #if defined XPG4 || defined XPG42 || defined UNIX98
>> diff --git a/conform/data/sys/stat.h-data b/conform/data/sys/stat.h-data
>> index 03be4814ec..4eb1434ab1 100644
>> --- a/conform/data/sys/stat.h-data
>> +++ b/conform/data/sys/stat.h-data
>> @@ -63,8 +63,7 @@ element {struct stat} blkcnt_t st_blocks
>>  # if defined XOPEN2K8 || defined POSIX2008
>>  type {struct timespec}
>>  element {struct timespec} time_t tv_sec
>> -// Bug 16437: tv_nsec has wrong type.
>> -xfail[x86_64-x32-linux]-element {struct timespec} long tv_nsec
>> +element {struct timespec} long tv_nsec
>>  # endif
>>
>>  #if !defined POSIX && !defined POSIX2008
>> diff --git a/conform/data/time.h-data b/conform/data/time.h-data
>> index 9c1c19596e..d4cb6514f5 100644
>> --- a/conform/data/time.h-data
>> +++ b/conform/data/time.h-data
>> @@ -9,8 +9,7 @@ macro-int-constant TIME_UTC > 0
>>  type {struct timespec}
>>
>>  element {struct timespec} time_t tv_sec
>> -// Bug 16437: tv_nsec has wrong type.
>> -xfail[x86_64-x32-linux]-element {struct timespec} long tv_nsec
>> +element {struct timespec} long tv_nsec
>>  #endif
>>
>>  type size_t
>> diff --git a/nptl/futex-internal.c b/nptl/futex-internal.c
>> index 58605b2fca..050f38884b 100644
>> --- a/nptl/futex-internal.c
>> +++ b/nptl/futex-internal.c
>> @@ -53,13 +53,19 @@ __futex_abstimed_wait_common64 (unsigned int* futex_word,
>>                                  const struct __timespec64* abstime,
>>                                  int private, bool cancel)
>>  {
>> +  long long int ts[2];
>> +  if (abstime != NULL)
>> +    {
>> +      ts[0] = abstime->tv_sec;
>> +      ts[1] = abstime->tv_nsec;
>> +    }
>>    if (cancel)
>>      return INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op, expected,
>> -                                   abstime, NULL /* Unused.  */,
>> +                                   abstime != NULL ? ts : NULL, NULL,
>>                                     FUTEX_BITSET_MATCH_ANY);
>>    else
>>      return INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op, expected,
>> -                                 abstime, NULL /* Ununsed.  */,
>> +                                 abstime != NULL ? ts : NULL, NULL,
>>                                   FUTEX_BITSET_MATCH_ANY);
>>  }
>>
>> diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c
>> index 74e2407575..09ae954360 100644
>> --- a/sysdeps/unix/sysv/linux/clock_nanosleep.c
>> +++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c
>> @@ -45,7 +45,8 @@ __clock_nanosleep_time64 (clockid_t clock_id, int flags,
>>
>>    int r;
>>  #ifdef __ASSUME_TIME64_SYSCALLS
>> -  r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, clock_id, flags, req,
>> +  long long int ts[2] = { req->tv_sec, req->tv_nsec };
>> +  r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, clock_id, flags, &ts,
>>                                rem);
>>  #else
>>    if (!in_time_t_range (req->tv_sec))
>> diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c
>> index 598d72b8b1..c57ae06311 100644
>> --- a/sysdeps/unix/sysv/linux/clock_settime.c
>> +++ b/sysdeps/unix/sysv/linux/clock_settime.c
>> @@ -35,7 +35,8 @@ __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp)
>>  #ifndef __NR_clock_settime64
>>  # define __NR_clock_settime64 __NR_clock_settime
>>  #endif
>> -  int ret = INLINE_SYSCALL_CALL (clock_settime64, clock_id, tp);
>> +  long long int ts[2] = { tp->tv_sec, tp->tv_nsec };
>> +  int ret = INLINE_SYSCALL_CALL (clock_settime64, clock_id, ts);
>>
>>  #ifndef __ASSUME_TIME64_SYSCALLS
>>    if (ret == 0 || errno != ENOSYS)
>> diff --git a/sysdeps/unix/sysv/linux/mq_timedreceive.c b/sysdeps/unix/sysv/linux/mq_timedreceive.c
>> index 7f3a112d7f..be7dc56c8b 100644
>> --- a/sysdeps/unix/sysv/linux/mq_timedreceive.c
>> +++ b/sysdeps/unix/sysv/linux/mq_timedreceive.c
>> @@ -32,8 +32,14 @@ ___mq_timedreceive_time64 (mqd_t mqdes, char *__restrict msg_ptr, size_t msg_len
>>  #endif
>>
>>  #ifdef __ASSUME_TIME64_SYSCALLS
>> +  long long int ts[2];
>> +  if (abs_timeout != NULL)
>> +    {
>> +      ts[0] = abs_timeout->tv_sec;
>> +      ts[1] = abs_timeout->tv_nsec;
>> +    }
>>    return SYSCALL_CANCEL (mq_timedreceive_time64, mqdes, msg_ptr, msg_len,
>> -                        msg_prio, abs_timeout);
>> +                        msg_prio, abs_timeout != NULL ? ts : NULL);
>>  #else
>>    bool need_time64 = abs_timeout != NULL
>>                      && !in_time_t_range (abs_timeout->tv_sec);
>> diff --git a/sysdeps/unix/sysv/linux/mq_timedsend.c b/sysdeps/unix/sysv/linux/mq_timedsend.c
>> index f93f865721..dcbb1a00d5 100644
>> --- a/sysdeps/unix/sysv/linux/mq_timedsend.c
>> +++ b/sysdeps/unix/sysv/linux/mq_timedsend.c
>> @@ -32,8 +32,14 @@ ___mq_timedsend_time64 (mqd_t mqdes, const char *msg_ptr, size_t msg_len,
>>  # endif
>>
>>  #ifdef __ASSUME_TIME64_SYSCALLS
>> +  long long int ts[2];
>> +  if (abs_timeout != NULL)
>> +    {
>> +      ts[0] = abs_timeout->tv_sec;
>> +      ts[1] = abs_timeout->tv_nsec;
>> +    }
>>    return SYSCALL_CANCEL (mq_timedsend_time64, mqdes, msg_ptr, msg_len,
>> -                        msg_prio, abs_timeout);
>> +                        msg_prio, abs_timeout != NULL ? ts : NULL);
>>  #else
>>    bool need_time64 = abs_timeout != NULL
>>                      && !in_time_t_range (abs_timeout->tv_sec);
>> diff --git a/sysdeps/unix/sysv/linux/ppoll.c b/sysdeps/unix/sysv/linux/ppoll.c
>> index 465dea623d..d01379ba2d 100644
>> --- a/sysdeps/unix/sysv/linux/ppoll.c
>> +++ b/sysdeps/unix/sysv/linux/ppoll.c
>> @@ -27,11 +27,11 @@ __ppoll64 (struct pollfd *fds, nfds_t nfds, const struct __timespec64 *timeout,
>>  {
>>    /* The Linux kernel can in some situations update the timeout value.
>>       We do not want that so use a local variable.  */
>> -  struct __timespec64 tval;
>> +  long long int ts[2];
>>    if (timeout != NULL)
>>      {
>> -      tval = *timeout;
>> -      timeout = &tval;
>> +      ts[0] = timeout->tv_sec;
>> +      ts[1] = timeout->tv_nsec;
>>      }
>>
>>  #ifndef __NR_ppoll_time64
>> @@ -39,15 +39,14 @@ __ppoll64 (struct pollfd *fds, nfds_t nfds, const struct __timespec64 *timeout,
>>  #endif
>>
>>  #ifdef __ASSUME_TIME64_SYSCALLS
>> -  return SYSCALL_CANCEL (ppoll_time64, fds, nfds, timeout, sigmask,
>> -                        __NSIG_BYTES);
>> +  return SYSCALL_CANCEL (ppoll_time64, fds, nfds, timeout != NULL ? ts : NULL,
>> +                        sigmask, __NSIG_BYTES);
>>  #else
>>    int ret;
>>    bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec);
>>    if (need_time64)
>>      {
>> -      ret = SYSCALL_CANCEL (ppoll_time64, fds, nfds, timeout, sigmask,
>> -                           __NSIG_BYTES);
>> +      ret = SYSCALL_CANCEL (ppoll_time64, fds, nfds, ts, sigmask, __NSIG_BYTES);
>>        if (ret == 0 || errno != ENOSYS)
>>         return ret;
>>        __set_errno (EOVERFLOW);
>> diff --git a/sysdeps/unix/sysv/linux/pselect.c b/sysdeps/unix/sysv/linux/pselect.c
>> index f58c01f25a..8cf22c8aa4 100644
>> --- a/sysdeps/unix/sysv/linux/pselect.c
>> +++ b/sysdeps/unix/sysv/linux/pselect.c
>> @@ -31,23 +31,22 @@ pselect64_syscall (int nfds, fd_set *readfds, fd_set *writefds,
>>      {
>>        (uintptr_t) sigmask, __NSIG_BYTES
>>      };
>> +  /* The Linux kernel can in some situations update the timeout value.
>> +     We do not want that so use a local variable.  */
>> +  long long int ts[2];
>> +  if (timeout != NULL)
>> +    {
>> +      ts[0] = timeout->tv_sec;
>> +      ts[1] = timeout->tv_nsec;
>> +    }
>>    return SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
>> -                        timeout, data);
>> +                        timeout != NULL ? ts : NULL, data);
>>  }
>>
>>  int
>>  __pselect64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>>              const struct __timespec64 *timeout, const sigset_t *sigmask)
>>  {
>> -  /* The Linux kernel can in some situations update the timeout value.
>> -     We do not want that so use a local variable.  */
>> -  struct __timespec64 tval;
>> -  if (timeout != NULL)
>> -    {
>> -      tval = *timeout;
>> -      timeout = &tval;
>> -    }
>> -
>>    /* Note: the system call expects 7 values but on most architectures
>>       we can only pass in 6 directly.  If there is an architecture with
>>       support for more parameters a new version of this file needs to
>> diff --git a/sysdeps/unix/sysv/linux/recvmmsg.c b/sysdeps/unix/sysv/linux/recvmmsg.c
>> index c214321e94..8d6cd238eb 100644
>> --- a/sysdeps/unix/sysv/linux/recvmmsg.c
>> +++ b/sysdeps/unix/sysv/linux/recvmmsg.c
>> @@ -26,8 +26,14 @@ __recvmmsg64 (int fd, struct mmsghdr *vmessages, unsigned int vlen, int flags,
>>  #ifndef __NR_recvmmsg_time64
>>  # define __NR_recvmmsg_time64 __NR_recvmmsg
>>  #endif
>> +  long long int ts[2];
>> +  if (timeout != NULL)
>> +    {
>> +      ts[0] = timeout->tv_sec;
>> +      ts[1] = timeout->tv_nsec;
>> +    }
>>    int r = SYSCALL_CANCEL (recvmmsg_time64, fd, vmessages, vlen, flags,
>> -                         timeout);
>> +                         timeout != NULL ? ts : NULL);
>>  #ifndef __ASSUME_TIME64_SYSCALLS
>>    if (r >= 0 || errno != ENOSYS)
>>      return r;
>> diff --git a/sysdeps/unix/sysv/linux/select.c b/sysdeps/unix/sysv/linux/select.c
>> index da25b4b4cf..dd47238572 100644
>> --- a/sysdeps/unix/sysv/linux/select.c
>> +++ b/sysdeps/unix/sysv/linux/select.c
>> @@ -53,13 +53,7 @@ __select64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>>        ns = us * NSEC_PER_USEC;
>>      }
>>
>> -  struct __timespec64 ts64, *pts64 = NULL;
>> -   if (timeout != NULL)
>> -     {
>> -       ts64.tv_sec = s;
>> -       ts64.tv_nsec = ns;
>> -       pts64 = &ts64;
>> -     }
>> +  long long int ts[2] = { s, ns };
>>
>>  #ifndef __NR_pselect6_time64
>>  # define __NR_pselect6_time64 __NR_pselect6
>> @@ -67,19 +61,23 @@ __select64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>>
>>  #ifdef __ASSUME_TIME64_SYSCALLS
>>    int r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, exceptfds,
>> -                         pts64, NULL);
>> +                         timeout != NULL ? ts : NULL, NULL);
>>    if (timeout != NULL)
>> -    TIMESPEC_TO_TIMEVAL (timeout, pts64);
>> +    {
>> +      timeout->tv_sec = ts[0];
>> +      timeout->tv_usec = ts[1] / 1000;
>> +    }
>>    return r;
>>  #else
>>    bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec);
>>    if (need_time64)
>>      {
>>        int r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds,
>> -                             exceptfds, pts64, NULL);
>> +                             exceptfds, ts, NULL);
>>        if ((r >= 0 || errno != ENOSYS) && timeout != NULL)
>>         {
>> -         TIMESPEC_TO_TIMEVAL (timeout, &ts64);
>> +         timeout->tv_sec = ts[0];
>> +         timeout->tv_usec = ts[1] / 1000;
>>         }
>>        else
>>         __set_errno (EOVERFLOW);
>> @@ -88,10 +86,10 @@ __select64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>>
>>  # ifdef __ASSUME_PSELECT
>>    struct timespec ts32, *pts32 = NULL;
>> -  if (pts64 != NULL)
>> +  if (timeout != NULL)
>>      {
>> -      ts32.tv_sec = pts64->tv_sec;
>> -      ts32.tv_nsec = pts64->tv_nsec;
>> +      ts32.tv_sec = ts[0];
>> +      ts32.tv_nsec = ts[1];
>>        pts32 = &ts32;
>>      }
>>
>> @@ -102,9 +100,10 @@ __select64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
>>    return r;
>>  # else
>>    struct timeval tv32, *ptv32 = NULL;
>> -  if (pts64 != NULL)
>> +  if (timeout != NULL)
>>      {
>> -      tv32 = valid_timespec64_to_timeval (*pts64);
>> +      tv32.tv_sec = ts[0];
>> +      tv32.tv_usec = ts[1];
>>        ptv32 = &tv32;
>>      }
>>
>> diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c
>> index 43577535ef..5816577a9f 100644
>> --- a/sysdeps/unix/sysv/linux/semtimedop.c
>> +++ b/sysdeps/unix/sysv/linux/semtimedop.c
>> @@ -22,7 +22,7 @@
>>
>>  static int
>>  semtimedop_syscall (int semid, struct sembuf *sops, size_t nsops,
>> -                   const struct __timespec64 *timeout)
>> +                   const void *timeout)
>>  {
>>  #ifdef __NR_semtimedop_time64
>>    return INLINE_SYSCALL_CALL (semtimedop_time64, semid, sops, nsops, timeout);
>> @@ -40,7 +40,13 @@ __semtimedop64 (int semid, struct sembuf *sops, size_t nsops,
>>                 const struct __timespec64 *timeout)
>>  {
>>  #ifdef __ASSUME_TIME64_SYSCALLS
>> -  return semtimedop_syscall (semid, sops, nsops, timeout);
>> +  long long int ts[2];
>> +  if (timeout != NULL)
>> +    {
>> +      ts[0] = timeout->tv_sec;
>> +      ts[1] = timeout->tv_nsec;
>> +    }
>> +  return semtimedop_syscall (semid, sops, nsops, timeout != NULL ? ts : NULL);
>>  #else
>>    bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec);
>>    if (need_time64)
>> diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c
>> index 607381a0a7..f08cc688db 100644
>> --- a/sysdeps/unix/sysv/linux/sigtimedwait.c
>> +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c
>> @@ -28,8 +28,14 @@ __sigtimedwait64 (const sigset_t *set, siginfo_t *info,
>>
>>    int result;
>>  #ifdef __ASSUME_TIME64_SYSCALLS
>> -  result = SYSCALL_CANCEL (rt_sigtimedwait_time64, set, info, timeout,
>> -                          __NSIG_BYTES);
>> +  long long int ts[2];
>> +  if (timeout != NULL)
>> +    {
>> +      ts[0] = timeout->tv_sec;
>> +      ts[1] = timeout->tv_nsec;
>> +    }
>> +  result = SYSCALL_CANCEL (rt_sigtimedwait_time64, set, info,
>> +                          timeout != NULL ? ts : NULL, __NSIG_BYTES);
>>  #else
>>    bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec);
>>    if (need_time64)
>> diff --git a/sysdeps/unix/sysv/linux/timer_settime.c b/sysdeps/unix/sysv/linux/timer_settime.c
>> index 24706e45d8..86199ebac5 100644
>> --- a/sysdeps/unix/sysv/linux/timer_settime.c
>> +++ b/sysdeps/unix/sysv/linux/timer_settime.c
>> @@ -35,8 +35,11 @@ ___timer_settime64 (timer_t timerid, int flags,
>>  #  ifndef __NR_timer_settime64
>>  #   define __NR_timer_settime64 __NR_timer_settime
>>  #  endif
>> -  return INLINE_SYSCALL_CALL (timer_settime64, ktimerid, flags, value,
>> -                              ovalue);
>> +  long long int ts[4] = { value->it_interval.tv_sec,
>> +                         value->it_interval.tv_nsec,
>> +                         value->it_value.tv_sec,
>> +                         value->it_value.tv_nsec };
>> +  return INLINE_SYSCALL_CALL (timer_settime64, ktimerid, flags, ts, ovalue);
>>  # else
>>  #  ifdef __NR_timer_settime64
>>    int ret = INLINE_SYSCALL_CALL (timer_settime64, ktimerid, flags, value,
>> diff --git a/sysdeps/unix/sysv/linux/timerfd_settime.c b/sysdeps/unix/sysv/linux/timerfd_settime.c
>> index 0069e1374a..709d54d37c 100644
>> --- a/sysdeps/unix/sysv/linux/timerfd_settime.c
>> +++ b/sysdeps/unix/sysv/linux/timerfd_settime.c
>> @@ -31,7 +31,11 @@ __timerfd_settime64 (int fd, int flags, const struct __itimerspec64 *value,
>>  #endif
>>
>>  #ifdef __ASSUME_TIME64_SYSCALLS
>> -  return INLINE_SYSCALL_CALL (timerfd_settime64, fd, flags, value, ovalue);
>> +  long long int ts[4] = { value->it_interval.tv_sec,
>> +                         value->it_interval.tv_nsec,
>> +                         value->it_value.tv_sec,
>> +                         value->it_value.tv_nsec };
>> +  return INLINE_SYSCALL_CALL (timerfd_settime64, fd, flags, ts, ovalue);
>>  #else
>>    bool need_time64 = !in_time_t_range (value->it_value.tv_sec)
>>                      || !in_time_t_range (value->it_interval.tv_sec);
>> diff --git a/sysdeps/unix/sysv/linux/utimensat.c b/sysdeps/unix/sysv/linux/utimensat.c
>> index e9061d2323..d64fb46899 100644
>> --- a/sysdeps/unix/sysv/linux/utimensat.c
>> +++ b/sysdeps/unix/sysv/linux/utimensat.c
>> @@ -22,6 +22,10 @@
>>  #include <time.h>
>>  #include <kernel-features.h>
>>
>> +/* For UTIME_NOW and UTIME_OMIT the value of tv_sec field is ignored.  */
>> +# define TS_SPECIAL(ts) \
>> +  ((ts).tv_nsec == UTIME_NOW || (ts).tv_nsec == UTIME_OMIT)
>> +
>>  /* Helper function defined for easy reusage of the code which calls utimensat
>>     and utimensat_time64 syscall.  */
>>  int
>> @@ -33,12 +37,17 @@ __utimensat64_helper (int fd, const char *file,
>>  #endif
>>
>>  #ifdef __ASSUME_TIME64_SYSCALLS
>> -  return INLINE_SYSCALL_CALL (utimensat_time64, fd, file, &tsp64[0], flags);
>> +  long long int ts[4];
>> +  if (tsp64 != NULL)
>> +    {
>> +      ts[0] = !TS_SPECIAL(tsp64[0]) ? tsp64[0].tv_sec : 0;
>> +      ts[1] = tsp64[0].tv_nsec;
>> +      ts[2] = !TS_SPECIAL(tsp64[1]) ? tsp64[1].tv_sec : 0;
>> +      ts[3] = tsp64[1].tv_nsec;
>> +    }
>> +  return INLINE_SYSCALL_CALL (utimensat_time64, fd, file,
>> +                             tsp64 != NULL ? ts : NULL, flags);
>>  #else
>> -  /* For UTIME_NOW and UTIME_OMIT the value of tv_sec field is ignored.  */
>> -# define TS_SPECIAL(ts) \
>> -  ((ts).tv_nsec == UTIME_NOW || (ts).tv_nsec == UTIME_OMIT)
>> -
>>    bool need_time64 = tsp64 != NULL
>>                      && ((!TS_SPECIAL (tsp64[0])
>>                           && !in_time_t_range (tsp64[0].tv_sec))
>> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile
>> index 16b768d8ba..74b3d30e2f 100644
>> --- a/sysdeps/unix/sysv/linux/x86_64/x32/Makefile
>> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/Makefile
>> @@ -6,6 +6,6 @@ sysdep_routines += arch_prctl
>>  endif
>>
>>  ifeq ($(subdir),conform)
>> -# For bugs 16437 and 21279.
>> +# For bug 21279.
>>  conformtest-xfail-conds += x86_64-x32-linux
>>  endif
>> diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h
>> index 489e81136d..923f8579e5 100644
>> --- a/time/bits/types/struct_timespec.h
>> +++ b/time/bits/types/struct_timespec.h
>> @@ -10,23 +10,15 @@
>>     has nanoseconds instead of microseconds.  */
>>  struct timespec
>>  {
>> -#ifdef __USE_TIME_BITS64
>> -  __time64_t tv_sec;           /* Seconds.  */
>> -#else
>> -  __time_t tv_sec;             /* Seconds.  */
>> +  time_t tv_sec;               /* Seconds.  */
>> +#if __WORDSIZE == 32 && (__TIMESIZE == 64 || defined __USE_TIME_BITS64) \
>> +  && __BYTE_ORDER == __BIG_ENDIAN
>> +  int: 32;                     /* Padding.  */
>>  #endif
>> -#if __WORDSIZE == 64 \
>> -  || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
>> -  || (__TIMESIZE == 32 && !defined __USE_TIME_BITS64)
>> -  __syscall_slong_t tv_nsec;   /* Nanoseconds.  */
>> -#else
>> -# if __BYTE_ORDER == __BIG_ENDIAN
>> -  int: 32;           /* Padding.  */
>> -  long int tv_nsec;  /* Nanoseconds.  */
>> -# else
>> -  long int tv_nsec;  /* Nanoseconds.  */
>> -  int: 32;           /* Padding.  */
>> -# endif
>> +  long int tv_nsec;            /* Nanoseconds.  */
>> +#if __WORDSIZE == 32 && (__TIMESIZE == 64 || defined __USE_TIME_BITS64) \
>> +  && __BYTE_ORDER == __LITTLE_ENDIAN
>> +  int: 32;                     /* Padding.  */
>>  #endif
>>  };
>>
>> --
>> 2.32.0
>>
> 
>
  

On 10/12/2021 12:44, Florian Weimer wrote:
> * H. J. Lu via Libc-alpha:
> 
>> On Fri, Dec 10, 2021 at 3:03 AM Adhemerval Zanella via Libc-alpha
>> <libc-alpha@sourceware.org> wrote:
>>>
>>> Although the kernel ABI uses 64-bit (so it can re-use the syscall
>>> logic from x86_64), POSIX states the type to be 'long int'.  It
>>> requires to explicit signal extend the value on syscalls that
>>> take the value to avoid requiring bumping the minimum kernel version
>>> (ea2ce8f3514e ("time: Fix get_timespec64() for y2038 safe compat
>>> interfaces") got merged into v4.18, futex and recvmsg followed in v5.1,
>>> but v5.6 is considered 'complete').
>>>
>>> The fix uses 'long long int' array to represent 'struct timespec',
>>> since it allows a simpler code than define a kernel specific timespec.
>>>
>>> There is no need to compat symbol, the now high-bits are not used
>>> to represent the nanoseconds and for some syscalls it only issues
>>> an error (which hardly indicates the need of compat symbol).
>>>
>>> Checked on x86_64-linux-gnu-x32 with kernel 4.4.0 and 5.13.
>>
>> Isn't it an ABI change for x32?
> 
> I agree.  I said so repeatedly.

Right, I will just drop this patch and set the patch as WONTFIX.
  

On Fri, Dec 10, 2021 at 04:44:46PM +0100, Florian Weimer via Libc-alpha wrote:
> * H. J. Lu via Libc-alpha:
> 
> > On Fri, Dec 10, 2021 at 3:03 AM Adhemerval Zanella via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >> Although the kernel ABI uses 64-bit (so it can re-use the syscall
> >> logic from x86_64), POSIX states the type to be 'long int'.  It
> >> requires to explicit signal extend the value on syscalls that
> >> take the value to avoid requiring bumping the minimum kernel version
> >> (ea2ce8f3514e ("time: Fix get_timespec64() for y2038 safe compat
> >> interfaces") got merged into v4.18, futex and recvmsg followed in v5.1,
> >> but v5.6 is considered 'complete').
> >>
> >> The fix uses 'long long int' array to represent 'struct timespec',
> >> since it allows a simpler code than define a kernel specific timespec.
> >>
> >> There is no need to compat symbol, the now high-bits are not used
> >> to represent the nanoseconds and for some syscalls it only issues
> >> an error (which hardly indicates the need of compat symbol).
> >>
> >> Checked on x86_64-linux-gnu-x32 with kernel 4.4.0 and 5.13.
> >
> > Isn't it an ABI change for x32?
> 
> I agree.  I said so repeatedly.

Can you clarify the sense in which it's an "ABI change"? I don't think
it's a change at all in the "mechanical" ABI linkage between libc and
the libc consumer. Of course it is a change in the types at the high
level language level.

If this were just an enhancement request, I'd be inclined to say just
drop it, but this is a bug fix for a serious conformance problem
that's going to have people writing wacky code to work around x32 (if
anyone even actually cares about x32) and that keeps coming up in
messy ways like the request to change tv_nsec to snseconds_t. From my
perspective there is very high value in getting rid of this exception
to the specification so that stuff doesn't get built around it. On top
of that, to my knowledge there are no binary distros for x32, and very
few users at all (so few that "remove x32" has been proposed multiple
times). I don't think any minor "ABI change" is going to have an
impact on them; glibc has probably made more-breaking changes of this
class before, even.

Rich
  

On 12/10/21 16:08, Rich Felker wrote:
> this is a bug fix for a serious conformance problem
> that's going to have people writing wacky code to work around x32

In my experience it's not a significant conformance issue. I've had very 
little trouble writing user code that's portable to x32 struct timespec. 
Generally speaking the code just works anyway; in the few cases where 
there might have been trouble the fixes were trivial and non-wacky.

Anybody writing portable struct timespec code has more important 
problems than this (e.g., Solaris 11 'stat' can set st_mtim.tv_nsec to a 
negative integer, something that's caused me far more hassle). And 
anybody porting to x32 has waaaaayy more important problems than this.

> if anyone even actually cares about x32

If the x32 user community (small as it is) doesn't care about this issue 
then we all have better things to do than worry about it.
  

On Sat, Dec 11, 2021 at 1:47 AM Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 12/10/21 16:08, Rich Felker wrote:
> > this is a bug fix for a serious conformance problem
> > that's going to have people writing wacky code to work around x32
>
> In my experience it's not a significant conformance issue. I've had very
> little trouble writing user code that's portable to x32 struct timespec.
> Generally speaking the code just works anyway; in the few cases where
> there might have been trouble the fixes were trivial and non-wacky.
>
> Anybody writing portable struct timespec code has more important
> problems than this (e.g., Solaris 11 'stat' can set st_mtim.tv_nsec to a
> negative integer, something that's caused me far more hassle). And
> anybody porting to x32 has waaaaayy more important problems than this.
>
> > if anyone even actually cares about x32
>
> If the x32 user community (small as it is) doesn't care about this issue
> then we all have better things to do than worry about it.

Would it help to resume the thread about removing the kernel support,
followed by removing glibc support? The last time Andy brought this up
was three years ago[1], and it was borderline then with about five users
saying they actually used it at the time and several kernel developers
speaking out in favor of removing the kernel bits.

The timing probably isn't bad either, with Debian 11 and Ubuntu 22.04
(both of which include marginal support for x32) being supported for
years to come. Anyone building their own stuff can rely on linux-5.15lts
and an existing glibc release to do the same.

         Arnd

[1]  https://lore.kernel.org/lkml/CALCETrXoRAibsbWa9nfbDrt0iEuebMnCMhSFg-d9W-J2g8mDjw@mail.gmail.com/
  

* Arnd Bergmann:

> On Sat, Dec 11, 2021 at 1:47 AM Paul Eggert <eggert@cs.ucla.edu> wrote:
>> On 12/10/21 16:08, Rich Felker wrote:
>> > this is a bug fix for a serious conformance problem
>> > that's going to have people writing wacky code to work around x32
>>
>> In my experience it's not a significant conformance issue. I've had very
>> little trouble writing user code that's portable to x32 struct timespec.
>> Generally speaking the code just works anyway; in the few cases where
>> there might have been trouble the fixes were trivial and non-wacky.
>>
>> Anybody writing portable struct timespec code has more important
>> problems than this (e.g., Solaris 11 'stat' can set st_mtim.tv_nsec to a
>> negative integer, something that's caused me far more hassle). And
>> anybody porting to x32 has waaaaayy more important problems than this.
>>
>> > if anyone even actually cares about x32
>>
>> If the x32 user community (small as it is) doesn't care about this issue
>> then we all have better things to do than worry about it.
>
> Would it help to resume the thread about removing the kernel support,
> followed by removing glibc support? The last time Andy brought this up
> was three years ago[1], and it was borderline then with about five users
> saying they actually used it at the time and several kernel developers
> speaking out in favor of removing the kernel bits.
>
> The timing probably isn't bad either, with Debian 11 and Ubuntu 22.04
> (both of which include marginal support for x32) being supported for
> years to come. Anyone building their own stuff can rely on linux-5.15lts
> and an existing glibc release to do the same.

I don't see x86-64 x32 causing us significant trouble on the glibc side.
I have worked on some x86-64-specific enhancements, and covering both
word sizes wasn't too hard because everything else is so similar.  But I
am very emphatically not an x32 user.

Much higher on my annoyance list are alpha, hppa, ia64.

Thanks,
Florian
  

On Sat, Dec 11, 2021 at 12:26:21PM +0100, Arnd Bergmann wrote:
> On Sat, Dec 11, 2021 at 1:47 AM Paul Eggert <eggert@cs.ucla.edu> wrote:
> > On 12/10/21 16:08, Rich Felker wrote:
> > > this is a bug fix for a serious conformance problem
> > > that's going to have people writing wacky code to work around x32
> >
> > In my experience it's not a significant conformance issue. I've had very
> > little trouble writing user code that's portable to x32 struct timespec.
> > Generally speaking the code just works anyway; in the few cases where
> > there might have been trouble the fixes were trivial and non-wacky.
> >
> > Anybody writing portable struct timespec code has more important
> > problems than this (e.g., Solaris 11 'stat' can set st_mtim.tv_nsec to a
> > negative integer, something that's caused me far more hassle). And
> > anybody porting to x32 has waaaaayy more important problems than this.
> >
> > > if anyone even actually cares about x32
> >
> > If the x32 user community (small as it is) doesn't care about this issue
> > then we all have better things to do than worry about it.
> 
> Would it help to resume the thread about removing the kernel support,
> followed by removing glibc support? The last time Andy brought this up
> was three years ago[1], and it was borderline then with about five users
> saying they actually used it at the time and several kernel developers
> speaking out in favor of removing the kernel bits.
> 
> The timing probably isn't bad either, with Debian 11 and Ubuntu 22.04
> (both of which include marginal support for x32) being supported for
> years to come. Anyone building their own stuff can rely on linux-5.15lts
> and an existing glibc release to do the same.
> 
>          Arnd
> 
> [1]  https://lore.kernel.org/lkml/CALCETrXoRAibsbWa9nfbDrt0iEuebMnCMhSFg-d9W-J2g8mDjw@mail.gmail.com/

I know this would make things easier for kernel folks, but I'd really
like to avoid removal and especially avoid being part of the impetus
for removal. I suspect a large portion of real-world x32 users are
musl users, and musl's x32 port has never had the problem this thread
is about.

If there is renewed desire for removal, could we look at doing it in a
way that enables moving support to userspace? That might actually
already be possible with the framework that was added for intercepting
syscalls (for wine) and binfmt_misc.

Rich
  

On Sat, Dec 11, 2021 at 1:51 PM Florian Weimer <fweimer@redhat.com> wrote:
> * Arnd Bergmann:
> > On Sat, Dec 11, 2021 at 1:47 AM Paul Eggert <eggert@cs.ucla.edu> wrote:
> >
> > The timing probably isn't bad either, with Debian 11 and Ubuntu 22.04
> > (both of which include marginal support for x32) being supported for
> > years to come. Anyone building their own stuff can rely on linux-5.15lts
> > and an existing glibc release to do the same.
>
> I don't see x86-64 x32 causing us significant trouble on the glibc side.
> I have worked on some x86-64-specific enhancements, and covering both
> word sizes wasn't too hard because everything else is so similar.

Ok, fair enough.

I assumed that it would help reduce the mess with the system call
names, as x32 is usually the one exception, being the one 32-bit
architecture that uses the 64-bit syscalls for off_t and time_t.

> Much higher on my annoyance list are alpha, hppa, ia64.

Makes sense. There are definitely more users on those than on
x32, so unless they are dropped from glibc or gcc first, I don't
see those go away from the kernel yet.

        Arnd
  

* Rich Felker:

> On Fri, Dec 10, 2021 at 04:44:46PM +0100, Florian Weimer via Libc-alpha wrote:
>> * H. J. Lu via Libc-alpha:
>> 
>> > On Fri, Dec 10, 2021 at 3:03 AM Adhemerval Zanella via Libc-alpha
>> > <libc-alpha@sourceware.org> wrote:
>> >>
>> >> Although the kernel ABI uses 64-bit (so it can re-use the syscall
>> >> logic from x86_64), POSIX states the type to be 'long int'.  It
>> >> requires to explicit signal extend the value on syscalls that
>> >> take the value to avoid requiring bumping the minimum kernel version
>> >> (ea2ce8f3514e ("time: Fix get_timespec64() for y2038 safe compat
>> >> interfaces") got merged into v4.18, futex and recvmsg followed in v5.1,
>> >> but v5.6 is considered 'complete').
>> >>
>> >> The fix uses 'long long int' array to represent 'struct timespec',
>> >> since it allows a simpler code than define a kernel specific timespec.
>> >>
>> >> There is no need to compat symbol, the now high-bits are not used
>> >> to represent the nanoseconds and for some syscalls it only issues
>> >> an error (which hardly indicates the need of compat symbol).
>> >>
>> >> Checked on x86_64-linux-gnu-x32 with kernel 4.4.0 and 5.13.
>> >
>> > Isn't it an ABI change for x32?
>> 
>> I agree.  I said so repeatedly.
>
> Can you clarify the sense in which it's an "ABI change"? I don't think
> it's a change at all in the "mechanical" ABI linkage between libc and
> the libc consumer. Of course it is a change in the types at the high
> level language level.

If struct timespec is passed as a register pair, the old type guarantees
that the callee sees a sign-extended value for the nanoseconds in the
register.  With the new type, the upper 32 bits of the register are
either undefined or zero (not sure which).  Existing compiler practice
favors zero extension, I think.  This may cause an observable difference
if only part of the link is upgraded to the new type, which I think
qualifies the change as an ABI break.

> If this were just an enhancement request, I'd be inclined to say just
> drop it, but this is a bug fix for a serious conformance problem
> that's going to have people writing wacky code to work around x32 (if
> anyone even actually cares about x32) and that keeps coming up in
> messy ways like the request to change tv_nsec to snseconds_t. From my
> perspective there is very high value in getting rid of this exception
> to the specification so that stuff doesn't get built around it.

If the kernel view and the userspace view differ, that also causes
practical problems.  Low-level C programmers do not expect a managed
run-time that uses different types, locks around system calls etc., even
though the resulting C implementation is not really POSIX-compliant.

Thanks,
Florian
  

On Dez 11 2021, Florian Weimer via Libc-alpha wrote:

> * Rich Felker:
>
>> On Fri, Dec 10, 2021 at 04:44:46PM +0100, Florian Weimer via Libc-alpha wrote:
>>> * H. J. Lu via Libc-alpha:
>>> 
>>> > On Fri, Dec 10, 2021 at 3:03 AM Adhemerval Zanella via Libc-alpha
>>> > <libc-alpha@sourceware.org> wrote:
>>> >>
>>> >> Although the kernel ABI uses 64-bit (so it can re-use the syscall
>>> >> logic from x86_64), POSIX states the type to be 'long int'.  It
>>> >> requires to explicit signal extend the value on syscalls that
>>> >> take the value to avoid requiring bumping the minimum kernel version
>>> >> (ea2ce8f3514e ("time: Fix get_timespec64() for y2038 safe compat
>>> >> interfaces") got merged into v4.18, futex and recvmsg followed in v5.1,
>>> >> but v5.6 is considered 'complete').
>>> >>
>>> >> The fix uses 'long long int' array to represent 'struct timespec',
>>> >> since it allows a simpler code than define a kernel specific timespec.
>>> >>
>>> >> There is no need to compat symbol, the now high-bits are not used
>>> >> to represent the nanoseconds and for some syscalls it only issues
>>> >> an error (which hardly indicates the need of compat symbol).
>>> >>
>>> >> Checked on x86_64-linux-gnu-x32 with kernel 4.4.0 and 5.13.
>>> >
>>> > Isn't it an ABI change for x32?
>>> 
>>> I agree.  I said so repeatedly.
>>
>> Can you clarify the sense in which it's an "ABI change"? I don't think
>> it's a change at all in the "mechanical" ABI linkage between libc and
>> the libc consumer. Of course it is a change in the types at the high
>> level language level.
>
> If struct timespec is passed as a register pair, the old type guarantees
> that the callee sees a sign-extended value for the nanoseconds in the
> register.  With the new type, the upper 32 bits of the register are
> either undefined or zero (not sure which).  Existing compiler practice
> favors zero extension, I think.  This may cause an observable difference
> if only part of the link is upgraded to the new type, which I think
> qualifies the change as an ABI break.

Or if a struct timespec is passed in memory, where a writer uses the new
definition and keeps the padding bits uninitialized, a reader using the
old definition will read those unitialized bits as part of the value.

Andreas.
  

On Sat, Dec 11, 2021 at 08:22:16PM +0100, Andreas Schwab wrote:
> On Dez 11 2021, Florian Weimer via Libc-alpha wrote:
> 
> > * Rich Felker:
> >
> >> On Fri, Dec 10, 2021 at 04:44:46PM +0100, Florian Weimer via Libc-alpha wrote:
> >>> * H. J. Lu via Libc-alpha:
> >>> 
> >>> > On Fri, Dec 10, 2021 at 3:03 AM Adhemerval Zanella via Libc-alpha
> >>> > <libc-alpha@sourceware.org> wrote:
> >>> >>
> >>> >> Although the kernel ABI uses 64-bit (so it can re-use the syscall
> >>> >> logic from x86_64), POSIX states the type to be 'long int'.  It
> >>> >> requires to explicit signal extend the value on syscalls that
> >>> >> take the value to avoid requiring bumping the minimum kernel version
> >>> >> (ea2ce8f3514e ("time: Fix get_timespec64() for y2038 safe compat
> >>> >> interfaces") got merged into v4.18, futex and recvmsg followed in v5.1,
> >>> >> but v5.6 is considered 'complete').
> >>> >>
> >>> >> The fix uses 'long long int' array to represent 'struct timespec',
> >>> >> since it allows a simpler code than define a kernel specific timespec.
> >>> >>
> >>> >> There is no need to compat symbol, the now high-bits are not used
> >>> >> to represent the nanoseconds and for some syscalls it only issues
> >>> >> an error (which hardly indicates the need of compat symbol).
> >>> >>
> >>> >> Checked on x86_64-linux-gnu-x32 with kernel 4.4.0 and 5.13.
> >>> >
> >>> > Isn't it an ABI change for x32?
> >>> 
> >>> I agree.  I said so repeatedly.
> >>
> >> Can you clarify the sense in which it's an "ABI change"? I don't think
> >> it's a change at all in the "mechanical" ABI linkage between libc and
> >> the libc consumer. Of course it is a change in the types at the high
> >> level language level.
> >
> > If struct timespec is passed as a register pair, the old type guarantees
> > that the callee sees a sign-extended value for the nanoseconds in the
> > register.  With the new type, the upper 32 bits of the register are
> > either undefined or zero (not sure which).  Existing compiler practice
> > favors zero extension, I think.  This may cause an observable difference
> > if only part of the link is upgraded to the new type, which I think
> > qualifies the change as an ABI break.
> 
> Or if a struct timespec is passed in memory, where a writer uses the new
> definition and keeps the padding bits uninitialized, a reader using the
> old definition will read those unitialized bits as part of the value.

Yes, these do not affect the libc/libc-consumer ABI linkage, but do
potentially affect ABI linkage between pairs of consumers. I suspect
there are no real-world applications affected by the pass-by-value
parts of the above, but there may be some affected by access in
memory.

Still, the issue can only happen if the pair of consumers was compiled
with different versions of the headers. It cannot happen just by
upgrading libc. I suspect glibc has made changes along these lines to
type definitions already in the past (sometimes with larger changes
necessitating symbol version bump on the glibc side, but completely
ignoring the corresponding version skew between pairs of libc
consumers). If so, I'm not sure why it would be a counterindication
now but not before. Note that you also already have this kind of issue
between libc consumers when one was compiled with _FILE_OFFSET_BITS=32
and another with _FILE_OFFSET_BITS=64, etc.

Rich
  

On Dez 11 2021, Rich Felker wrote:

> Still, the issue can only happen if the pair of consumers was compiled
> with different versions of the headers.

Which can happen quite easily, if the producer is recompiled after the
libc upgrade, while the consumer remains unchanged.

Andreas.
  

On Sat, Dec 11, 2021 at 3:49 PM Rich Felker <dalias@libc.org> wrote:
>
> I know this would make things easier for kernel folks, but I'd really
> like to avoid removal and especially avoid being part of the impetus
> for removal. I suspect a large portion of real-world x32 users are
> musl users, and musl's x32 port has never had the problem this thread
> is about.

Do you know of actual users, or products shipping with x32 musl?

My assumption was that there are not really, and that it would just
be an easy way to end a pointless email discussion ;-)

> If there is renewed desire for removal, could we look at doing it in a
> way that enables moving support to userspace? That might actually
> already be possible with the framework that was added for intercepting
> syscalls (for wine) and binfmt_misc.

There is some ongoing debate on doing foreign-architecture system
calls, which would work here, but I don't think this is happening soon.

There isn't much code that is involved in x32 support in the kernel,
but it is in rather tricky places:

- ELF loader is separate, but can probably be done as a binfmt-misc
  loader the way that qemu-user does it without too much trouble
- address space limitation is something we should really have in the
  kernel, as there are several use cases for that, this is mainly
  a question of defining a new syscall or other ABI to set it from user
  space
- most system calls are either the normal x86-64 version or the x86-32
  version for x32, so this is a question of calling the right one. I think
  that x86 still allows calling either one
- not sure about signal handling, the kernel currently does it differently
  for x32 tasks, and my assumption was that the reason is that this
  can't be easily emulated in libc on top of x86-64 signals.
- the hardest part are the ioctl (and similar syscalls) with arguments
  that are incompatible between x86-32 and x32, which usually
  happens because of misaligned 'long long' having different padding.
  x32 ioctl is compatible with most other architectures here, so this
  requires the generalized ioctl syscall that lets user space pick between
  more ABIs.

          Arnd