Skip to content

Commit 8d641ba

Browse files
committed
Repro duration_since regression from issue 146228
1 parent e10aa88 commit 8d641ba

File tree

3 files changed

+26
-17
lines changed

3 files changed

+26
-17
lines changed

library/std/src/sys/pal/hermit/time.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,14 @@ impl Timespec {
2929
if self >= other {
3030
Ok(if self.t.tv_nsec >= other.t.tv_nsec {
3131
Duration::new(
32-
(self.t.tv_sec - other.t.tv_sec) as u64,
32+
self.t.tv_sec.wrapping_sub(other.t.tv_sec) as u64,
3333
(self.t.tv_nsec - other.t.tv_nsec) as u32,
3434
)
3535
} else {
36+
// Logic here is identical to Unix version of `Timestamp::sub_timspec`,
37+
// check comments there why `- 1` does not underflow.
3638
Duration::new(
37-
(self.t.tv_sec - 1 - other.t.tv_sec) as u64,
39+
(self.t.tv_sec - 1).wrapping_sub(other.t.tv_sec) as u64,
3840
(self.t.tv_nsec + NSEC_PER_SEC - other.t.tv_nsec) as u32,
3941
)
4042
})

library/std/src/sys/pal/unix/time.rs

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -135,27 +135,18 @@ impl Timespec {
135135

136136
pub fn sub_timespec(&self, other: &Timespec) -> Result<Duration, Duration> {
137137
if self >= other {
138-
// NOTE(eddyb) two aspects of this `if`-`else` are required for LLVM
139-
// to optimize it into a branchless form (see also #75545):
140-
//
141-
// 1. `self.tv_sec - other.tv_sec` shows up as a common expression
142-
// in both branches, i.e. the `else` must have its `- 1`
143-
// subtraction after the common one, not interleaved with it
144-
// (it used to be `self.tv_sec - 1 - other.tv_sec`)
145-
//
146-
// 2. the `Duration::new` call (or any other additional complexity)
147-
// is outside of the `if`-`else`, not duplicated in both branches
148-
//
149-
// Ideally this code could be rearranged such that it more
150-
// directly expresses the lower-cost behavior we want from it.
151138
let (secs, nsec) = if self.tv_nsec.as_inner() >= other.tv_nsec.as_inner() {
152139
(
153-
(self.tv_sec - other.tv_sec) as u64,
140+
self.tv_sec.wrapping_sub(other.tv_sec) as u64,
154141
self.tv_nsec.as_inner() - other.tv_nsec.as_inner(),
155142
)
156143
} else {
144+
// Following sequence of assertions explain why `self.tv_sec - 1` does not underflow.
145+
debug_assert!(self.tv_nsec < other.tv_nsec);
146+
debug_assert!(self.tv_sec > other.tv_sec);
147+
debug_assert!(self.tv_sec > i64::MIN);
157148
(
158-
(self.tv_sec - other.tv_sec - 1) as u64,
149+
(self.tv_sec - 1).wrapping_sub(other.tv_sec) as u64,
159150
self.tv_nsec.as_inner() + (NSEC_PER_SEC as u32) - other.tv_nsec.as_inner(),
160151
)
161152
};

library/std/tests/time.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,3 +227,19 @@ fn big_math() {
227227
check(instant.checked_add(Duration::from_secs(100)), Instant::checked_sub);
228228
check(instant.checked_add(Duration::from_secs(i64::MAX as _)), Instant::checked_sub);
229229
}
230+
231+
#[test]
232+
#[cfg(unix)]
233+
fn system_time_duration_since_max_range_on_unix() {
234+
// Repro regression https://github.com/rust-lang/rust/issues/146228
235+
236+
// Min and max values of `SystemTime` on Unix.
237+
let min = SystemTime::UNIX_EPOCH - (Duration::new(i64::MAX as u64 + 1, 0));
238+
let max = SystemTime::UNIX_EPOCH + (Duration::new(i64::MAX as u64, 999_999_999));
239+
240+
let delta_a = max.duration_since(min).expect("duration_since overflow");
241+
let delta_b = min.duration_since(max).expect_err("duration_since overflow").duration();
242+
243+
assert_eq!(Duration::MAX, delta_a);
244+
assert_eq!(Duration::MAX, delta_b);
245+
}

0 commit comments

Comments
 (0)