From 2b512cc329a87202003d41828fe8f6f2fbeaf720 Mon Sep 17 00:00:00 2001 From: The8472 Date: Thu, 16 Sep 2021 18:32:28 +0200 Subject: [PATCH 1/2] fix potential race in AtomicU64 time monotonizer --- library/std/src/time/monotonic.rs | 62 +++++++++++++++++-------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/library/std/src/time/monotonic.rs b/library/std/src/time/monotonic.rs index fa96b7abff6d6..fb1c893275667 100644 --- a/library/std/src/time/monotonic.rs +++ b/library/std/src/time/monotonic.rs @@ -37,35 +37,41 @@ pub mod inner { // This could be a problem for programs that call instants at intervals greater // than 68 years. Interstellar probes may want to ensure that actually_monotonic() is true. let packed = (secs << 32) | nanos; - let old = mono.load(Relaxed); - - if old == UNINITIALIZED || packed.wrapping_sub(old) < u64::MAX / 2 { - mono.store(packed, Relaxed); - raw - } else { - // Backslide occurred. We reconstruct monotonized time from the upper 32 bit of the - // passed in value and the 64bits loaded from the atomic - let seconds_lower = old >> 32; - let mut seconds_upper = secs & 0xffff_ffff_0000_0000; - if secs & 0xffff_ffff > seconds_lower { - // Backslide caused the lower 32bit of the seconds part to wrap. - // This must be the case because the seconds part is larger even though - // we are in the backslide branch, i.e. the seconds count should be smaller or equal. - // - // We assume that backslides are smaller than 2^32 seconds - // which means we need to add 1 to the upper half to restore it. - // - // Example: - // most recent observed time: 0xA1_0000_0000_0000_0000u128 - // bits stored in AtomicU64: 0x0000_0000_0000_0000u64 - // backslide by 1s - // caller time is 0xA0_ffff_ffff_0000_0000u128 - // -> we can fix up the upper half time by adding 1 << 32 - seconds_upper = seconds_upper.wrapping_add(0x1_0000_0000); + let mut old = mono.load(Relaxed); + loop { + if old == UNINITIALIZED || packed.wrapping_sub(old) < u64::MAX / 2 { + match mono.compare_exchange_weak(old, packed, Relaxed, Relaxed) { + Ok(_) => return raw, + Err(x) => { + old = x; + continue; + } + } + } else { + // Backslide occurred. We reconstruct monotonized time from the upper 32 bit of the + // passed in value and the 64bits loaded from the atomic + let seconds_lower = old >> 32; + let mut seconds_upper = secs & 0xffff_ffff_0000_0000; + if secs & 0xffff_ffff > seconds_lower { + // Backslide caused the lower 32bit of the seconds part to wrap. + // This must be the case because the seconds part is larger even though + // we are in the backslide branch, i.e. the seconds count should be smaller or equal. + // + // We assume that backslides are smaller than 2^32 seconds + // which means we need to add 1 to the upper half to restore it. + // + // Example: + // most recent observed time: 0xA1_0000_0000_0000_0000u128 + // bits stored in AtomicU64: 0x0000_0000_0000_0000u64 + // backslide by 1s + // caller time is 0xA0_ffff_ffff_0000_0000u128 + // -> we can fix up the upper half time by adding 1 << 32 + seconds_upper = seconds_upper.wrapping_add(0x1_0000_0000); + } + let secs = seconds_upper | seconds_lower; + let nanos = old as u32; + return ZERO.checked_add_duration(&Duration::new(secs, nanos)).unwrap(); } - let secs = seconds_upper | seconds_lower; - let nanos = old as u32; - ZERO.checked_add_duration(&Duration::new(secs, nanos)).unwrap() } } } From 57465d9c1bb979a64c6e7c5220b4ab20291547aa Mon Sep 17 00:00:00 2001 From: The8472 Date: Fri, 17 Sep 2021 18:42:16 +0200 Subject: [PATCH 2/2] use AtomicU64::fetch_update instead of handrolled RMW-loop --- library/std/src/lib.rs | 1 + library/std/src/time/monotonic.rs | 23 +++++++++-------------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 83c6ba0e6ea45..f1640ab8b1e23 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -234,6 +234,7 @@ #![feature(atomic_mut_ptr)] #![feature(auto_traits)] #![feature(bench_black_box)] +#![feature(bool_to_option)] #![feature(box_syntax)] #![feature(c_unwind)] #![feature(c_variadic)] diff --git a/library/std/src/time/monotonic.rs b/library/std/src/time/monotonic.rs index fb1c893275667..198ae739b5567 100644 --- a/library/std/src/time/monotonic.rs +++ b/library/std/src/time/monotonic.rs @@ -37,20 +37,15 @@ pub mod inner { // This could be a problem for programs that call instants at intervals greater // than 68 years. Interstellar probes may want to ensure that actually_monotonic() is true. let packed = (secs << 32) | nanos; - let mut old = mono.load(Relaxed); - loop { - if old == UNINITIALIZED || packed.wrapping_sub(old) < u64::MAX / 2 { - match mono.compare_exchange_weak(old, packed, Relaxed, Relaxed) { - Ok(_) => return raw, - Err(x) => { - old = x; - continue; - } - } - } else { + let updated = mono.fetch_update(Relaxed, Relaxed, |old| { + (old == UNINITIALIZED || packed.wrapping_sub(old) < u64::MAX / 2).then_some(packed) + }); + match updated { + Ok(_) => raw, + Err(newer) => { // Backslide occurred. We reconstruct monotonized time from the upper 32 bit of the // passed in value and the 64bits loaded from the atomic - let seconds_lower = old >> 32; + let seconds_lower = newer >> 32; let mut seconds_upper = secs & 0xffff_ffff_0000_0000; if secs & 0xffff_ffff > seconds_lower { // Backslide caused the lower 32bit of the seconds part to wrap. @@ -69,8 +64,8 @@ pub mod inner { seconds_upper = seconds_upper.wrapping_add(0x1_0000_0000); } let secs = seconds_upper | seconds_lower; - let nanos = old as u32; - return ZERO.checked_add_duration(&Duration::new(secs, nanos)).unwrap(); + let nanos = newer as u32; + ZERO.checked_add_duration(&Duration::new(secs, nanos)).unwrap() } } }