From c25f69a1e3993bba59853767b366068685f64766 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 8 Oct 2020 22:52:26 +0200 Subject: [PATCH 1/6] Remove unsafety from unsupported/mutex.rs by using a Cell. Replacing the UnsafeCell by a Cell simplifies things and makes it all safe. --- library/std/src/sys/unsupported/mutex.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/library/std/src/sys/unsupported/mutex.rs b/library/std/src/sys/unsupported/mutex.rs index 06ea9a1e2c109..0e01edcaf397d 100644 --- a/library/std/src/sys/unsupported/mutex.rs +++ b/library/std/src/sys/unsupported/mutex.rs @@ -1,7 +1,9 @@ -use crate::cell::UnsafeCell; +#![deny(unsafe_op_in_unsafe_fn)] + +use crate::cell::Cell; pub struct Mutex { - locked: UnsafeCell, + locked: Cell, } pub type MovableMutex = Mutex; @@ -12,7 +14,7 @@ unsafe impl Sync for Mutex {} // no threads on this platform impl Mutex { #[rustc_const_stable(feature = "const_sys_mutex_new", since = "1.0.0")] pub const fn new() -> Mutex { - Mutex { locked: UnsafeCell::new(false) } + Mutex { locked: Cell::new(false) } } #[inline] @@ -20,25 +22,17 @@ impl Mutex { #[inline] pub unsafe fn lock(&self) { - let locked = self.locked.get(); - assert!(!*locked, "cannot recursively acquire mutex"); - *locked = true; + assert_eq!(self.locked.replace(true), false, "cannot recursively acquire mutex"); } #[inline] pub unsafe fn unlock(&self) { - *self.locked.get() = false; + self.locked.set(false); } #[inline] pub unsafe fn try_lock(&self) -> bool { - let locked = self.locked.get(); - if *locked { - false - } else { - *locked = true; - true - } + self.locked.replace(true) == false } #[inline] From 3d192ace34bcb13d3c033735cd7415260040c252 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 8 Oct 2020 22:52:26 +0200 Subject: [PATCH 2/6] Remove unsafety from unsupported/rwlosck.rs by using a Cell. Replacing the UnsafeCell by a Cell makes it all safe. --- library/std/src/sys/unsupported/rwlock.rs | 34 +++++++++++------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/library/std/src/sys/unsupported/rwlock.rs b/library/std/src/sys/unsupported/rwlock.rs index d37f34ac9352d..4e4f0f00f69d4 100644 --- a/library/std/src/sys/unsupported/rwlock.rs +++ b/library/std/src/sys/unsupported/rwlock.rs @@ -1,7 +1,9 @@ -use crate::cell::UnsafeCell; +#![deny(unsafe_op_in_unsafe_fn)] + +use crate::cell::Cell; pub struct RWLock { - mode: UnsafeCell, + mode: Cell, } unsafe impl Send for RWLock {} @@ -9,14 +11,14 @@ unsafe impl Sync for RWLock {} // no threads on this platform impl RWLock { pub const fn new() -> RWLock { - RWLock { mode: UnsafeCell::new(0) } + RWLock { mode: Cell::new(0) } } #[inline] pub unsafe fn read(&self) { - let mode = self.mode.get(); - if *mode >= 0 { - *mode += 1; + let m = self.mode.get(); + if m >= 0 { + self.mode.set(m + 1); } else { rtabort!("rwlock locked for writing"); } @@ -24,9 +26,9 @@ impl RWLock { #[inline] pub unsafe fn try_read(&self) -> bool { - let mode = self.mode.get(); - if *mode >= 0 { - *mode += 1; + let m = self.mode.get(); + if m >= 0 { + self.mode.set(m + 1); true } else { false @@ -35,19 +37,15 @@ impl RWLock { #[inline] pub unsafe fn write(&self) { - let mode = self.mode.get(); - if *mode == 0 { - *mode = -1; - } else { + if self.mode.replace(-1) != 0 { rtabort!("rwlock locked for reading") } } #[inline] pub unsafe fn try_write(&self) -> bool { - let mode = self.mode.get(); - if *mode == 0 { - *mode = -1; + if self.mode.get() == 0 { + self.mode.set(-1); true } else { false @@ -56,12 +54,12 @@ impl RWLock { #[inline] pub unsafe fn read_unlock(&self) { - *self.mode.get() -= 1; + self.mode.set(self.mode.get() - 1); } #[inline] pub unsafe fn write_unlock(&self) { - *self.mode.get() += 1; + self.mode.set(0); } #[inline] From f4e884288d0a1d6210e15bc7f8b014ef4cf86c32 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 8 Oct 2020 23:37:23 +0200 Subject: [PATCH 3/6] Apply deny(unsafe_op_in_unsafe_fn) to all of sys/unsupported. --- library/std/src/sys/unsupported/common.rs | 13 ++++++++----- library/std/src/sys/unsupported/mod.rs | 2 ++ library/std/src/sys/unsupported/mutex.rs | 2 -- library/std/src/sys/unsupported/rwlock.rs | 2 -- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/library/std/src/sys/unsupported/common.rs b/library/std/src/sys/unsupported/common.rs index 80311d26819ad..2cdd9c4d19e6e 100644 --- a/library/std/src/sys/unsupported/common.rs +++ b/library/std/src/sys/unsupported/common.rs @@ -39,10 +39,13 @@ pub fn hashmap_random_keys() -> (u64, u64) { pub enum Void {} pub unsafe fn strlen(mut s: *const c_char) -> usize { - let mut n = 0; - while *s != 0 { - n += 1; - s = s.offset(1); + // SAFETY: The caller must guarantee `s` points to a valid 0-terminated string. + unsafe { + let mut n = 0; + while *s != 0 { + n += 1; + s = s.offset(1); + } + n } - return n; } diff --git a/library/std/src/sys/unsupported/mod.rs b/library/std/src/sys/unsupported/mod.rs index 8ba870c5dbc14..d9efdec33d937 100644 --- a/library/std/src/sys/unsupported/mod.rs +++ b/library/std/src/sys/unsupported/mod.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + pub mod alloc; pub mod args; pub mod cmath; diff --git a/library/std/src/sys/unsupported/mutex.rs b/library/std/src/sys/unsupported/mutex.rs index 0e01edcaf397d..3b97d0875bdc1 100644 --- a/library/std/src/sys/unsupported/mutex.rs +++ b/library/std/src/sys/unsupported/mutex.rs @@ -1,5 +1,3 @@ -#![deny(unsafe_op_in_unsafe_fn)] - use crate::cell::Cell; pub struct Mutex { diff --git a/library/std/src/sys/unsupported/rwlock.rs b/library/std/src/sys/unsupported/rwlock.rs index 4e4f0f00f69d4..1a9c266196fe7 100644 --- a/library/std/src/sys/unsupported/rwlock.rs +++ b/library/std/src/sys/unsupported/rwlock.rs @@ -1,5 +1,3 @@ -#![deny(unsafe_op_in_unsafe_fn)] - use crate::cell::Cell; pub struct RWLock { From f1c3edbfaba6dd1723fcd63857f7de5ef2278f57 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Fri, 9 Oct 2020 00:39:03 +0200 Subject: [PATCH 4/6] Assert state in sys/unsupported's RwLock::write_unlock. Co-authored-by: Joshua Nelson --- library/std/src/sys/unsupported/rwlock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/unsupported/rwlock.rs b/library/std/src/sys/unsupported/rwlock.rs index 1a9c266196fe7..9aaba8bff1167 100644 --- a/library/std/src/sys/unsupported/rwlock.rs +++ b/library/std/src/sys/unsupported/rwlock.rs @@ -57,7 +57,7 @@ impl RWLock { #[inline] pub unsafe fn write_unlock(&self) { - self.mode.set(0); + assert_eq!(self.mode.replace(0), -1); } #[inline] From b26aa5d97353bed9ace56d8f247c88b2f9d4f8fd Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 13 Oct 2020 15:29:38 +0200 Subject: [PATCH 5/6] Add note about using cells in the locks on the 'unsupported' platform. --- library/std/src/sys/unsupported/mutex.rs | 1 + library/std/src/sys/unsupported/rwlock.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/library/std/src/sys/unsupported/mutex.rs b/library/std/src/sys/unsupported/mutex.rs index 3b97d0875bdc1..7830faae601a0 100644 --- a/library/std/src/sys/unsupported/mutex.rs +++ b/library/std/src/sys/unsupported/mutex.rs @@ -1,6 +1,7 @@ use crate::cell::Cell; pub struct Mutex { + // This platform has no threads, so we can use a Cell here. locked: Cell, } diff --git a/library/std/src/sys/unsupported/rwlock.rs b/library/std/src/sys/unsupported/rwlock.rs index 9aaba8bff1167..6982b2b155fa5 100644 --- a/library/std/src/sys/unsupported/rwlock.rs +++ b/library/std/src/sys/unsupported/rwlock.rs @@ -1,6 +1,7 @@ use crate::cell::Cell; pub struct RWLock { + // This platform has no threads, so we can use a Cell here. mode: Cell, } From af414dc274d30bc3f4aea1d396ac2663e0c08c56 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Tue, 13 Oct 2020 18:52:57 +0200 Subject: [PATCH 6/6] Deny unsafe_op_in_unsafe_fn for unsupported/common.rs through sys/wasm too. --- library/std/src/sys/wasi/mod.rs | 1 + library/std/src/sys/wasm/mod.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/library/std/src/sys/wasi/mod.rs b/library/std/src/sys/wasi/mod.rs index a7a4407ac38e2..a0a37ef8316a8 100644 --- a/library/std/src/sys/wasi/mod.rs +++ b/library/std/src/sys/wasi/mod.rs @@ -53,6 +53,7 @@ pub mod thread_local_key; pub mod time; #[path = "../unsupported/common.rs"] +#[deny(unsafe_op_in_unsafe_fn)] #[allow(unused)] mod common; pub use common::*; diff --git a/library/std/src/sys/wasm/mod.rs b/library/std/src/sys/wasm/mod.rs index 2934ea59ab5ff..18295e1129a05 100644 --- a/library/std/src/sys/wasm/mod.rs +++ b/library/std/src/sys/wasm/mod.rs @@ -66,5 +66,6 @@ cfg_if::cfg_if! { } #[path = "../unsupported/common.rs"] +#[deny(unsafe_op_in_unsafe_fn)] mod common; pub use common::*;