* [PATCH] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437)
@ 2021-12-09 19:10 Adhemerval Zanella
2021-12-09 19:19 ` Florian Weimer
0 siblings, 1 reply; 8+ messages in thread
From: Adhemerval Zanella @ 2021-12-09 19:10 UTC (permalink / raw)
To: libc-alpha, Zack Weinberg
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.
---
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 | 16 ++---------
20 files changed, 111 insertions(+), 74 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..b962d3f95f 100644
--- a/time/bits/types/struct_timespec.h
+++ b/time/bits/types/struct_timespec.h
@@ -15,19 +15,9 @@ struct timespec
#else
__time_t tv_sec; /* Seconds. */
#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
-#endif
+ int : 8 * (sizeof (time_t) - sizeof (long)) * (__BYTE_ORDER == __BIG_ENDIAN);
+ long int tv_nsec;
+ int : 8 * (sizeof (time_t) - sizeof (long)) * (__BYTE_ORDER == __LITTLE_ENDIAN);
};
#endif
--
2.32.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437)
2021-12-09 19:10 [PATCH] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437) Adhemerval Zanella
@ 2021-12-09 19:19 ` Florian Weimer
2021-12-09 19:35 ` Adhemerval Zanella
0 siblings, 1 reply; 8+ messages in thread
From: Florian Weimer @ 2021-12-09 19:19 UTC (permalink / raw)
To: Adhemerval Zanella via Libc-alpha; +Cc: Zack Weinberg, Adhemerval Zanella
* Adhemerval Zanella via Libc-alpha:
> diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h
> index 489e81136d..b962d3f95f 100644
> --- a/time/bits/types/struct_timespec.h
> +++ b/time/bits/types/struct_timespec.h
> @@ -15,19 +15,9 @@ struct timespec
> #else
> __time_t tv_sec; /* Seconds. */
> #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
> -#endif
> + int : 8 * (sizeof (time_t) - sizeof (long)) * (__BYTE_ORDER == __BIG_ENDIAN);
> + long int tv_nsec;
> + int : 8 * (sizeof (time_t) - sizeof (long)) * (__BYTE_ORDER == __LITTLE_ENDIAN);
> };
I think zero-width bitfields are problematic from an ABI perspective:
zero width bitfields and ABIs
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102024>
The change is also not entirely ABI-compatible because existing code on
x32 depends on the implementation clearing the upper 32 bits, and if new
code copies around structs, it won't necessary copy the zero bits.
Thanks,
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437)
2021-12-09 19:19 ` Florian Weimer
@ 2021-12-09 19:35 ` Adhemerval Zanella
2021-12-10 11:23 ` Arnd Bergmann
0 siblings, 1 reply; 8+ messages in thread
From: Adhemerval Zanella @ 2021-12-09 19:35 UTC (permalink / raw)
To: Florian Weimer, Adhemerval Zanella via Libc-alpha; +Cc: Zack Weinberg
On 09/12/2021 16:19, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
>
>> diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h
>> index 489e81136d..b962d3f95f 100644
>> --- a/time/bits/types/struct_timespec.h
>> +++ b/time/bits/types/struct_timespec.h
>> @@ -15,19 +15,9 @@ struct timespec
>> #else
>> __time_t tv_sec; /* Seconds. */
>> #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
>> -#endif
>> + int : 8 * (sizeof (time_t) - sizeof (long)) * (__BYTE_ORDER == __BIG_ENDIAN);
>> + long int tv_nsec;
>> + int : 8 * (sizeof (time_t) - sizeof (long)) * (__BYTE_ORDER == __LITTLE_ENDIAN);
>> };
>
> I think zero-width bitfields are problematic from an ABI perspective:
>
> zero width bitfields and ABIs
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102024>
Sigh... so each backend now might have a different behavior. Alright, we
can use Zack's suggestion then:
diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h
index b962d3f95f..a93100b0dd 100644
--- a/time/bits/types/struct_timespec.h
+++ b/time/bits/types/struct_timespec.h
@@ -15,9 +15,15 @@ struct timespec
#else
__time_t tv_sec; /* Seconds. */
#endif
- int : 8 * (sizeof (time_t) - sizeof (long)) * (__BYTE_ORDER == __BIG_ENDIAN);
- long int tv_nsec;
- int : 8 * (sizeof (time_t) - sizeof (long)) * (__BYTE_ORDER == __LITTLE_ENDIAN);
+#if __WORDSIZE == 32 && (__TIMESIZE == 64 || defined __USE_TIME_BITS64) \
+ && __BYTE_ORDER == __BIG_ENDIAN
+ 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
};
#endif
>
> The change is also not entirely ABI-compatible because existing code on
> x32 depends on the implementation clearing the upper 32 bits, and if new
> code copies around structs, it won't necessary copy the zero bits.
Afaiu the high-bits clearing is strictly necessary on kABI boundary for
set functions. Some issues indeed might arise if user use timespec
embedded in some function and mix objects built with different glibc
version, but it is similar to 32-bit architectures with mixing time_t
objects.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437)
2021-12-09 19:35 ` Adhemerval Zanella
@ 2021-12-10 11:23 ` Arnd Bergmann
2021-12-10 11:29 ` Florian Weimer
0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2021-12-10 11:23 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Florian Weimer, Adhemerval Zanella via Libc-alpha, Zack Weinberg
On Thu, Dec 9, 2021 at 8:35 PM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
> On 09/12/2021 16:19, Florian Weimer wrote: * Adhemerval Zanella via Libc-alpha:
> > The change is also not entirely ABI-compatible because existing code on
> > x32 depends on the implementation clearing the upper 32 bits, and if new
> > code copies around structs, it won't necessary copy the zero bits.
>
> Afaiu the high-bits clearing is strictly necessary on kABI boundary for
> set functions. Some issues indeed might arise if user use timespec
> embedded in some function and mix objects built with different glibc
> version, but it is similar to 32-bit architectures with mixing time_t
> objects.
The kernel's 32-bit ABI ignores the upper 32 bits of tv_nsec for structures
passed from user space, but zeroes them when returning a timespec to user
space.
Only x32 is weird here, as it traditionally uses a mix of hte 32-bit user
space ABI and the 64-bit user space ABI.
Arnd
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437)
2021-12-10 11:23 ` Arnd Bergmann
@ 2021-12-10 11:29 ` Florian Weimer
2021-12-10 12:48 ` Arnd Bergmann
0 siblings, 1 reply; 8+ messages in thread
From: Florian Weimer @ 2021-12-10 11:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Adhemerval Zanella, Adhemerval Zanella via Libc-alpha, Zack Weinberg
* Arnd Bergmann:
> On Thu, Dec 9, 2021 at 8:35 PM Adhemerval Zanella via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>> On 09/12/2021 16:19, Florian Weimer wrote: * Adhemerval Zanella via Libc-alpha:
>
>> > The change is also not entirely ABI-compatible because existing code on
>> > x32 depends on the implementation clearing the upper 32 bits, and if new
>> > code copies around structs, it won't necessary copy the zero bits.
>>
>> Afaiu the high-bits clearing is strictly necessary on kABI boundary for
>> set functions. Some issues indeed might arise if user use timespec
>> embedded in some function and mix objects built with different glibc
>> version, but it is similar to 32-bit architectures with mixing time_t
>> objects.
>
> The kernel's 32-bit ABI ignores the upper 32 bits of tv_nsec for structures
> passed from user space, but zeroes them when returning a timespec to user
> space.
And that zeroing may get lost if the struct has padding. That's my
concern.
Thanks,
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437)
2021-12-10 11:29 ` Florian Weimer
@ 2021-12-10 12:48 ` Arnd Bergmann
2021-12-10 12:51 ` Florian Weimer
0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2021-12-10 12:48 UTC (permalink / raw)
To: Florian Weimer
Cc: Arnd Bergmann, Adhemerval Zanella,
Adhemerval Zanella via Libc-alpha, Zack Weinberg
On Fri, Dec 10, 2021 at 12:29 PM Florian Weimer <fweimer@redhat.com> wrote:
> > On Thu, Dec 9, 2021 at 8:35 PM Adhemerval Zanella via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >> On 09/12/2021 16:19, Florian Weimer wrote: * Adhemerval Zanella via Libc-alpha:
> >
> >> > The change is also not entirely ABI-compatible because existing code on
> >> > x32 depends on the implementation clearing the upper 32 bits, and if new
> >> > code copies around structs, it won't necessary copy the zero bits.
> >>
> >> Afaiu the high-bits clearing is strictly necessary on kABI boundary for
> >> set functions. Some issues indeed might arise if user use timespec
> >> embedded in some function and mix objects built with different glibc
> >> version, but it is similar to 32-bit architectures with mixing time_t
> >> objects.
> >
> > The kernel's 32-bit ABI ignores the upper 32 bits of tv_nsec for structures
> > passed from user space, but zeroes them when returning a timespec to user
> > space.
>
> And that zeroing may get lost if the struct has padding. That's my
> concern.
Which struct? I am talking about __kernel_timespec, which has
no padding on any architecture, and this is what gets used in the uabi
headers.
Arnd
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437)
2021-12-10 12:48 ` Arnd Bergmann
@ 2021-12-10 12:51 ` Florian Weimer
2021-12-10 13:11 ` Arnd Bergmann
0 siblings, 1 reply; 8+ messages in thread
From: Florian Weimer @ 2021-12-10 12:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Adhemerval Zanella, Adhemerval Zanella via Libc-alpha, Zack Weinberg
* Arnd Bergmann:
> On Fri, Dec 10, 2021 at 12:29 PM Florian Weimer <fweimer@redhat.com> wrote:
>> > On Thu, Dec 9, 2021 at 8:35 PM Adhemerval Zanella via Libc-alpha
>> > <libc-alpha@sourceware.org> wrote:
>> >> On 09/12/2021 16:19, Florian Weimer wrote: * Adhemerval Zanella via Libc-alpha:
>> >
>> >> > The change is also not entirely ABI-compatible because existing code on
>> >> > x32 depends on the implementation clearing the upper 32 bits, and if new
>> >> > code copies around structs, it won't necessary copy the zero bits.
>> >>
>> >> Afaiu the high-bits clearing is strictly necessary on kABI boundary for
>> >> set functions. Some issues indeed might arise if user use timespec
>> >> embedded in some function and mix objects built with different glibc
>> >> version, but it is similar to 32-bit architectures with mixing time_t
>> >> objects.
>> >
>> > The kernel's 32-bit ABI ignores the upper 32 bits of tv_nsec for structures
>> > passed from user space, but zeroes them when returning a timespec to user
>> > space.
>>
>> And that zeroing may get lost if the struct has padding. That's my
>> concern.
>
> Which struct? I am talking about __kernel_timespec, which has
> no padding on any architecture, and this is what gets used in the uabi
> headers.
The proposed new glibc struct timespec for x32.
Thanks,
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437)
2021-12-10 12:51 ` Florian Weimer
@ 2021-12-10 13:11 ` Arnd Bergmann
0 siblings, 0 replies; 8+ messages in thread
From: Arnd Bergmann @ 2021-12-10 13:11 UTC (permalink / raw)
To: Florian Weimer
Cc: Arnd Bergmann, Adhemerval Zanella,
Adhemerval Zanella via Libc-alpha, Zack Weinberg
On Fri, Dec 10, 2021 at 1:51 PM Florian Weimer <fweimer@redhat.com> wrote:
> > On Fri, Dec 10, 2021 at 12:29 PM Florian Weimer <fweimer@redhat.com> wrote:
> >> > On Thu, Dec 9, 2021 at 8:35 PM Adhemerval Zanella via Libc-alpha
> >> > <libc-alpha@sourceware.org> wrote:
> >> >> On 09/12/2021 16:19, Florian Weimer wrote: * Adhemerval Zanella via Libc-alpha:
> >> >
> >> >> > The change is also not entirely ABI-compatible because existing code on
> >> >> > x32 depends on the implementation clearing the upper 32 bits, and if new
> >> >> > code copies around structs, it won't necessary copy the zero bits.
> >> >>
> >> >> Afaiu the high-bits clearing is strictly necessary on kABI boundary for
> >> >> set functions. Some issues indeed might arise if user use timespec
> >> >> embedded in some function and mix objects built with different glibc
> >> >> version, but it is similar to 32-bit architectures with mixing time_t
> >> >> objects.
> >> >
> >> > The kernel's 32-bit ABI ignores the upper 32 bits of tv_nsec for structures
> >> > passed from user space, but zeroes them when returning a timespec to user
> >> > space.
> >>
> >> And that zeroing may get lost if the struct has padding. That's my
> >> concern.
> >
> > Which struct? I am talking about __kernel_timespec, which has
> > no padding on any architecture, and this is what gets used in the uabi
> > headers.
>
> The proposed new glibc struct timespec for x32.
Ok, I don't care then. If anyone is still running x32 user space, they have
bigger problems to worry about than the timespec layout. It probably works
fine in modern kernels if you make glibc pretend that it's using the normal
32-bit ABI for x32 timespec rather than the one that is declared in the
kernel headers (using __kernel_long_t), but I wouldn't stress out about it
either way.
Arnd
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-12-10 13:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-09 19:10 [PATCH] linux: Use 'long int' for timespec tv_nsec on x32 (BZ #16437) Adhemerval Zanella
2021-12-09 19:19 ` Florian Weimer
2021-12-09 19:35 ` Adhemerval Zanella
2021-12-10 11:23 ` Arnd Bergmann
2021-12-10 11:29 ` Florian Weimer
2021-12-10 12:48 ` Arnd Bergmann
2021-12-10 12:51 ` Florian Weimer
2021-12-10 13:11 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).