Re: [committed] libstdc++: Use custom timespec in system calls [PR 93421]

[Mike Crowe <mac@mcrowe.com>]

On Wednesday 18 November 2020 at 20:22:53 +0000, Jonathan Wakely wrote:
> On 18/11/20 00:01 +0000, Jonathan Wakely wrote:
> > On 14/11/20 14:23 +0000, Jonathan Wakely wrote:
> > > On Sat, 14 Nov 2020, 13:30 Mike Crowe wrote:
> > > > > @@ -195,7 +205,7 @@ namespace
> > > > >           if (__s.count() < 0) [[unlikely]]
> > > > >             return false;
> > > > > 
> > > > > -         struct timespec rt;
> > > > > +         syscall_timespec rt;
> > > > >           if (__s.count() > __int_traits<time_t>::__max) [[unlikely]]
> > > > >             rt.tv_sec = __int_traits<time_t>::__max;
> > > > 
> > > > Do these now need to be __int_traits<long>::__max in case time_t is 64-bit
> > > > yet syscall_timespec is using 32-bit long?
> > > > 
> > > 
> > > Ah yes. Maybe decltype(rt.tv_sec).
> > 
> > I'll fix that in the next patch.
> 
> And here's that next patch. I'm testing this and will commit if all
> goes well.
> 
> 

> commit 11dfb2a0cca90b277f6bfff9306339f4424bbbdb
> Author: Jonathan Wakely <jwakely@redhat.com>
> Date:   Wed Nov 18 15:05:25 2020
> 
>     libstdc++: Fix overflow checks to use the correct "time_t" [PR 93456]
>     
>     I recently added overflow checks to src/c++11/futex.cc for PR 93456, but
>     then changed the type of the timespec for PR 93421. This meant the
>     overflow checks were no longer using the right range, because the
>     variable being written to might be smaller than time_t.
>     
>     This introduces new typedef that corresponds to the tv_sec member of the
>     struct being passed to the syscall, and uses that typedef in the range
>     checks.
>     
>     libstdc++-v3/ChangeLog:
>     
>             PR libstdc++/93421
>             PR libstdc++/93456
>             * src/c++11/futex.cc (syscall_time_t): New typedef for
>             the type of the syscall_timespec::tv_sec member.
>             (relative_timespec, _M_futex_wait_until)
>             (_M_futex_wait_until_steady): Use syscall_time_t in overflow
>             checks, not time_t.
> 
> diff --git a/libstdc++-v3/src/c++11/futex.cc b/libstdc++-v3/src/c++11/futex.cc
> index 33e2097e19cf..290201ae2540 100644
> --- a/libstdc++-v3/src/c++11/futex.cc
> +++ b/libstdc++-v3/src/c++11/futex.cc
> @@ -64,8 +64,10 @@ namespace
>    // The SYS_futex syscall still uses the old definition of timespec
>    // where tv_sec is 32 bits, so define a type that matches that.
>    struct syscall_timespec { long tv_sec; long tv_nsec; };
> +  using syscall_time_t = long;
>  #else
>    using syscall_timespec = ::timespec;
> +  using syscall_time_t = time_t;
>  #endif
>  
>    // Return the relative duration from (now_s + now_ns) to (abs_s + abs_ns)
> @@ -86,9 +88,9 @@ namespace
>      const auto rel_s = abs_s.count() - now_s;
>  
>      // Convert the absolute timeout to a relative timeout, without overflow.
> -    if (rel_s > __int_traits<time_t>::__max) [[unlikely]]
> +    if (rel_s > __int_traits<syscall_time_t>::__max) [[unlikely]]
>        {
> -	rt.tv_sec = __int_traits<time_t>::__max;
> +	rt.tv_sec = __int_traits<syscall_time_t>::__max;
>  	rt.tv_nsec = 999999999;
>        }
>      else
> @@ -130,8 +132,8 @@ namespace
>  	      return false;
>  
>  	    syscall_timespec rt;
> -	    if (__s.count() > __int_traits<time_t>::__max) [[unlikely]]
> -	      rt.tv_sec = __int_traits<time_t>::__max;
> +	    if (__s.count() > __int_traits<syscall_time_t>::__max) [[unlikely]]
> +	      rt.tv_sec = __int_traits<syscall_time_t>::__max;
>  	    else
>  	      rt.tv_sec = __s.count();
>  	    rt.tv_nsec = __ns.count();
> @@ -206,8 +208,8 @@ namespace
>  	      return false;
>  
>  	    syscall_timespec rt;
> -	    if (__s.count() > __int_traits<time_t>::__max) [[unlikely]]
> -	      rt.tv_sec = __int_traits<time_t>::__max;
> +	    if (__s.count() > __int_traits<syscall_time_t>::__max) [[unlikely]]
> +	      rt.tv_sec = __int_traits<syscall_time_t>::__max;
>  	    else
>  	      rt.tv_sec = __s.count();
>  	    rt.tv_nsec = __ns.count();

LGTM.

Mike.