Re: [PATCH v2] time: Fix overflow itimer tests on 32-bit systems

[Stafford Horne via Libc-alpha <libc-alpha@sourceware.org>]

On Wed, Aug 11, 2021 at 05:30:40PM -0300, Adhemerval Zanella wrote:
> 
> 
> On 06/08/2021 06:42, Stafford Horne wrote:
> > On the port of OpenRISC I am working on and it appears the rv32 port
> > we have sets __TIMESIZE == 64 && __WORDSIZE == 32.  This causes the
> > size of time_t to be 8 bytes, but the tv_sec in the kernel is still 32-bit
> > causing truncation.
> > 
> > The truncations are unavoidable on these systems so skip the
> > testing/failures by guarding with __KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64.
> > 
> > Also, futher in the tests and in other parts of code checking for time_t
> > overflow does not work on 32-bit systems when time_t is 64-bit.  As
> > suggested by Adhemerval, update the in_time_t_range function to assume
> > 32-bits by using int32_t.
> > 
> > This also brings in the header for stdint.h so we can update other
> > usages of __int32_t to int32_t as suggested by Adhemerval.
> > ---
> > 
> > Hello,
> > 
> > Sorry for the delay to get this out I have been busy on the hardware side of
> > openrisc the last month so I haven't been able to spend time on getting this
> > out.
> > 
> > The patch ends up doing a test fix and some lib code fixes, I can split it to
> > separate small patches.  But since as a whole it's small I feel leaving it
> > together is best.
> 
> LGTM, thanks.
> 
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

Thanks for the review.

> > 
> > -Stafford
> > 
> >  include/time.h    | 10 ++++++----
> >  time/tst-itimer.c |  4 ++--
> >  2 files changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/time.h b/include/time.h
> > index 4372bfbd96..ba3c5116cf 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -11,6 +11,7 @@
> >  # include <sys/time.h>
> >  # include <time-clockid.h>
> >  # include <sys/time.h>
> > +# include <stdint.h>
> >  
> >  extern __typeof (strftime_l) __strftime_l;
> >  libc_hidden_proto (__strftime_l)
> > @@ -334,11 +335,12 @@ libc_hidden_proto (__time64)
> >     actual clock ID.  */
> >  #define CLOCK_IDFIELD_SIZE	3
> >  
> > -/* Check whether T fits in time_t.  */
> > +/* 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)
> >  {
> > -  time_t s = t;
> > +  int32_t s = t;
> >    return s == t;
> >  }
> >  
> > @@ -445,8 +447,8 @@ timespec64_to_timeval64 (const struct __timespec64 ts64)
> >     and suseconds_t.  */
> >  struct __timeval32
> >  {
> > -  __int32_t tv_sec;         /* Seconds.  */
> > -  __int32_t tv_usec;        /* Microseconds.  */
> > +  int32_t tv_sec;         /* Seconds.  */
> > +  int32_t tv_usec;        /* Microseconds.  */
> >  };
> >  
> >  /* Conversion functions for converting to/from __timeval32  */
> > diff --git a/time/tst-itimer.c b/time/tst-itimer.c
> > index 929c2b74c7..bd7d7afe83 100644
> > --- a/time/tst-itimer.c
> > +++ b/time/tst-itimer.c
> > @@ -100,7 +100,7 @@ do_test (void)
> >  
> >        /* Linux does not provide 64 bit time_t support for getitimer and
> >  	 setitimer on architectures with 32 bit time_t support.  */
> > -      if (sizeof (__time_t) == 8)
> > +      if (__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64)
> >  	{
> >  	  TEST_COMPARE (setitimer (timers[i], &it, NULL), 0);
> >  	  TEST_COMPARE (setitimer (timers[i], &(struct itimerval) { 0 },
> > @@ -131,7 +131,7 @@ do_test (void)
> >        it.it_interval.tv_usec = 20;
> >        it.it_value.tv_sec = 30;
> >        it.it_value.tv_usec = 40;
> > -      if (sizeof (__time_t) == 8)
> > +      if (__KERNEL_OLD_TIMEVAL_MATCHES_TIMEVAL64)
> >  	{
> >  	  TEST_COMPARE (setitimer (timers[i], &it, NULL), 0);
> >  
> >