Skip to content

Commit 92859e9

Browse files
committed
Repro duration_since regression from issue 146228
1 parent 15283f6 commit 92859e9

File tree

3 files changed

+37
-17
lines changed

3 files changed

+37
-17
lines changed

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,22 @@ impl Timespec {
2626
}
2727

2828
fn sub_timespec(&self, other: &Timespec) -> Result<Duration, Duration> {
29+
fn sub_ge_to_unsigned(a: i64, b: i64) -> u64 {
30+
debug_assert!(a >= b);
31+
a.wrapping_sub(b).cast_unsigned()
32+
}
33+
2934
if self >= other {
35+
// Logic here is identical to Unix version of `Timestamp::sub_timespec`,
36+
// check comments there why operations do not overflow.
3037
Ok(if self.t.tv_nsec >= other.t.tv_nsec {
3138
Duration::new(
32-
(self.t.tv_sec - other.t.tv_sec) as u64,
39+
sub_ge_to_unsigned(self.t.tv_sec, other.t.tv_sec),
3340
(self.t.tv_nsec - other.t.tv_nsec) as u32,
3441
)
3542
} else {
3643
Duration::new(
37-
(self.t.tv_sec - 1 - other.t.tv_sec) as u64,
44+
sub_ge_to_unsigned(self.t.tv_sec - 1, other.t.tv_sec),
3845
(self.t.tv_nsec + NSEC_PER_SEC - other.t.tv_nsec) as u32,
3946
)
4047
})

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

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -134,28 +134,25 @@ impl Timespec {
134134
}
135135

136136
pub fn sub_timespec(&self, other: &Timespec) -> Result<Duration, Duration> {
137+
// When a >= b, the difference fits in u64.
138+
fn sub_ge_to_unsigned(a: i64, b: i64) -> u64 {
139+
debug_assert!(a >= b);
140+
a.wrapping_sub(b).cast_unsigned()
141+
}
142+
137143
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.
151144
let (secs, nsec) = if self.tv_nsec.as_inner() >= other.tv_nsec.as_inner() {
152145
(
153-
(self.tv_sec - other.tv_sec) as u64,
146+
sub_ge_to_unsigned(self.tv_sec, other.tv_sec),
154147
self.tv_nsec.as_inner() - other.tv_nsec.as_inner(),
155148
)
156149
} else {
150+
// Following sequence of assertions explain why `self.tv_sec - 1` does not underflow.
151+
debug_assert!(self.tv_nsec < other.tv_nsec);
152+
debug_assert!(self.tv_sec > other.tv_sec);
153+
debug_assert!(self.tv_sec > i64::MIN);
157154
(
158-
(self.tv_sec - other.tv_sec - 1) as u64,
155+
sub_ge_to_unsigned(self.tv_sec - 1, other.tv_sec),
159156
self.tv_nsec.as_inner() + (NSEC_PER_SEC as u32) - other.tv_nsec.as_inner(),
160157
)
161158
};

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)