Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<thread>: Avoid overflow in _To_absolute_time() #5237

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

StephanTLavavej
Copy link
Member

Fixes #5234.

We were carefully forming decltype(_Now + _Rel_time) _Abs_time, the common type of the given _Rel_time (which could be coarse or fine) and chrono::steady_clock::now() (which is nanoseconds).

In this bug scenario, _Rel_time and therefore the common type are picoseconds.

The problem was that we weren't careful about our _Forever constant. We were using (chrono::steady_clock::time_point::max)(), but then converting it to the common type (implicitly in the subtraction _Forever - _Rel_time, and in the assignment _Abs_time = _Forever). When the common type is finer-grained, this attempts to multiply the stored value, but it's already the maximum, so we overflow.

The fix is to use the common type for _Forever. This stores the maximum rep with the proper period of the common type, so we won't attempt to multiply it further. (When _Rel_time is coarse, the common type is nanoseconds, so there's no change. When _Rel_time is fine, this means that _Forever is effectively "earlier", because INT64_MAX picoseconds from the epoch happens earlier than INT64_MAX nanoseconds. This is exactly what we want, since we're performing our overflow check and clamping with the common type that we're about to return.)

@StephanTLavavej StephanTLavavej added bug Something isn't working chrono C++20 chrono labels Jan 15, 2025
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner January 15, 2025 17:08
@StephanTLavavej StephanTLavavej self-assigned this Jan 24, 2025
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 27f936a into microsoft:main Jan 24, 2025
39 checks passed
@StephanTLavavej StephanTLavavej deleted the absolute-batman branch January 24, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working chrono C++20 chrono
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

<thread>: std::this_thread::sleep_for overflow with high precision durations
3 participants