Stl: <chrono>: std::chrono::steady_clock::now() overflow

[slay-er]

Describe the bug
I encountered an overflow issue with std::chrono::steady_clock::now() after ~923 seconds (~15 minutes).

Just copied some source from the original implementation to demonstrate the issue.

Test sample

#include <cassert>
#include <chrono>

_NODISCARD static long long _Scale_large_counter(const long long _Ctr, const long long _Freq) noexcept {
    const long long _Whole = (_Ctr / _Freq) * std::nano::den;
    const long long _Part = (_Ctr % _Freq) * std::nano::den / _Freq;
    return _Whole + _Part;
}

int main(int argc, char* argv[])
{
    static volatile long long _Cached_freq = LLONG_MAX;
    static volatile long long _Cached_ctr_base = LLONG_MAX;
    static volatile long long _Cached_result_base = LLONG_MAX;

    {
        const long long _Freq = _Query_perf_frequency();
        const long long _Ctr = _Query_perf_counter();
        const long long _Result = _Scale_large_counter(_Ctr, _Freq);

        _Cached_freq = _Freq;
        _Cached_ctr_base = _Ctr;
        _Cached_result_base = _Result;
    }

    const long long _Freq_from_cache = _Cached_freq;
    const long long _Ctr_base = _Cached_ctr_base;
    const long long _Result_base = _Cached_result_base;

    for (long long i = 1; i > 0; i += 1) {
        const long long _Ctr = _Ctr_base + i * 10000000;
        const long long now = _Result_base + (_Ctr - _Ctr_base) * std::nano::den / _Freq_from_cache;
        assert(now > _Result_base);
    }

    return 0;
}

Expected behavior
The overflow should happen at the earliest after 100 years as described on MSDN.
https://docs.microsoft.com/en-us/windows/win32/sysinfo/acquiring-high-resolution-time-stamps

How often does QPC roll over?

Not less than 100 years from the most recent system boot, and potentially longer based on the underlying hardware timer used. For most applications, rollover isn't a concern.

STL version

• Option 1: Visual Studio version
  Microsoft Visual Studio Enterprise 2019 Preview Version 16.7.0 Preview 3.0

Additional context
I think the formula to compute _now_ is wrong:
_Result_base + (_Ctr - _Ctr_base) * std::nano::den / _Freq_from_cache

The multiplication by std::nano::den results in a negative value which then is divided.

The formula should be:
_Result_base + (_Ctr - _Ctr_base) * (std::nano::den / _Freq_from_cache)

Also tracked by DevCom-1105325 and Microsoft-internal VSO-1153359 / AB#1153359.

[AlexGuteniev]

I see. It is my fault. I contributed #653 , which was based on a wrong calculation, Sorry.

[AlexGuteniev]

Reverting in #972 , thanks for reporting!

[MikeGitb]

Did this already land in 16.6?

[StephanTLavavej]

@MikeGitb No, the Changelog recorded that #653 was merged for 16.7. Unfortunately, 16.7 Preview 5 has locked down for release, so 16.7.0 will ship for production with this bug. @CaseyCarter suggested porting this fix to 16.7.1 and I agree (silent bad codegen in a popular function should meet the bar).

[StephanTLavavej]

Thanks for reporting this bug, we've merged the revert-fix for VS 2019 16.8 Preview 1. We'll also look into getting approval to fix this in servicing for 16.7.x.

[AlexGuteniev]

16.7.0 will ship for production

Is it not possible to cancel 16.7.0, and release 16.7.1? I'm afraid the consequences could be too bad for compiling with this bug in

[CaseyCarter]

16.7.0 will ship for production

Is it not possible to cancel 16.7.0, and release 16.7.1? I'm afraid the consequences could be too bad for compiling with this bug in

Good news: this will be fixed in a 16.7 Preview before GA, which we thought couldn't happen this late in the cycle.