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

Fix checked_{add,sub}_duration incorrectly returning None when other has more than i64::MAX seconds #103056

Merged
merged 1 commit into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions library/std/src/sys/hermit/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,7 @@ impl Timespec {
}

fn checked_add_duration(&self, other: &Duration) -> Option<Timespec> {
let mut secs = other
.as_secs()
.try_into() // <- target type would be `libc::time_t`
.ok()
.and_then(|secs| self.t.tv_sec.checked_add(secs))?;
let mut secs = self.tv_sec.checked_add_unsigned(other.as_secs())?;

// Nano calculations can't overflow because nanos are <1B which fit
// in a u32.
Expand All @@ -56,11 +52,7 @@ impl Timespec {
}

fn checked_sub_duration(&self, other: &Duration) -> Option<Timespec> {
let mut secs = other
.as_secs()
.try_into() // <- target type would be `libc::time_t`
.ok()
.and_then(|secs| self.t.tv_sec.checked_sub(secs))?;
let mut secs = self.tv_sec.checked_sub_unsigned(other.as_secs())?;

// Similar to above, nanos can't overflow.
let mut nsec = self.t.tv_nsec as i32 - other.subsec_nanos() as i32;
Expand Down
4 changes: 2 additions & 2 deletions library/std/src/sys/solid/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ impl SystemTime {
}

pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
Some(SystemTime(self.0.checked_add(other.as_secs().try_into().ok()?)?))
Some(SystemTime(self.0.checked_add_unsigned(other.as_secs())?))
}

pub fn checked_sub_duration(&self, other: &Duration) -> Option<SystemTime> {
Some(SystemTime(self.0.checked_sub(other.as_secs().try_into().ok()?)?))
Some(SystemTime(self.0.checked_sub_unsigned(other.as_secs())?))
}
}
16 changes: 4 additions & 12 deletions library/std/src/sys/unix/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,7 @@ impl Timespec {
}

pub fn checked_add_duration(&self, other: &Duration) -> Option<Timespec> {
let mut secs = other
.as_secs()
.try_into() // <- target type would be `i64`
.ok()
.and_then(|secs| self.tv_sec.checked_add(secs))?;
let mut secs = self.tv_sec.checked_add_unsigned(other.as_secs())?;

// Nano calculations can't overflow because nanos are <1B which fit
// in a u32.
Expand All @@ -115,23 +111,19 @@ impl Timespec {
nsec -= NSEC_PER_SEC as u32;
secs = secs.checked_add(1)?;
}
Some(Timespec::new(secs, nsec as i64))
Some(Timespec::new(secs, nsec.into()))
}

pub fn checked_sub_duration(&self, other: &Duration) -> Option<Timespec> {
let mut secs = other
.as_secs()
.try_into() // <- target type would be `i64`
.ok()
.and_then(|secs| self.tv_sec.checked_sub(secs))?;
let mut secs = self.tv_sec.checked_sub_unsigned(other.as_secs())?;

// Similar to above, nanos can't overflow.
let mut nsec = self.tv_nsec.0 as i32 - other.subsec_nanos() as i32;
if nsec < 0 {
nsec += NSEC_PER_SEC as i32;
secs = secs.checked_sub(1)?;
}
Some(Timespec::new(secs, nsec as i64))
Some(Timespec::new(secs, nsec.into()))
}

#[allow(dead_code)]
Expand Down
27 changes: 27 additions & 0 deletions library/std/src/time/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use super::{Duration, Instant, SystemTime, UNIX_EPOCH};
use core::fmt::Debug;
#[cfg(not(target_arch = "wasm32"))]
use test::{black_box, Bencher};

Expand Down Expand Up @@ -193,6 +194,32 @@ fn since_epoch() {
assert!(a < hundred_twenty_years);
}

#[test]
fn big_math() {
// Check that the same result occurs when adding/subtracting each duration one at a time as when
// adding/subtracting them all at once.
#[track_caller]
fn check<T: Eq + Copy + Debug>(start: Option<T>, op: impl Fn(&T, Duration) -> Option<T>) {
const DURATIONS: [Duration; 2] =
[Duration::from_secs(i64::MAX as _), Duration::from_secs(50)];
if let Some(start) = start {
assert_eq!(
op(&start, DURATIONS.into_iter().sum()),
DURATIONS.into_iter().try_fold(start, |t, d| op(&t, d))
)
}
}

check(SystemTime::UNIX_EPOCH.checked_sub(Duration::from_secs(100)), SystemTime::checked_add);
check(SystemTime::UNIX_EPOCH.checked_add(Duration::from_secs(100)), SystemTime::checked_sub);

let instant = Instant::now();
check(instant.checked_sub(Duration::from_secs(100)), Instant::checked_add);
check(instant.checked_sub(Duration::from_secs(i64::MAX as _)), Instant::checked_add);
check(instant.checked_add(Duration::from_secs(100)), Instant::checked_sub);
check(instant.checked_add(Duration::from_secs(i64::MAX as _)), Instant::checked_sub);
}

macro_rules! bench_instant_threaded {
($bench_name:ident, $thread_count:expr) => {
#[bench]
Expand Down