From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x32c.google.com (mail-ot1-x32c.google.com [IPv6:2607:f8b0:4864:20::32c]) by sourceware.org (Postfix) with ESMTPS id 7222D3858C2D for ; Mon, 7 Nov 2022 19:49:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7222D3858C2D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-ot1-x32c.google.com with SMTP id f4-20020a056830264400b0066c8e56828aso5452808otu.1 for ; Mon, 07 Nov 2022 11:49:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=RetN91XAMAFO7WtyEf8VRT8W8r7DIeoYWCzxdIXEG1o=; b=YqQZJ7ujhd8WQBf8oBXFp0s796E6+8fDw4jkX9tjCMp0b4x63d00RoauHkYLb6DcLy dTXAeZVyGzGYdYEoR6CUb77QGDQ/Wf4XC8lCwKX1KQEyAIQSxkgwccz939QaU9GNAVHf 04E1WaPQKgE4+MN/S2FH5vSmeyAM3dm502Wk2Y4ED/4BrTNQPFCAoNRugjbFOqCrgh/6 NIl6+5mrf4C88d4FRDpPwQCxdAbpNko3paNTWGrKbCHR5IIBjmoBPTFERyro0YZJD6/c 6H8fcSpK6KPG5HeI0PiH3qgmb8gMY+zo7ePO9vIjBbw5Tr/wezyGxTWQF1jZA2iRLZbr afAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=RetN91XAMAFO7WtyEf8VRT8W8r7DIeoYWCzxdIXEG1o=; b=69DrMjL+QYDyW2nQCnOd+GnqHvymF2GVhPL5vjEr8ysfX7m8rQ1SbKwPg7+gqnbX7Y jKPhiL+0nn5Vp+Alxdbk9iUrnNT/VONvoICi13+X2WAF96yXH887xZJGhlPPkuTiug9M 8+makXCElSmULrgSinpTbFWyjljag4a51yVjH70ZV5tXX2Bnz+6v5N1etcgsepCK9EeR jKMjbyOu+2uBoVjVA2LH7ZioFk7sIw6r5vsAiGuIEylsG+wvLmjRp+utysH2hWjgL3xe LtXPyTt3QTad0yjIOtkPgXJxPE2SbX3qwIx9tKSeFmFGgGys/W60qJPh5ByjOtJa0PzX wCWg== X-Gm-Message-State: ACrzQf0IyHQGuHb2ZVCQrwvt1WC6AEnj2+hZhVOhqsPfW6nDJRAb+6JC iYS+5hr9Nkc8bzhwxUEJvHbmoQ== X-Google-Smtp-Source: AMsMyM4wJxNcHMlFY0APKhlzZX+oIqn2ODXJDKjUmSvqFEvV4GjPRTEBMXewj7zBv3ulT/pZAmZ0ng== X-Received: by 2002:a9d:7d18:0:b0:66c:5211:e972 with SMTP id v24-20020a9d7d18000000b0066c5211e972mr21964510otn.311.1667850555562; Mon, 07 Nov 2022 11:49:15 -0800 (PST) Received: from ?IPV6:2804:1b3:a7c0:a9f4:fc60:ae8f:c4b6:cf0a? ([2804:1b3:a7c0:a9f4:fc60:ae8f:c4b6:cf0a]) by smtp.gmail.com with ESMTPSA id w14-20020a056830060e00b0063696cbb6bdsm3228904oti.62.2022.11.07.11.49.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 07 Nov 2022 11:49:15 -0800 (PST) Message-ID: <52e1dbe6-3af2-fba3-38ef-5c05d2d4f2d6@linaro.org> Date: Mon, 7 Nov 2022 16:49:11 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH v2] Use in_int32_t_range to check need 64bit syscall Content-Language: en-US To: YunQiang Su , libc-alpha@sourceware.org Cc: aurelien@aurel32.net, jiaxun.yang@flygoat.com, macro@orcam.me.uk, syq@debian.org References: <20221104014757.1544793-1-yunqiang.su@cipunited.com> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <20221104014757.1544793-1-yunqiang.su@cipunited.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 03/11/22 22:47, YunQiang Su wrote: > The current version, we use in_time_t_range to detect overflow > of a time. Whenever overflow we try to use 64bit version syscall. > > While the function name is quite confused. So let's use a new > function called in_int32_t_range to do so. > > We still keep in_time_t_range, and use it to detect overflow of > the return value from syscalls. The commit message sounds confusing, maybe: "Currently glibc uses in_time_t_range to detects time_t overflow, and if it occurs fallbacks to 64 bit syscall version. The function name is confusing because internally time_t might be either 32 bits or 64 bits (depending on __TIMESIZE). This patch refactors the in_time_t_range by replacing it with in_int32_t_range for the case to check if the 64 bit time_t syscall should be used. The in_time_t range is used to detect overflow of the syscall return value". > --- > include/time.h | 10 +++++++++- > nptl/futex-internal.c | 4 ++-- > sysdeps/unix/sysv/linux/clock_adjtime.c | 2 +- > sysdeps/unix/sysv/linux/clock_nanosleep.c | 2 +- > sysdeps/unix/sysv/linux/clock_settime.c | 2 +- > sysdeps/unix/sysv/linux/mq_timedreceive.c | 2 +- > sysdeps/unix/sysv/linux/mq_timedsend.c | 2 +- > sysdeps/unix/sysv/linux/ppoll.c | 2 +- > sysdeps/unix/sysv/linux/pselect.c | 2 +- > sysdeps/unix/sysv/linux/recvmmsg.c | 2 +- > sysdeps/unix/sysv/linux/select.c | 2 +- > sysdeps/unix/sysv/linux/semtimedop.c | 2 +- > sysdeps/unix/sysv/linux/setitimer.c | 4 ++-- > sysdeps/unix/sysv/linux/setsockopt.c | 2 +- > sysdeps/unix/sysv/linux/sigtimedwait.c | 2 +- > sysdeps/unix/sysv/linux/timer_settime.c | 4 ++-- > sysdeps/unix/sysv/linux/timerfd_settime.c | 4 ++-- > sysdeps/unix/sysv/linux/utimensat.c | 4 ++-- > 18 files changed, 31 insertions(+), 23 deletions(-) > > diff --git a/include/time.h b/include/time.h > index 20abea69d4..f599eeed4e 100644 > --- a/include/time.h > +++ b/include/time.h > @@ -347,12 +347,20 @@ libc_hidden_proto (__time64) > /* Check whether T fits in int32_t, assume all usages are for > sizeof(time_t) == 32. */ > static inline bool > -in_time_t_range (__time64_t t) > +in_int32_t_range (__time64_t t) > { > int32_t s = t; > return s == t; > } > > +/* Check whether T fits in time_t. */ > +static inline bool > +in_time_t_range (__time64_t t) > +{ > + time_t s = t; > + return s == t; > +} > + I think it might be better to avoid define this function for __TIMESIZE == 64, the function always returns true and for 64 bit time_t its occurence might be a misuse. The rest of the path looks ok, the remaining in_time_t_range usage are only for check of syscall return value (besides the generic usage on mktime and gmtime). > /* Convert a known valid struct timeval into a struct __timespec64. */ > static inline struct __timespec64 > valid_timeval_to_timespec64 (const struct timeval tv) > diff --git a/nptl/futex-internal.c b/nptl/futex-internal.c > index 7ec228e8fb..6b8ac2304d 100644 > --- a/nptl/futex-internal.c > +++ b/nptl/futex-internal.c > @@ -87,7 +87,7 @@ __futex_abstimed_wait_common (unsigned int* futex_word, > err = __futex_abstimed_wait_common64 (futex_word, expected, op, abstime, > private, cancel); > #else > - bool need_time64 = abstime != NULL && !in_time_t_range (abstime->tv_sec); > + bool need_time64 = abstime != NULL && !in_int32_t_range (abstime->tv_sec); > if (need_time64) > { > err = __futex_abstimed_wait_common64 (futex_word, expected, op, abstime, > @@ -162,7 +162,7 @@ __futex_lock_pi64 (int *futex_word, clockid_t clockid, > # ifdef __ASSUME_TIME64_SYSCALLS > err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op_pi, 0, abstime); > # else > - bool need_time64 = abstime != NULL && !in_time_t_range (abstime->tv_sec); > + bool need_time64 = abstime != NULL && !in_int32_t_range (abstime->tv_sec); > if (need_time64) > err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op_pi, 0, abstime); > else > diff --git a/sysdeps/unix/sysv/linux/clock_adjtime.c b/sysdeps/unix/sysv/linux/clock_adjtime.c > index 5ded82f506..63ea68c3af 100644 > --- a/sysdeps/unix/sysv/linux/clock_adjtime.c > +++ b/sysdeps/unix/sysv/linux/clock_adjtime.c > @@ -35,7 +35,7 @@ __clock_adjtime64 (const clockid_t clock_id, struct __timex64 *tx64) > return r; > > if (tx64->modes & ADJ_SETOFFSET > - && ! in_time_t_range (tx64->time.tv_sec)) > + && ! in_int32_t_range (tx64->time.tv_sec)) > { > __set_errno (EOVERFLOW); > return -1; > diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c > index e610fd4e8d..9031060486 100644 > --- a/sysdeps/unix/sysv/linux/clock_nanosleep.c > +++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c > @@ -48,7 +48,7 @@ __clock_nanosleep_time64 (clockid_t clock_id, int flags, > r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, clock_id, flags, req, > rem); > #else > - if (!in_time_t_range (req->tv_sec)) > + if (!in_int32_t_range (req->tv_sec)) > { > r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, clock_id, flags, > req, rem); > diff --git a/sysdeps/unix/sysv/linux/clock_settime.c b/sysdeps/unix/sysv/linux/clock_settime.c > index 2a32e2eb13..db9698ba0d 100644 > --- a/sysdeps/unix/sysv/linux/clock_settime.c > +++ b/sysdeps/unix/sysv/linux/clock_settime.c > @@ -41,7 +41,7 @@ __clock_settime64 (clockid_t clock_id, const struct __timespec64 *tp) > if (ret == 0 || errno != ENOSYS) > return ret; > > - if (! in_time_t_range (tp->tv_sec)) > + if (! in_int32_t_range (tp->tv_sec)) > { > __set_errno (EOVERFLOW); > return -1; > diff --git a/sysdeps/unix/sysv/linux/mq_timedreceive.c b/sysdeps/unix/sysv/linux/mq_timedreceive.c > index 5bf1e0a83b..6c719d4b70 100644 > --- a/sysdeps/unix/sysv/linux/mq_timedreceive.c > +++ b/sysdeps/unix/sysv/linux/mq_timedreceive.c > @@ -36,7 +36,7 @@ ___mq_timedreceive_time64 (mqd_t mqdes, char *__restrict msg_ptr, size_t msg_len > msg_prio, abs_timeout); > #else > bool need_time64 = abs_timeout != NULL > - && !in_time_t_range (abs_timeout->tv_sec); > + && !in_int32_t_range (abs_timeout->tv_sec); > if (need_time64) > { > int r = SYSCALL_CANCEL (mq_timedreceive_time64, mqdes, msg_ptr, msg_len, > diff --git a/sysdeps/unix/sysv/linux/mq_timedsend.c b/sysdeps/unix/sysv/linux/mq_timedsend.c > index 3ca5050976..eb8353feef 100644 > --- a/sysdeps/unix/sysv/linux/mq_timedsend.c > +++ b/sysdeps/unix/sysv/linux/mq_timedsend.c > @@ -36,7 +36,7 @@ ___mq_timedsend_time64 (mqd_t mqdes, const char *msg_ptr, size_t msg_len, > msg_prio, abs_timeout); > #else > bool need_time64 = abs_timeout != NULL > - && !in_time_t_range (abs_timeout->tv_sec); > + && !in_int32_t_range (abs_timeout->tv_sec); > if (need_time64) > { > int r = SYSCALL_CANCEL (mq_timedsend_time64, mqdes, msg_ptr, msg_len, > diff --git a/sysdeps/unix/sysv/linux/ppoll.c b/sysdeps/unix/sysv/linux/ppoll.c > index 1105e29b00..6653e1bc55 100644 > --- a/sysdeps/unix/sysv/linux/ppoll.c > +++ b/sysdeps/unix/sysv/linux/ppoll.c > @@ -43,7 +43,7 @@ __ppoll64 (struct pollfd *fds, nfds_t nfds, const struct __timespec64 *timeout, > __NSIG_BYTES); > #else > int ret; > - bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec); > + bool need_time64 = timeout != NULL && !in_int32_t_range (timeout->tv_sec); > if (need_time64) > { > ret = SYSCALL_CANCEL (ppoll_time64, fds, nfds, timeout, sigmask, > diff --git a/sysdeps/unix/sysv/linux/pselect.c b/sysdeps/unix/sysv/linux/pselect.c > index eba1c111f8..23fd0dfad6 100644 > --- a/sysdeps/unix/sysv/linux/pselect.c > +++ b/sysdeps/unix/sysv/linux/pselect.c > @@ -56,7 +56,7 @@ __pselect64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, > return pselect64_syscall (nfds, readfds, writefds, exceptfds, timeout, > sigmask); > #else > - bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec); > + bool need_time64 = timeout != NULL && !in_int32_t_range (timeout->tv_sec); > if (need_time64) > { > int r = pselect64_syscall (nfds, readfds, writefds, exceptfds, timeout, > diff --git a/sysdeps/unix/sysv/linux/recvmmsg.c b/sysdeps/unix/sysv/linux/recvmmsg.c > index d9664d61c4..1ba5a4f25c 100644 > --- a/sysdeps/unix/sysv/linux/recvmmsg.c > +++ b/sysdeps/unix/sysv/linux/recvmmsg.c > @@ -35,7 +35,7 @@ recvmmsg_syscall (int fd, struct mmsghdr *vmessages, unsigned int vlen, > struct timespec ts32, *pts32 = NULL; > if (timeout != NULL) > { > - if (! in_time_t_range (timeout->tv_sec)) > + if (! in_int32_t_range (timeout->tv_sec)) > { > __set_errno (EINVAL); > return -1; > diff --git a/sysdeps/unix/sysv/linux/select.c b/sysdeps/unix/sysv/linux/select.c > index a3f0a2eba7..fd147dfdd4 100644 > --- a/sysdeps/unix/sysv/linux/select.c > +++ b/sysdeps/unix/sysv/linux/select.c > @@ -72,7 +72,7 @@ __select64 (int nfds, fd_set *readfds, fd_set *writefds, fd_set *exceptfds, > TIMESPEC_TO_TIMEVAL (timeout, pts64); > return r; > #else > - bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec); > + bool need_time64 = timeout != NULL && !in_int32_t_range (timeout->tv_sec); > if (need_time64) > { > int r = SYSCALL_CANCEL (pselect6_time64, nfds, readfds, writefds, > diff --git a/sysdeps/unix/sysv/linux/semtimedop.c b/sysdeps/unix/sysv/linux/semtimedop.c > index 38a401bb6f..0cdc0fad4b 100644 > --- a/sysdeps/unix/sysv/linux/semtimedop.c > +++ b/sysdeps/unix/sysv/linux/semtimedop.c > @@ -42,7 +42,7 @@ __semtimedop64 (int semid, struct sembuf *sops, size_t nsops, > #ifdef __ASSUME_TIME64_SYSCALLS > return semtimedop_syscall (semid, sops, nsops, timeout); > #else > - bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec); > + bool need_time64 = timeout != NULL && !in_int32_t_range (timeout->tv_sec); > if (need_time64) > { > int r = semtimedop_syscall (semid, sops, nsops, timeout); > diff --git a/sysdeps/unix/sysv/linux/setitimer.c b/sysdeps/unix/sysv/linux/setitimer.c > index 6d5183aa63..55e6003e0f 100644 > --- a/sysdeps/unix/sysv/linux/setitimer.c > +++ b/sysdeps/unix/sysv/linux/setitimer.c > @@ -32,8 +32,8 @@ __setitimer64 (__itimer_which_t which, > #else > struct __itimerval32 new_value_32; > > - if (! in_time_t_range (new_value->it_interval.tv_sec) > - || ! in_time_t_range (new_value->it_value.tv_sec)) > + if (! in_int32_t_range (new_value->it_interval.tv_sec) > + || ! in_int32_t_range (new_value->it_value.tv_sec)) > { > __set_errno (EOVERFLOW); > return -1; > diff --git a/sysdeps/unix/sysv/linux/setsockopt.c b/sysdeps/unix/sysv/linux/setsockopt.c > index 4292f411f3..6506ca03ab 100644 > --- a/sysdeps/unix/sysv/linux/setsockopt.c > +++ b/sysdeps/unix/sysv/linux/setsockopt.c > @@ -54,7 +54,7 @@ setsockopt32 (int fd, int level, int optname, const void *optval, > } > > struct __timeval64 *tv64 = (struct __timeval64 *) optval; > - if (! in_time_t_range (tv64->tv_sec)) > + if (! in_int32_t_range (tv64->tv_sec)) > { > __set_errno (EOVERFLOW); > break; > diff --git a/sysdeps/unix/sysv/linux/sigtimedwait.c b/sysdeps/unix/sysv/linux/sigtimedwait.c > index 069fa1d5e4..1993e3f85a 100644 > --- a/sysdeps/unix/sysv/linux/sigtimedwait.c > +++ b/sysdeps/unix/sysv/linux/sigtimedwait.c > @@ -31,7 +31,7 @@ __sigtimedwait64 (const sigset_t *set, siginfo_t *info, > result = SYSCALL_CANCEL (rt_sigtimedwait_time64, set, info, timeout, > __NSIG_BYTES); > #else > - bool need_time64 = timeout != NULL && !in_time_t_range (timeout->tv_sec); > + bool need_time64 = timeout != NULL && !in_int32_t_range (timeout->tv_sec); > if (need_time64) > { > result = SYSCALL_CANCEL (rt_sigtimedwait_time64, set, info, timeout, > diff --git a/sysdeps/unix/sysv/linux/timer_settime.c b/sysdeps/unix/sysv/linux/timer_settime.c > index 518fed9c59..056e0aff59 100644 > --- a/sysdeps/unix/sysv/linux/timer_settime.c > +++ b/sysdeps/unix/sysv/linux/timer_settime.c > @@ -46,8 +46,8 @@ ___timer_settime64 (timer_t timerid, int flags, > # endif > struct itimerspec its32, oits32; > > - if (! in_time_t_range ((value->it_value).tv_sec) > - || ! in_time_t_range ((value->it_interval).tv_sec)) > + if (! in_int32_t_range ((value->it_value).tv_sec) > + || ! in_int32_t_range ((value->it_interval).tv_sec)) > { > __set_errno (EOVERFLOW); > return -1; > diff --git a/sysdeps/unix/sysv/linux/timerfd_settime.c b/sysdeps/unix/sysv/linux/timerfd_settime.c > index 59282bf1ad..c5f3f14c3b 100644 > --- a/sysdeps/unix/sysv/linux/timerfd_settime.c > +++ b/sysdeps/unix/sysv/linux/timerfd_settime.c > @@ -33,8 +33,8 @@ __timerfd_settime64 (int fd, int flags, const struct __itimerspec64 *value, > #ifdef __ASSUME_TIME64_SYSCALLS > return INLINE_SYSCALL_CALL (timerfd_settime64, fd, flags, value, ovalue); > #else > - bool need_time64 = !in_time_t_range (value->it_value.tv_sec) > - || !in_time_t_range (value->it_interval.tv_sec); > + bool need_time64 = !in_int32_t_range (value->it_value.tv_sec) > + || !in_int32_t_range (value->it_interval.tv_sec); > if (need_time64) > { > int r = INLINE_SYSCALL_CALL (timerfd_settime64, fd, flags, value, > diff --git a/sysdeps/unix/sysv/linux/utimensat.c b/sysdeps/unix/sysv/linux/utimensat.c > index 5662ecc9aa..54a5e47618 100644 > --- a/sysdeps/unix/sysv/linux/utimensat.c > +++ b/sysdeps/unix/sysv/linux/utimensat.c > @@ -41,9 +41,9 @@ __utimensat64_helper (int fd, const char *file, > > bool need_time64 = tsp64 != NULL > && ((!TS_SPECIAL (tsp64[0]) > - && !in_time_t_range (tsp64[0].tv_sec)) > + && !in_int32_t_range (tsp64[0].tv_sec)) > || (!TS_SPECIAL (tsp64[1]) > - && !in_time_t_range (tsp64[1].tv_sec))); > + && !in_int32_t_range (tsp64[1].tv_sec))); > if (need_time64) > { > int r = INLINE_SYSCALL_CALL (utimensat_time64, fd, file, &tsp64[0],