Skip to content

Commit

Permalink
Rollup merge of #103056 - beetrees:timespec-bug-fix, r=thomcc
Browse files Browse the repository at this point in the history
Fix `checked_{add,sub}_duration` incorrectly returning `None` when `other` has more than `i64::MAX` seconds

Use `checked_{add,sub}_unsigned` in `checked_{add,sub}_duration` so that the correct result is returned when adding/subtracting durations with more than `i64::MAX` seconds.
  • Loading branch information
Dylan-DPC committed May 5, 2023
2 parents dd9a7bf + 5def753 commit 3502e48
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
Expand Up @@ -40,11 +40,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 @@ -57,11 +53,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
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
Expand Up @@ -113,11 +113,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 @@ -126,23 +122,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
@@ -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 @@ -201,6 +202,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 3502e48

Please sign in to comment.