[committed] libstdc++: Optimise std::future::wait_for and fix futex polling

[Jonathan Wakely]

On 13/11/20 21:12 +0000, Jonathan Wakely wrote:
>On 13/11/20 20:29 +0000, Mike Crowe via Libstdc++ wrote:
>>On Friday 13 November 2020 at 17:25:22 +0000, Jonathan Wakely wrote:
>>>+  // Return the relative duration from (now_s + now_ns) to (abs_s + abs_ns)
>>>+  // as a timespec.
>>>+  struct timespec
>>>+  relative_timespec(chrono::seconds abs_s, chrono::nanoseconds abs_ns,
>>>+		    time_t now_s, long now_ns)
>>>+  {
>>>+    struct timespec rt;
>>>+
>>>+    // Did we already time out?
>>>+    if (now_s > abs_s.count())
>>>+      {
>>>+	rt.tv_sec = -1;
>>>+	return rt;
>>>+      }
>>>+
>>>+    auto rel_s = abs_s.count() - now_s;
>>>+
>>>+    // Avoid overflows
>>>+    if (rel_s > __gnu_cxx::__int_traits<time_t>::__max)
>>>+      rel_s = __gnu_cxx::__int_traits<time_t>::__max;
>>>+    else if (rel_s < __gnu_cxx::__int_traits<time_t>::__min)
>>>+      rel_s = __gnu_cxx::__int_traits<time_t>::__min;
>>
>>I may be missing something, but if the line above executes...
>>
>>>+
>>>+    // Convert the absolute timeout value to a relative timeout
>>>+    rt.tv_sec = rel_s;
>>>+    rt.tv_nsec = abs_ns.count() - now_ns;
>>>+    if (rt.tv_nsec < 0)
>>>+      {
>>>+	rt.tv_nsec += 1000000000;
>>>+	--rt.tv_sec;
>>
>>...and so does this line above, then I think that we'll end up
>>underflowing. (Presumably rt.tv_sec will wrap round to being some time in
>>2038 on most 32-bit targets.)
>
>Ugh.
>
>>I'm currently trying to persuade myself that this can actually happen and
>>if so work out how to come up with a test case for it.
>
>Maybe something like:
>
>auto d = chrono::floor<chrono::seconds>(system_clock::now().time_since_epoch() - seconds(INT_MAX + 2LL));
>fut.wait_until(system_clock::time_point(d));
>
>This will create a sys_time with a value that is slightly more than
>INT_MAX seconds before the current time, with a zero nanoseconds
>component. The difference between the gettimeofday result and this
>time will be slightly more negative than INT_MIN and so will overflow
>a 32-bit time_t, causing the code to use __int_traits<time_t>::__min.
>As long as the gettimeofday call doesn't happen to also have a zero
>nanoseconds component, the difference of the nanoseconds values will
>be negative, and we will decrement the time_t value.
>
>>I don't believe that this part changed in your later patch.
>
>Right. Thanks for catching this.
>
>The attached patch should fix it. There's no point clamping very
>negative values to the minimum time_t value, since any relative
>timeout less than zero has already passed. So we can just use -1
>there (and not bother with the tv_nsec field at all):
>
>    else if (rel_s <= 0) [[unlikely]]
>      {
>	rt.tv_sec = -1;
>      }

Gah, no, this has to be < 0 not <= 0 otherwise we treat 0.1s as -1.1

And we should probably lose the [[unlikely]] there, since it's quite
feasible that an absolute time in the recent past gets used.