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 <jwak...@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.
