Auto merge of #44220 - kennytm:fix-44216-instance-plus-max-duration-should-panic, r=alexcrichton

Properly detect overflow in Instance ± Duration.

Fix #44216.
Fix #42622

The computation `Instant::now() + Duration::from_secs(u64::max_value())` now panics. The call `receiver.recv_timeout(Duration::from_secs(u64::max_value()))`, which involves such time addition, will also panic.

The reason #44216 arises is because of an unchecked cast from `u64` to `i64`, making the duration equivalent to -1 second.

Note that the current implementation is over-conservative, since e.g. (-2⁶²) + (2⁶³) is perfectly fine for an `i64`, yet this is rejected because (2⁶³) overflows the `i64`.

Author: bors

src/libstd/sys/redox/time.rs

@@ -12,6 +12,7 @@ use cmp::Ordering;
 use fmt;
 use sys::{cvt, syscall};
 use time::Duration;
+use convert::TryInto;
 
 const NSEC_PER_SEC: u64 = 1_000_000_000;
 
@@ -40,8 +41,12 @@ impl Timespec {
     }
 
     fn add_duration(&self, other: &Duration) -> Timespec {
-        let secs = (self.t.tv_sec as i64).checked_add(other.as_secs() as i64);
-        let mut secs = secs.expect("overflow when adding duration to time");
+        let mut secs = other
+            .as_secs()
+            .try_into() // <- target type would be `i64`
+            .ok()
+            .and_then(|secs| self.t.tv_sec.checked_add(secs))
+            .expect("overflow when adding duration to time");
 
         // Nano calculations can't overflow because nanos are <1B which fit
         // in a u32.
@@ -53,16 +58,19 @@ impl Timespec {
         }
         Timespec {
             t: syscall::TimeSpec {
-                tv_sec: secs as i64,
+                tv_sec: secs,
                 tv_nsec: nsec as i32,
             },
         }
     }
 
     fn sub_duration(&self, other: &Duration) -> Timespec {
-        let secs = (self.t.tv_sec as i64).checked_sub(other.as_secs() as i64);
-        let mut secs = secs.expect("overflow when subtracting duration \
-                                    from time");
+        let mut secs = other
+            .as_secs()
+            .try_into() // <- target type would be `i64`
+            .ok()
+            .and_then(|secs| self.t.tv_sec.checked_sub(secs))
+            .expect("overflow when subtracting duration from time");
 
         // Similar to above, nanos can't overflow.
         let mut nsec = self.t.tv_nsec as i32 - other.subsec_nanos() as i32;
@@ -73,7 +81,7 @@ impl Timespec {
         }
         Timespec {
             t: syscall::TimeSpec {
-                tv_sec: secs as i64,
+                tv_sec: secs,
                 tv_nsec: nsec as i32,
             },
         }

src/libstd/sys/unix/time.rs

@@ -13,6 +13,7 @@ use libc;
 use time::Duration;
 
 pub use self::inner::{Instant, SystemTime, UNIX_EPOCH};
+use convert::TryInto;
 
 const NSEC_PER_SEC: u64 = 1_000_000_000;
 
@@ -41,8 +42,12 @@ impl Timespec {
     }
 
     fn add_duration(&self, other: &Duration) -> Timespec {
-        let secs = (self.t.tv_sec as i64).checked_add(other.as_secs() as i64);
-        let mut secs = secs.expect("overflow when adding duration to time");
+        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))
+            .expect("overflow when adding duration to time");
 
         // Nano calculations can't overflow because nanos are <1B which fit
         // in a u32.
@@ -54,16 +59,19 @@ impl Timespec {
         }
         Timespec {
             t: libc::timespec {
-                tv_sec: secs as libc::time_t,
+                tv_sec: secs,
                 tv_nsec: nsec as libc::c_long,
             },
         }
     }
 
     fn sub_duration(&self, other: &Duration) -> Timespec {
-        let secs = (self.t.tv_sec as i64).checked_sub(other.as_secs() as i64);
-        let mut secs = secs.expect("overflow when subtracting duration \
-                                    from time");
+        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))
+            .expect("overflow when subtracting duration from time");
 
         // Similar to above, nanos can't overflow.
         let mut nsec = self.t.tv_nsec as i32 - other.subsec_nanos() as i32;
@@ -74,7 +82,7 @@ impl Timespec {
         }
         Timespec {
             t: libc::timespec {
-                tv_sec: secs as libc::time_t,
+                tv_sec: secs,
                 tv_nsec: nsec as libc::c_long,
             },
         }

src/libstd/sys/windows/time.rs

@@ -16,6 +16,7 @@ use sys::c;
 use sys::cvt;
 use sys_common::mul_div_u64;
 use time::Duration;
+use convert::TryInto;
 
 const NANOS_PER_SEC: u64 = 1_000_000_000;
 const INTERVALS_PER_SEC: u64 = NANOS_PER_SEC / 100;
@@ -173,9 +174,11 @@ impl From<c::FILETIME> for SystemTime {
 }
 
 fn dur2intervals(d: &Duration) -> i64 {
-    d.as_secs().checked_mul(INTERVALS_PER_SEC).and_then(|i| {
-        i.checked_add(d.subsec_nanos() as u64 / 100)
-    }).expect("overflow when converting duration to intervals") as i64
+    d.as_secs()
+        .checked_mul(INTERVALS_PER_SEC)
+        .and_then(|i| i.checked_add(d.subsec_nanos() as u64 / 100))
+        .and_then(|i| i.try_into().ok())
+        .expect("overflow when converting duration to intervals")
 }
 
 fn intervals2dur(intervals: u64) -> Duration {

src/libstd/time/mod.rs

@@ -509,7 +509,7 @@ mod tests {
                 let dur = dur.duration();
                 assert!(a > b);
                 assert_almost_eq!(b + dur, a);
-                assert_almost_eq!(b - dur, a);
+                assert_almost_eq!(a - dur, b);
             }
         }
 
@@ -520,9 +520,12 @@ mod tests {
 
         assert_almost_eq!(a - second + second, a);
 
-        let eighty_years = second * 60 * 60 * 24 * 365 * 80;
-        assert_almost_eq!(a - eighty_years + eighty_years, a);
-        assert_almost_eq!(a - (eighty_years * 10) + (eighty_years * 10), a);
+        // A difference of 80 and 800 years cannot fit inside a 32-bit time_t
+        if !(cfg!(unix) && ::mem::size_of::<::libc::time_t>() <= 4) {
+            let eighty_years = second * 60 * 60 * 24 * 365 * 80;
+            assert_almost_eq!(a - eighty_years + eighty_years, a);
+            assert_almost_eq!(a - (eighty_years * 10) + (eighty_years * 10), a);
+        }
 
         let one_second_from_epoch = UNIX_EPOCH + Duration::new(1, 0);
         let one_second_from_epoch2 = UNIX_EPOCH + Duration::new(0, 500_000_000)

src/test/run-fail/issue-44216-add-instant.rs

@@ -0,0 +1,18 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// error-pattern:overflow
+
+use std::time::{Instant, Duration};
+
+fn main() {
+    let now = Instant::now();
+    let _ = now + Duration::from_secs(u64::max_value());
+}

src/test/run-fail/issue-44216-add-system-time.rs

@@ -0,0 +1,18 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// error-pattern:overflow
+
+use std::time::{Duration, SystemTime};
+
+fn main() {
+    let now = SystemTime::now();
+    let _ = now + Duration::from_secs(u64::max_value());
+}

src/test/run-fail/issue-44216-sub-instant.rs

@@ -0,0 +1,18 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// error-pattern:overflow
+
+use std::time::{Instant, Duration};
+
+fn main() {
+    let now = Instant::now();
+    let _ = now - Duration::from_secs(u64::max_value());
+}

src/test/run-fail/issue-44216-sub-system-time.rs

@@ -0,0 +1,18 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// error-pattern:overflow
+
+use std::time::{Duration, SystemTime};
+
+fn main() {
+    let now = SystemTime::now();
+    let _ = now - Duration::from_secs(u64::max_value());
+}