`<chrono>`: `file_clock::to_utc()` overflows for `file_time<nanoseconds>`
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 11.1k
- Forks
- 1.7k
- Avg merge
- 4d 15h
- Merged PRs (30d)
- 22
Description
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
__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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the referenced implementation in stl/inc/chrono around lines 2788 and 2804-2821, then reproduce the failure with std/time/time.clock/time.clock.file/ostream.pass.cpp or the provided file_time example. Done means file_clock::to_utc() no longer overflows when adjusting a fine-grained file time, while preserving the existing seconds behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100