[libcxx] Avoid overflows in the windows __libcpp_steady_clock_now()

[mstorsjo]

Summary

As freq.QuadValue can be in the range of 10000000 to 19200000, the multiplication before division makes the calculation overflow and wrap to negative values every 16-30 minutes.

Instead count the whole seconds separately before adding the scaled fractional seconds.

Add a testcase for steady_clock to check that the values returned for now() compare as bigger than the zero time origin; this corresponds to a testcase in Qt [1] [2] (that failed spuriously due to this).

[1] https://bugreports.qt.io/browse/QTBUG-89539
[2] https://code.qt.io/cgit/qt/qtbase.git/tree/tests/auto/corelib/kernel/qdeadlinetimer/tst_qdeadlinetimer.cpp?id=f8de5e54022b8b7471131b7ad55c83b69b2684c0#n569

[amccarth]

This looks correct and straightforward, so LGTM.

There's probably a "clever" solution involving finding the greatest common divisor (once) of nano::den and freq.QuadPart. That would depend on there being a significant common divisor (which I assume is probably true, but not guaranteed). I think the straightforward solution in this patch is better.

libcxx/test/std/utilities/time/time.clock/time.clock.steady/now.pass.cpp

28

I don't know what the libcxx style guidelines say, but my inclination would be to add a comment explaining this assertion. Since the original bug would only occasionally be caught by this assertion, somebody who encounters it might need a hint to look for arithmetic overflows.

[ldionne]

LGTM with or without the test comment.

libcxx/test/std/utilities/time/time.clock/time.clock.steady/now.pass.cpp

28

I think a short comment doesn't hurt. Ideally something that isn't strictly related to the bug, like // make sure t2 didn't wrap around or something along those lines.

libcxx/src/chrono.cpp

__QueryPerformanceFrequency()
     return val;
 }
 
 static steady_clock::time_point __libcpp_steady_clock_now() {
   static const LARGE_INTEGER freq = __QueryPerformanceFrequency();
 
   LARGE_INTEGER counter;
   (void) QueryPerformanceCounter(&counter);
-  return steady_clock::time_point(steady_clock::duration(counter.QuadPart * nano::den / freq.QuadPart));
+  auto seconds = counter.QuadPart / freq.QuadPart;
+  auto fractions = counter.QuadPart % freq.QuadPart;
+  auto dur = seconds * nano::den + fractions * nano::den / freq.QuadPart;
+  return steady_clock::time_point(steady_clock::duration(dur));
 }
 
 #elif defined(CLOCK_MONOTONIC)
 
 static steady_clock::time_point __libcpp_steady_clock_now() {
     struct timespec tp;
     if (0 != clock_gettime(CLOCK_MONOTONIC, &tp))
         __throw_system_error(errno, "clock_gettime(CLOCK_MONOTONIC) failed");

libcxx/test/std/utilities/time/time.clock/time.clock.steady/now.pass.cpp

 #include "test_macros.h"
 
 int main(int, char**)
 {
     typedef std::chrono::steady_clock C;
     C::time_point t1 = C::now();
     C::time_point t2 = C::now();
     assert(t2 >= t1);
+    // make sure t2 didn't wrap around
+    assert(t2 > std::chrono::time_point<C>());
 
   return 0;
 }