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

[Jonathan Wakely <jwakely@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]

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.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2897 bytes --]

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();