From 7cae9e8c88e468e94c157d9aaee4b8e3cf90b9a4 Mon Sep 17 00:00:00 2001 From: maekawatoshiki Date: Fri, 31 Jul 2020 18:50:11 +0900 Subject: [PATCH] `#![deny(unsafe_op_in_unsafe_fn)]` in sys/hermit --- library/std/src/sys/hermit/alloc.rs | 25 ++++--- library/std/src/sys/hermit/args.rs | 16 +++-- library/std/src/sys/hermit/condvar.rs | 34 +++++++--- library/std/src/sys/hermit/fd.rs | 1 + library/std/src/sys/hermit/mod.rs | 22 ++++-- library/std/src/sys/hermit/mutex.rs | 26 ++++--- library/std/src/sys/hermit/os.rs | 2 + library/std/src/sys/hermit/rwlock.rs | 68 ++++++++++++------- library/std/src/sys/hermit/thread.rs | 21 +++--- .../std/src/sys/hermit/thread_local_dtor.rs | 7 +- 10 files changed, 147 insertions(+), 75 deletions(-) diff --git a/library/std/src/sys/hermit/alloc.rs b/library/std/src/sys/hermit/alloc.rs index d153914e77e..04446172197 100644 --- a/library/std/src/sys/hermit/alloc.rs +++ b/library/std/src/sys/hermit/alloc.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::alloc::{GlobalAlloc, Layout, System}; use crate::ptr; use crate::sys::hermit::abi; @@ -6,26 +8,33 @@ unsafe impl GlobalAlloc for System { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { - abi::malloc(layout.size(), layout.align()) + // SAFETY: The safety contract for `malloc` must be upheld by the caller. + unsafe { abi::malloc(layout.size(), layout.align()) } } unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { - let addr = abi::malloc(layout.size(), layout.align()); + // SAFETY: The safety contract for `malloc` must be upheld by the caller. + // Also, `addr` must be valid for writes of `layout.size() * size_of::()` bytes. + unsafe { + let addr = abi::malloc(layout.size(), layout.align()); - if !addr.is_null() { - ptr::write_bytes(addr, 0x00, layout.size()); - } + if !addr.is_null() { + ptr::write_bytes(addr, 0x00, layout.size()); + } - addr + addr + } } #[inline] unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { - abi::free(ptr, layout.size(), layout.align()) + // SAFETY: The safety contract for `free` must be upheld by the caller. + unsafe { abi::free(ptr, layout.size(), layout.align()) } } #[inline] unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { - abi::realloc(ptr, layout.size(), layout.align(), new_size) + // SAFETY: The safety contract for `realloc` must be upheld by the caller. + unsafe { abi::realloc(ptr, layout.size(), layout.align(), new_size) } } } diff --git a/library/std/src/sys/hermit/args.rs b/library/std/src/sys/hermit/args.rs index 72c1b8511ca..dd8830599c3 100644 --- a/library/std/src/sys/hermit/args.rs +++ b/library/std/src/sys/hermit/args.rs @@ -1,15 +1,17 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::ffi::OsString; use crate::marker::PhantomData; use crate::vec; /// One-time global initialization. pub unsafe fn init(argc: isize, argv: *const *const u8) { - imp::init(argc, argv) + unsafe { imp::init(argc, argv) } } /// One-time global cleanup. pub unsafe fn cleanup() { - imp::cleanup() + unsafe { imp::cleanup() } } /// Returns the command line arguments @@ -65,14 +67,18 @@ mod imp { pub unsafe fn init(argc: isize, argv: *const *const u8) { let _guard = LOCK.lock(); - ARGC = argc; - ARGV = argv; + unsafe { + ARGC = argc; + ARGV = argv; + } } pub unsafe fn cleanup() { let _guard = LOCK.lock(); ARGC = 0; - ARGV = ptr::null(); + unsafe { + ARGV = ptr::null(); + } } pub fn args() -> Args { diff --git a/library/std/src/sys/hermit/condvar.rs b/library/std/src/sys/hermit/condvar.rs index 52c8c3b17e8..662dc9394a3 100644 --- a/library/std/src/sys/hermit/condvar.rs +++ b/library/std/src/sys/hermit/condvar.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::ffi::c_void; use crate::ptr; use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst}; @@ -23,33 +25,43 @@ pub const fn new() -> Condvar { } pub unsafe fn init(&mut self) { - let _ = abi::sem_init(&mut self.sem1 as *mut *const c_void, 0); - let _ = abi::sem_init(&mut self.sem2 as *mut *const c_void, 0); + unsafe { + let _ = abi::sem_init(&mut self.sem1 as *mut *const c_void, 0); + let _ = abi::sem_init(&mut self.sem2 as *mut *const c_void, 0); + } } pub unsafe fn notify_one(&self) { if self.counter.load(SeqCst) > 0 { self.counter.fetch_sub(1, SeqCst); - abi::sem_post(self.sem1); - abi::sem_timedwait(self.sem2, 0); + unsafe { + abi::sem_post(self.sem1); + abi::sem_timedwait(self.sem2, 0); + } } } pub unsafe fn notify_all(&self) { let counter = self.counter.swap(0, SeqCst); for _ in 0..counter { - abi::sem_post(self.sem1); + unsafe { + abi::sem_post(self.sem1); + } } for _ in 0..counter { - abi::sem_timedwait(self.sem2, 0); + unsafe { + abi::sem_timedwait(self.sem2, 0); + } } } pub unsafe fn wait(&self, mutex: &Mutex) { self.counter.fetch_add(1, SeqCst); mutex.unlock(); - abi::sem_timedwait(self.sem1, 0); - abi::sem_post(self.sem2); + unsafe { + abi::sem_timedwait(self.sem1, 0); + abi::sem_post(self.sem2); + } mutex.lock(); } @@ -58,7 +70,9 @@ pub unsafe fn wait_timeout(&self, _mutex: &Mutex, _dur: Duration) -> bool { } pub unsafe fn destroy(&self) { - let _ = abi::sem_destroy(self.sem1); - let _ = abi::sem_destroy(self.sem2); + unsafe { + let _ = abi::sem_destroy(self.sem1); + let _ = abi::sem_destroy(self.sem2); + } } } diff --git a/library/std/src/sys/hermit/fd.rs b/library/std/src/sys/hermit/fd.rs index 97d1a38b41a..2914e5ad3db 100644 --- a/library/std/src/sys/hermit/fd.rs +++ b/library/std/src/sys/hermit/fd.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] #![unstable(reason = "not public", issue = "none", feature = "fd")] use crate::io::{self, ErrorKind, Read}; diff --git a/library/std/src/sys/hermit/mod.rs b/library/std/src/sys/hermit/mod.rs index 675b82ceb77..0e4504020df 100644 --- a/library/std/src/sys/hermit/mod.rs +++ b/library/std/src/sys/hermit/mod.rs @@ -13,6 +13,8 @@ //! compiling for wasm. That way it's a compile time error for something that's //! guaranteed to be a runtime error! +#![deny(unsafe_op_in_unsafe_fn)] + use crate::intrinsics; use crate::os::raw::c_char; @@ -62,8 +64,12 @@ pub enum Void {} pub unsafe fn strlen(start: *const c_char) -> usize { let mut str = start; - while *str != 0 { - str = str.offset(1); + // SAFETY: The safety contract for `*str != 0` must be upheld by the caller. + // `start` must not be null. + unsafe { + while *str != 0 { + str = str.offset(1); + } } (str as usize) - (start as usize) @@ -111,13 +117,15 @@ pub fn init() { fn main(argc: isize, argv: *const *const c_char) -> i32; } - // initialize environment - os::init_environment(env as *const *const i8); + unsafe { + // initialize environment + os::init_environment(env as *const *const i8); - let result = main(argc as isize, argv); + let result = main(argc as isize, argv); - run_dtors(); - abi::exit(result); + run_dtors(); + abi::exit(result); + } } pub fn decode_error_kind(errno: i32) -> ErrorKind { diff --git a/library/std/src/sys/hermit/mutex.rs b/library/std/src/sys/hermit/mutex.rs index 3d4813209cb..bcb2554ab29 100644 --- a/library/std/src/sys/hermit/mutex.rs +++ b/library/std/src/sys/hermit/mutex.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::ffi::c_void; use crate::ptr; use crate::sys::hermit::abi; @@ -16,28 +18,34 @@ pub const fn new() -> Mutex { #[inline] pub unsafe fn init(&mut self) { - let _ = abi::sem_init(&mut self.inner as *mut *const c_void, 1); + unsafe { + let _ = abi::sem_init(&mut self.inner as *mut *const c_void, 1); + } } #[inline] pub unsafe fn lock(&self) { - let _ = abi::sem_timedwait(self.inner, 0); + unsafe { + let _ = abi::sem_timedwait(self.inner, 0); + } } #[inline] pub unsafe fn unlock(&self) { - let _ = abi::sem_post(self.inner); + unsafe { + let _ = abi::sem_post(self.inner); + } } #[inline] pub unsafe fn try_lock(&self) -> bool { - let result = abi::sem_trywait(self.inner); + let result = unsafe { abi::sem_trywait(self.inner) }; result == 0 } #[inline] pub unsafe fn destroy(&self) { - let _ = abi::sem_destroy(self.inner); + let _ = unsafe { abi::sem_destroy(self.inner) }; } } @@ -52,12 +60,12 @@ impl ReentrantMutex { #[inline] pub unsafe fn init(&self) { - let _ = abi::recmutex_init(&self.inner as *const *const c_void as *mut _); + let _ = unsafe { abi::recmutex_init(&self.inner as *const *const c_void as *mut _) }; } #[inline] pub unsafe fn lock(&self) { - let _ = abi::recmutex_lock(self.inner); + let _ = unsafe { abi::recmutex_lock(self.inner) }; } #[inline] @@ -67,11 +75,11 @@ pub unsafe fn try_lock(&self) -> bool { #[inline] pub unsafe fn unlock(&self) { - let _ = abi::recmutex_unlock(self.inner); + let _ = unsafe { abi::recmutex_unlock(self.inner) }; } #[inline] pub unsafe fn destroy(&self) { - let _ = abi::recmutex_destroy(self.inner); + let _ = unsafe { abi::recmutex_destroy(self.inner) }; } } diff --git a/library/std/src/sys/hermit/os.rs b/library/std/src/sys/hermit/os.rs index 78eabf8f81e..d95940c9a76 100644 --- a/library/std/src/sys/hermit/os.rs +++ b/library/std/src/sys/hermit/os.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::collections::HashMap; use crate::error::Error as StdError; use crate::ffi::{CStr, OsStr, OsString}; diff --git a/library/std/src/sys/hermit/rwlock.rs b/library/std/src/sys/hermit/rwlock.rs index 06442e925f4..a6c7bcc7641 100644 --- a/library/std/src/sys/hermit/rwlock.rs +++ b/library/std/src/sys/hermit/rwlock.rs @@ -1,3 +1,5 @@ +#![deny(unsafe_op_in_unsafe_fn)] + use crate::cell::UnsafeCell; use crate::sys::condvar::Condvar; use crate::sys::mutex::Mutex; @@ -32,62 +34,76 @@ pub const fn new() -> RWLock { #[inline] pub unsafe fn read(&self) { - self.lock.lock(); - while !(*self.state.get()).inc_readers() { - self.cond.wait(&self.lock); + unsafe { + self.lock.lock(); + while !(*self.state.get()).inc_readers() { + self.cond.wait(&self.lock); + } + self.lock.unlock(); } - self.lock.unlock(); } #[inline] pub unsafe fn try_read(&self) -> bool { - self.lock.lock(); - let ok = (*self.state.get()).inc_readers(); - self.lock.unlock(); + unsafe { + self.lock.lock(); + let ok = (*self.state.get()).inc_readers(); + self.lock.unlock(); + } return ok; } #[inline] pub unsafe fn write(&self) { - self.lock.lock(); - while !(*self.state.get()).inc_writers() { - self.cond.wait(&self.lock); + unsafe { + self.lock.lock(); + while !(*self.state.get()).inc_writers() { + self.cond.wait(&self.lock); + } } self.lock.unlock(); } #[inline] pub unsafe fn try_write(&self) -> bool { - self.lock.lock(); - let ok = (*self.state.get()).inc_writers(); - self.lock.unlock(); + unsafe { + self.lock.lock(); + let ok = (*self.state.get()).inc_writers(); + self.lock.unlock(); + } return ok; } #[inline] pub unsafe fn read_unlock(&self) { - self.lock.lock(); - let notify = (*self.state.get()).dec_readers(); - self.lock.unlock(); - if notify { - // FIXME: should only wake up one of these some of the time - self.cond.notify_all(); + unsafe { + self.lock.lock(); + let notify = (*self.state.get()).dec_readers(); + self.lock.unlock(); + if notify { + // FIXME: should only wake up one of these some of the time + self.cond.notify_all(); + } } } #[inline] pub unsafe fn write_unlock(&self) { - self.lock.lock(); - (*self.state.get()).dec_writers(); - self.lock.unlock(); - // FIXME: should only wake up one of these some of the time - self.cond.notify_all(); + unsafe { + self.lock.lock(); + (*self.state.get()).dec_writers(); + self.lock.unlock(); + // FIXME: should only wake up one of these some of the time + self.cond.notify_all(); + } } #[inline] pub unsafe fn destroy(&self) { - self.lock.destroy(); - self.cond.destroy(); + unsafe { + self.lock.destroy(); + self.cond.destroy(); + } } } diff --git a/library/std/src/sys/hermit/thread.rs b/library/std/src/sys/hermit/thread.rs index e11afed6687..7c52112a80c 100644 --- a/library/std/src/sys/hermit/thread.rs +++ b/library/std/src/sys/hermit/thread.rs @@ -1,4 +1,5 @@ #![allow(dead_code)] +#![deny(unsafe_op_in_unsafe_fn)] use crate::ffi::CStr; use crate::io; @@ -25,18 +26,22 @@ pub unsafe fn new_with_coreid( core_id: isize, ) -> io::Result { let p = Box::into_raw(box p); - let tid = abi::spawn2( - thread_start, - p as usize, - abi::Priority::into(abi::NORMAL_PRIO), - stack, - core_id, - ); + let tid = unsafe { + abi::spawn2( + thread_start, + p as usize, + abi::Priority::into(abi::NORMAL_PRIO), + stack, + core_id, + ) + }; return if tid == 0 { // The thread failed to start and as a result p was not consumed. Therefore, it is // safe to reconstruct the box so that it gets deallocated. - drop(Box::from_raw(p)); + unsafe { + drop(Box::from_raw(p)); + } Err(io::Error::new(io::ErrorKind::Other, "Unable to create thread!")) } else { Ok(Thread { tid: tid }) diff --git a/library/std/src/sys/hermit/thread_local_dtor.rs b/library/std/src/sys/hermit/thread_local_dtor.rs index 9b683fce157..7998dd3cb43 100644 --- a/library/std/src/sys/hermit/thread_local_dtor.rs +++ b/library/std/src/sys/hermit/thread_local_dtor.rs @@ -1,5 +1,6 @@ #![cfg(target_thread_local)] #![unstable(feature = "thread_local_internals", issue = "none")] +#![deny(unsafe_op_in_unsafe_fn)] // Simplify dtor registration by using a list of destructors. // The this solution works like the implementation of macOS and @@ -19,7 +20,8 @@ pub unsafe fn register_dtor(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) { DTORS.set(Box::into_raw(v)); } - let list: &mut List = &mut *DTORS.get(); + // SAFETY: `DTORS.get()` is not null. + let list: &mut List = unsafe { &mut *DTORS.get() }; list.push((t, dtor)); } @@ -27,7 +29,8 @@ pub unsafe fn register_dtor(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) { pub unsafe fn run_dtors() { let mut ptr = DTORS.replace(ptr::null_mut()); while !ptr.is_null() { - let list = Box::from_raw(ptr); + // SAFETY: `ptr` is not null. + let list = unsafe { Box::from_raw(ptr) }; for (ptr, dtor) in list.into_iter() { dtor(ptr); } -- 2.44.0