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

Detect SystemTime overflow in debug mode #42622

Closed
dtolnay opened this issue Jun 13, 2017 · 1 comment
Closed

Detect SystemTime overflow in debug mode #42622

dtolnay opened this issue Jun 13, 2017 · 1 comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@dtolnay
Copy link
Member

dtolnay commented Jun 13, 2017

use std::time::{Duration, UNIX_EPOCH};
use std::u64;

fn main() {
    let t = UNIX_EPOCH + Duration::new(u64::MAX, 0) + Duration::new(2, 0);
    println!("{}", t.duration_since(UNIX_EPOCH).unwrap().as_secs()); // 1
}

For me (Linux + Rust 1.19.0-nightly) this program prints 1. In debug mode I would expect this overflow to be caught and panic, similar to u64::MAX + 2.

@cuviper
Copy link
Member

cuviper commented Jun 20, 2017

There are explicit checks regardless of debug mode, but the overflow occurs silently when add_duration first converts your u64::MAX to i64. So it basically sees the signed addition UNIX_EPOCH + (-1) + 2 = 1.

Your example starting with i64::MAX instead does panic, "overflow when adding duration to time".

@Mark-Simulacrum Mark-Simulacrum added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jun 23, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 27, 2017
bors added a commit that referenced this issue Sep 10, 2017
…hould-panic, r=alexcrichton

Properly detect overflow in Instance ± Duration.

Fix #44216.
Fix #42622

The computation `Instant::now() + Duration::from_secs(u64::max_value())` now panics. The call `receiver.recv_timeout(Duration::from_secs(u64::max_value()))`, which involves such time addition, will also panic.

The reason #44216 arises is because of an unchecked cast from `u64` to `i64`, making the duration equivalent to -1 second.

Note that the current implementation is over-conservative, since e.g. (-2⁶²) + (2⁶³) is perfectly fine for an `i64`, yet this is rejected because (2⁶³) overflows the `i64`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants