Skip to content

Commit

Permalink
Fix checked_{add,sub}_duration incorrectly returning None when `o…
Browse files Browse the repository at this point in the history
…ther` has more than `i64::MAX` seconds
  • Loading branch information
beetrees committed Oct 14, 2022
1 parent 9b0a099 commit 5def753
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 24 deletions.
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

0 comments on commit 5def753

Please sign in to comment.