From bee923f0df0e4a568811e24dc74af77c464d10f3 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 9 Mar 2022 12:25:46 -0800 Subject: [PATCH] unix: always use 64-bit Timespec --- library/std/src/sys/unix/futex.rs | 9 +- library/std/src/sys/unix/thread_parker.rs | 3 +- library/std/src/sys/unix/time.rs | 120 +++++++++++----------- 3 files changed, 65 insertions(+), 67 deletions(-) diff --git a/library/std/src/sys/unix/futex.rs b/library/std/src/sys/unix/futex.rs index 678c6f0d6ea..c1966d67078 100644 --- a/library/std/src/sys/unix/futex.rs +++ b/library/std/src/sys/unix/futex.rs @@ -24,8 +24,9 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - // Calculate the timeout as an absolute timespec. // // Overflows are rounded up to an infinite timeout (None). - let timespec = - timeout.and_then(|d| Some(Timespec::now(libc::CLOCK_MONOTONIC).checked_add_duration(&d)?)); + let timespec = timeout + .and_then(|d| Some(Timespec::now(libc::CLOCK_MONOTONIC).checked_add_duration(&d)?)) + .and_then(|t| t.to_timespec()); loop { // No need to wait if the value already changed. @@ -41,7 +42,7 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - // identical. It supports absolute timeouts through a flag // in the _umtx_time struct. let umtx_timeout = timespec.map(|t| libc::_umtx_time { - _timeout: t.t, + _timeout: t, _flags: libc::UMTX_ABSTIME, _clockid: libc::CLOCK_MONOTONIC as u32, }); @@ -62,7 +63,7 @@ pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option) - futex as *const AtomicU32, libc::FUTEX_WAIT_BITSET | libc::FUTEX_PRIVATE_FLAG, expected, - timespec.as_ref().map_or(null(), |t| &t.t as *const libc::timespec), + timespec.as_ref().map_or(null(), |t| t as *const libc::timespec), null::(), // This argument is unused for FUTEX_WAIT_BITSET. !0u32, // A full bitmask, to make it behave like a regular FUTEX_WAIT. ) diff --git a/library/std/src/sys/unix/thread_parker.rs b/library/std/src/sys/unix/thread_parker.rs index cf37c01598b..30ed2ec7f54 100644 --- a/library/std/src/sys/unix/thread_parker.rs +++ b/library/std/src/sys/unix/thread_parker.rs @@ -79,7 +79,8 @@ unsafe fn wait_timeout( (Timespec::now(libc::CLOCK_MONOTONIC), dur) }; - let timeout = now.checked_add_duration(&dur).map(|t| t.t).unwrap_or(TIMESPEC_MAX); + let timeout = + now.checked_add_duration(&dur).and_then(|t| t.to_timespec()).unwrap_or(TIMESPEC_MAX); let r = libc::pthread_cond_timedwait(cond, lock, &timeout); debug_assert!(r == libc::ETIMEDOUT || r == 0); } diff --git a/library/std/src/sys/unix/time.rs b/library/std/src/sys/unix/time.rs index d43ceec9c8a..6cf06848399 100644 --- a/library/std/src/sys/unix/time.rs +++ b/library/std/src/sys/unix/time.rs @@ -1,21 +1,23 @@ -use crate::cmp::Ordering; use crate::time::Duration; -use core::hash::{Hash, Hasher}; - pub use self::inner::{Instant, SystemTime, UNIX_EPOCH}; use crate::convert::TryInto; const NSEC_PER_SEC: u64 = 1_000_000_000; -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub(in crate::sys::unix) struct Timespec { - pub t: libc::timespec, + tv_sec: i64, + tv_nsec: i64, } impl Timespec { const fn zero() -> Timespec { - Timespec { t: libc::timespec { tv_sec: 0, tv_nsec: 0 } } + Timespec { tv_sec: 0, tv_nsec: 0 } + } + + fn new(tv_sec: i64, tv_nsec: i64) -> Timespec { + Timespec { tv_sec, tv_nsec } } pub fn sub_timespec(&self, other: &Timespec) -> Result { @@ -23,22 +25,22 @@ impl Timespec { // NOTE(eddyb) two aspects of this `if`-`else` are required for LLVM // to optimize it into a branchless form (see also #75545): // - // 1. `self.t.tv_sec - other.t.tv_sec` shows up as a common expression + // 1. `self.tv_sec - other.tv_sec` shows up as a common expression // in both branches, i.e. the `else` must have its `- 1` // subtraction after the common one, not interleaved with it - // (it used to be `self.t.tv_sec - 1 - other.t.tv_sec`) + // (it used to be `self.tv_sec - 1 - other.tv_sec`) // // 2. the `Duration::new` call (or any other additional complexity) // is outside of the `if`-`else`, not duplicated in both branches // // Ideally this code could be rearranged such that it more // directly expresses the lower-cost behavior we want from it. - let (secs, nsec) = if self.t.tv_nsec >= other.t.tv_nsec { - ((self.t.tv_sec - other.t.tv_sec) as u64, (self.t.tv_nsec - other.t.tv_nsec) as u32) + let (secs, nsec) = if self.tv_nsec >= other.tv_nsec { + ((self.tv_sec - other.tv_sec) as u64, (self.tv_nsec - other.tv_nsec) as u32) } else { ( - (self.t.tv_sec - other.t.tv_sec - 1) as u64, - self.t.tv_nsec as u32 + (NSEC_PER_SEC as u32) - other.t.tv_nsec as u32, + (self.tv_sec - other.tv_sec - 1) as u64, + self.tv_nsec as u32 + (NSEC_PER_SEC as u32) - other.tv_nsec as u32, ) }; @@ -54,63 +56,34 @@ impl Timespec { pub fn checked_add_duration(&self, other: &Duration) -> Option { let mut secs = other .as_secs() - .try_into() // <- target type would be `libc::time_t` + .try_into() // <- target type would be `i64` .ok() - .and_then(|secs| self.t.tv_sec.checked_add(secs))?; + .and_then(|secs| self.tv_sec.checked_add(secs))?; // Nano calculations can't overflow because nanos are <1B which fit // in a u32. - let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32; + let mut nsec = other.subsec_nanos() + self.tv_nsec as u32; if nsec >= NSEC_PER_SEC as u32 { nsec -= NSEC_PER_SEC as u32; secs = secs.checked_add(1)?; } - Some(Timespec { t: libc::timespec { tv_sec: secs, tv_nsec: nsec as _ } }) + Some(Timespec::new(secs, nsec as i64)) } pub fn checked_sub_duration(&self, other: &Duration) -> Option { let mut secs = other .as_secs() - .try_into() // <- target type would be `libc::time_t` + .try_into() // <- target type would be `i64` .ok() - .and_then(|secs| self.t.tv_sec.checked_sub(secs))?; + .and_then(|secs| self.tv_sec.checked_sub(secs))?; // Similar to above, nanos can't overflow. - let mut nsec = self.t.tv_nsec as i32 - other.subsec_nanos() as i32; + let mut nsec = self.tv_nsec as i32 - other.subsec_nanos() as i32; if nsec < 0 { nsec += NSEC_PER_SEC as i32; secs = secs.checked_sub(1)?; } - Some(Timespec { t: libc::timespec { tv_sec: secs, tv_nsec: nsec as _ } }) - } -} - -impl PartialEq for Timespec { - fn eq(&self, other: &Timespec) -> bool { - self.t.tv_sec == other.t.tv_sec && self.t.tv_nsec == other.t.tv_nsec - } -} - -impl Eq for Timespec {} - -impl PartialOrd for Timespec { - fn partial_cmp(&self, other: &Timespec) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for Timespec { - fn cmp(&self, other: &Timespec) -> Ordering { - let me = (self.t.tv_sec, self.t.tv_nsec); - let other = (other.t.tv_sec, other.t.tv_nsec); - me.cmp(&other) - } -} - -impl Hash for Timespec { - fn hash(&self, state: &mut H) { - self.t.tv_sec.hash(state); - self.t.tv_nsec.hash(state); + Some(Timespec::new(secs, nsec as i64)) } } @@ -192,26 +165,35 @@ mod inner { } } + impl From for Timespec { + fn from(t: libc::timeval) -> Timespec { + Timespec::new(t.tv_sec as i64, 1000 * t.tv_usec as i64) + } + } + impl From for SystemTime { fn from(t: libc::timeval) -> SystemTime { - SystemTime::from(libc::timespec { - tv_sec: t.tv_sec, - tv_nsec: (t.tv_usec * 1000) as libc::c_long, - }) + SystemTime { t: Timespec::from(t) } + } + } + + impl From for Timespec { + fn from(t: libc::timespec) -> Timespec { + Timespec::new(t.tv_sec as i64, t.tv_nsec as i64) } } impl From for SystemTime { fn from(t: libc::timespec) -> SystemTime { - SystemTime { t: Timespec { t } } + SystemTime { t: Timespec::from(t) } } } impl fmt::Debug for SystemTime { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("SystemTime") - .field("tv_sec", &self.t.t.tv_sec) - .field("tv_nsec", &self.t.t.tv_nsec) + .field("tv_sec", &self.t.tv_sec) + .field("tv_nsec", &self.t.tv_nsec) .finish() } } @@ -305,8 +287,8 @@ mod inner { impl fmt::Debug for Instant { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("Instant") - .field("tv_sec", &self.t.t.tv_sec) - .field("tv_nsec", &self.t.t.tv_nsec) + .field("tv_sec", &self.t.tv_sec) + .field("tv_nsec", &self.t.tv_nsec) .finish() } } @@ -329,17 +311,23 @@ mod inner { } } + impl From for Timespec { + fn from(t: libc::timespec) -> Timespec { + Timespec::new(t.tv_sec as i64, t.tv_nsec as i64) + } + } + impl From for SystemTime { fn from(t: libc::timespec) -> SystemTime { - SystemTime { t: Timespec { t } } + SystemTime { t: Timespec::from(t) } } } impl fmt::Debug for SystemTime { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("SystemTime") - .field("tv_sec", &self.t.t.tv_sec) - .field("tv_nsec", &self.t.t.tv_nsec) + .field("tv_sec", &self.t.tv_sec) + .field("tv_nsec", &self.t.tv_nsec) .finish() } } @@ -353,7 +341,15 @@ mod inner { pub fn now(clock: clock_t) -> Timespec { let mut t = MaybeUninit::uninit(); cvt(unsafe { libc::clock_gettime(clock, t.as_mut_ptr()) }).unwrap(); - Timespec { t: unsafe { t.assume_init() } } + Timespec::from(unsafe { t.assume_init() }) + } + + pub fn to_timespec(&self) -> Option { + use crate::convert::TryInto; + Some(libc::timespec { + tv_sec: self.tv_sec.try_into().ok()?, + tv_nsec: self.tv_nsec.try_into().ok()?, + }) } } }