`<chrono>`: `file_clock::to_utc()` overflows for `file_time<nanoseconds>`

[StephanTLavavej]

trafficstars

Found by std/time/time.clock/time.clock.file/ostream.pass.cpp in the upcoming libcxx update (which has an unrelated epoch assumption issue):

C:\Temp>type meow.cpp

#include <chrono>
#include <iostream>
using namespace std;
using namespace std::chrono;

int main() {
    const file_time<seconds> ft1{946'688'523s};
    cout << "ft1: " << ft1 << "\n";
    const utc_time<seconds> ut1 = file_clock::to_utc(ft1);
    cout << "ut1: " << ut1 << "\n";

    const file_time<nanoseconds> ft2{ft1};
    cout << "ft2: " << ft2 << "\n";
    const utc_time<nanoseconds> ut2 = file_clock::to_utc(ft2);
    cout << "ut2: " << ut2 << "\n";
}

C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od meow.cpp && meow
meow.cpp
ft1: 1631-01-01 01:02:03
ut1: 1631-01-01 01:02:03
ft2: 2215-07-23 00:36:36.709551616
ut2: 2215-07-23 00:36:36.709551616

Thanks to @statementreply who tracked this down to:

https://github.com/microsoft/STL/blob/0403d19f5461fd15983737c3f01ec34800ea9275/stl/inc/chrono#L2788

https://github.com/microsoft/STL/blob/0403d19f5461fd15983737c3f01ec34800ea9275/stl/inc/chrono#L2804-L2821

__std_fs_file_time_epoch_adjustment is 0x19DB1DED53E8000LL, or 116,444,736,000,000,000 in decimal. That's the number of 100 ns ticks between the 1601 and 1970 epochs. We can comfortably represent this with a 64-bit representation, and we can duration_cast<seconds> it too (as that also uses a 64-bit representation).

However, the subtraction of _File_time.time_since_epoch() with that duration_cast<seconds> value uses the incoming _File_time's potentially very fine-grained period. As the test case demonstrates and @statementreply found, if the input period is nanoseconds, then we overflow. We're doomed if we ever try to represent the epoch adjustment in nanoseconds:

11,644,473,600,000,000,000 == __std_fs_file_time_epoch_adjustment * 100
 9,223,372,036,854,775,807 == 2^63 - 1

I'm not immediately sure what a fix should be, but it seems that since we're computing _Ticks to compare to a sys_days _Cutoff, we ought to be able to coarsely duration_cast<seconds> the _File_time.time_since_epoch() before adjusting.

Aside: We should boil down _CHRONO duration_cast<seconds>(duration{__std_fs_file_time_epoch_adjustment}) to a local constexpr variable, there's no reason to compute it on each call, and stepping into this was deeply confusing me.

[MattStephanson]

I agree with the analysis, but the repro could be improved. duration periods smaller than hours are only guaranteed to have a range of $\pm292$ years. If you're starting from file_time<seconds>, the earliest date that can be represented by utc_time<nanoseconds> is file_time<seconds>{ 2'421'101'564s }, which is in late 1677. Earlier dates are unrepresentable with 64 bits. This date happens to give the correct result but internally has a signed overflow.

[Note for posterity: the file_clock epoch is unspecified, and this comment applies only to MSVC's choice.]

[StephanTLavavej]

Hmm, file_time<nanoseconds> is kind of a doomed type then. jan 1 1601 + (2^63 - 1) nanoseconds is in April 1893, so while there is overlap from Sept 1677 to that, it's not really a useful range.

It does seem that our implementation should be improved, but this libcxx test really is doubly bogus for the epoch assumption and for forming file_time<nanoseconds>.

[FrankHB]

Not only the epoch. It has the additional assumption on the representation. _LIBCPP_HAS_NO_INT128 is out of luck, though.

Too bad there seems no portable way to prevent UB in the user code, given that the epoch can be unspecified. Is there any LWG issue?