microsoft / microsoft/STL

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

Open
#4,238 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug chrono
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

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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.