Skip to content

Commit

Permalink
Deny unsafe ops in unsafe fns in std::sys_common
Browse files Browse the repository at this point in the history
  • Loading branch information
LeSeulArtichaut committed Sep 13, 2020
1 parent b4bdc07 commit a04a2fa
Show file tree
Hide file tree
Showing 12 changed files with 151 additions and 82 deletions.
21 changes: 16 additions & 5 deletions library/std/src/sys_common/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,25 @@ pub unsafe fn realloc_fallback(
old_layout: Layout,
new_size: usize,
) -> *mut u8 {
// Docs for GlobalAlloc::realloc require this to be valid:
let new_layout = Layout::from_size_align_unchecked(new_size, old_layout.align());
// SAFETY: as stated in docs for GlobalAlloc::realloc, the caller
// must guarantee that `new_size` is valid for a `Layout`.
// The `old_layout.align()` is guaranteed to be valid as it comes
// from a `Layout`.
let new_layout = unsafe { Layout::from_size_align_unchecked(new_size, old_layout.align()) };

let new_ptr = GlobalAlloc::alloc(alloc, new_layout);
// SAFETY: as stated in docs for GlobalAlloc::realloc, the caller
// must guarantee that `new_size` is greater than zero.
let new_ptr = unsafe { GlobalAlloc::alloc(alloc, new_layout) };
if !new_ptr.is_null() {
let size = cmp::min(old_layout.size(), new_size);
ptr::copy_nonoverlapping(ptr, new_ptr, size);
GlobalAlloc::dealloc(alloc, ptr, old_layout);
// SAFETY: the newly allocated memory cannot overlap the previously
// allocated memory. Also, the call to `dealloc` is safe since
// the caller must guarantee that `ptr` is allocated via this allocator
// and layout is the same layout that was used to allocate `ptr`.
unsafe {
ptr::copy_nonoverlapping(ptr, new_ptr, size);
GlobalAlloc::dealloc(alloc, ptr, old_layout);
}
}
new_ptr
}
10 changes: 7 additions & 3 deletions library/std/src/sys_common/at_exit_imp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,14 @@ const DONE: *mut Queue = 1_usize as *mut _;
const ITERS: usize = 10;

unsafe fn init() -> bool {
if QUEUE.is_null() {
// SAFETY: the caller must ensure that `QUEUE` is not in use anywhere else,
// which `push` below does by locking `LOCK`.
if unsafe { QUEUE.is_null() } {
let state: Box<Queue> = box Vec::new();
QUEUE = Box::into_raw(state);
} else if QUEUE == DONE {
unsafe {
QUEUE = Box::into_raw(state);
}
} else if unsafe { QUEUE == DONE } {
// can't re-init after a cleanup
return false;
}
Expand Down
64 changes: 33 additions & 31 deletions library/std/src/sys_common/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,44 +76,46 @@ unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt::
let mut res = Ok(());
// Start immediately if we're not using a short backtrace.
let mut start = print_fmt != PrintFmt::Short;
backtrace_rs::trace_unsynchronized(|frame| {
if print_fmt == PrintFmt::Short && idx > MAX_NB_FRAMES {
return false;
}
unsafe {
backtrace_rs::trace_unsynchronized(|frame| {
if print_fmt == PrintFmt::Short && idx > MAX_NB_FRAMES {
return false;
}

let mut hit = false;
let mut stop = false;
backtrace_rs::resolve_frame_unsynchronized(frame, |symbol| {
hit = true;
if print_fmt == PrintFmt::Short {
if let Some(sym) = symbol.name().and_then(|s| s.as_str()) {
if sym.contains("__rust_begin_short_backtrace") {
stop = true;
return;
}
if sym.contains("__rust_end_short_backtrace") {
start = true;
return;
let mut hit = false;
let mut stop = false;
backtrace_rs::resolve_frame_unsynchronized(frame, |symbol| {
hit = true;
if print_fmt == PrintFmt::Short {
if let Some(sym) = symbol.name().and_then(|s| s.as_str()) {
if sym.contains("__rust_begin_short_backtrace") {
stop = true;
return;
}
if sym.contains("__rust_end_short_backtrace") {
start = true;
return;
}
}
}
}

if start {
res = bt_fmt.frame().symbol(frame, symbol);
if start {
res = bt_fmt.frame().symbol(frame, symbol);
}
});
if stop {
return false;
}
});
if stop {
return false;
}
if !hit {
if start {
res = bt_fmt.frame().print_raw(frame.ip(), None, None, None);
if !hit {
if start {
res = bt_fmt.frame().print_raw(frame.ip(), None, None, None);
}
}
}

idx += 1;
res.is_ok()
});
idx += 1;
res.is_ok()
});
}
res?;
bt_fmt.finish()?;
if print_fmt == PrintFmt::Short {
Expand Down
18 changes: 12 additions & 6 deletions library/std/src/sys_common/condvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,22 @@ impl Condvar {
/// address.
#[inline]
pub unsafe fn init(&mut self) {
self.0.init()
// SAFETY: the caller must uphold the safety contract for `init`.
unsafe { self.0.init() }
}

/// Signals one waiter on this condition variable to wake up.
#[inline]
pub unsafe fn notify_one(&self) {
self.0.notify_one()
// SAFETY: the caller must uphold the safety contract for `notify_one`.
unsafe { self.0.notify_one() }
}

/// Awakens all current waiters on this condition variable.
#[inline]
pub unsafe fn notify_all(&self) {
self.0.notify_all()
// SAFETY: the caller must uphold the safety contract for `notify_all`.
unsafe { self.0.notify_all() }
}

/// Waits for a signal on the specified mutex.
Expand All @@ -47,7 +50,8 @@ impl Condvar {
/// on this condition variable.
#[inline]
pub unsafe fn wait(&self, mutex: &Mutex) {
self.0.wait(mutex::raw(mutex))
// SAFETY: the caller must uphold the safety contract for `wait`.
unsafe { self.0.wait(mutex::raw(mutex)) }
}

/// Waits for a signal on the specified mutex with a timeout duration
Expand All @@ -58,7 +62,8 @@ impl Condvar {
/// on this condition variable.
#[inline]
pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
self.0.wait_timeout(mutex::raw(mutex), dur)
// SAFETY: the caller must uphold the safety contract for `wait_timeout`.
unsafe { self.0.wait_timeout(mutex::raw(mutex), dur) }
}

/// Deallocates all resources associated with this condition variable.
Expand All @@ -67,6 +72,7 @@ impl Condvar {
/// this condition variable.
#[inline]
pub unsafe fn destroy(&self) {
self.0.destroy()
// SAFETY: the caller must uphold the safety contract for `destroy`.
unsafe { self.0.destroy() }
}
}
1 change: 1 addition & 0 deletions library/std/src/sys_common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#![allow(missing_docs)]
#![allow(missing_debug_implementations)]
#![deny(unsafe_op_in_unsafe_fn)]

#[cfg(test)]
mod tests;
Expand Down
20 changes: 14 additions & 6 deletions library/std/src/sys_common/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ impl Mutex {
/// `init()`) is undefined behavior.
#[inline]
pub unsafe fn init(&mut self) {
self.0.init()
// SAFETY: the caller must uphold the safety contract for `init`.
unsafe { self.0.init() }
}

/// Locks the mutex blocking the current thread until it is available.
Expand All @@ -39,14 +40,18 @@ impl Mutex {
/// previous function call.
#[inline]
pub unsafe fn raw_lock(&self) {
self.0.lock()
// SAFETY: the caller must uphold the safety contract for `lock`.
unsafe { self.0.lock() }
}

/// Calls raw_lock() and then returns an RAII guard to guarantee the mutex
/// will be unlocked.
#[inline]
pub unsafe fn lock(&self) -> MutexGuard<'_> {
self.raw_lock();
// SAFETY: the caller must uphold the safety contract for `raw_lock`.
unsafe {
self.raw_lock();
}
MutexGuard(&self.0)
}

Expand All @@ -57,7 +62,8 @@ impl Mutex {
/// previous function call.
#[inline]
pub unsafe fn try_lock(&self) -> bool {
self.0.try_lock()
// SAFETY: the caller must uphold the safety contract for `try_lock`.
unsafe { self.0.try_lock() }
}

/// Unlocks the mutex.
Expand All @@ -69,7 +75,8 @@ impl Mutex {
/// lock() whenever possible.
#[inline]
pub unsafe fn raw_unlock(&self) {
self.0.unlock()
// SAFETY: the caller must uphold the safety contract for `unlock`.
unsafe { self.0.unlock() }
}

/// Deallocates all resources associated with this mutex.
Expand All @@ -78,7 +85,8 @@ impl Mutex {
/// this mutex.
#[inline]
pub unsafe fn destroy(&self) {
self.0.destroy()
// SAFETY: the caller must uphold the safety contract for `destroy`.
unsafe { self.0.destroy() }
}
}

Expand Down
11 changes: 9 additions & 2 deletions library/std/src/sys_common/remutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,12 @@ impl<T> ReentrantMutex<T> {
/// once this mutex is in its final resting place, and only then are the
/// lock/unlock methods safe.
pub const unsafe fn new(t: T) -> ReentrantMutex<T> {
ReentrantMutex { inner: sys::ReentrantMutex::uninitialized(), data: t }
// `ReentrantMutex::uninitialized()` is not unsafe on all platforms
#[allow(unused_unsafe)]
// SAFETY: the caller must guarantee that `init` is called.
unsafe {
ReentrantMutex { inner: sys::ReentrantMutex::uninitialized(), data: t }
}
}

/// Initializes this mutex so it's ready for use.
Expand All @@ -63,7 +68,8 @@ impl<T> ReentrantMutex<T> {
/// Unsafe to call more than once, and must be called after this will no
/// longer move in memory.
pub unsafe fn init(&self) {
self.inner.init();
// SAFETY: the caller must uphold the safety contract for `init`.
unsafe { self.inner.init() };
}

/// Acquires a mutex, blocking the current thread until it is able to do so.
Expand All @@ -79,6 +85,7 @@ impl<T> ReentrantMutex<T> {
/// this call will return failure if the mutex would otherwise be
/// acquired.
pub fn lock(&self) -> ReentrantMutexGuard<'_, T> {
// SAFETY: the caller must uphold the safety contract for `lock`.
unsafe { self.inner.lock() }
ReentrantMutexGuard::new(&self)
}
Expand Down
21 changes: 14 additions & 7 deletions library/std/src/sys_common/rwlock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ impl RWLock {
/// previous method call.
#[inline]
pub unsafe fn read(&self) {
self.0.read()
// SAFETY: the caller must uphold the safety contract for `read`.
unsafe { self.0.read() }
}

/// Attempts to acquire shared access to this lock, returning whether it
Expand All @@ -35,7 +36,8 @@ impl RWLock {
/// previous method call.
#[inline]
pub unsafe fn try_read(&self) -> bool {
self.0.try_read()
// SAFETY: the caller must uphold the safety contract for `try_read`.
unsafe { self.0.try_read() }
}

/// Acquires write access to the underlying lock, blocking the current thread
Expand All @@ -45,7 +47,8 @@ impl RWLock {
/// previous method call.
#[inline]
pub unsafe fn write(&self) {
self.0.write()
// SAFETY: the caller must uphold the safety contract for `write`.
unsafe { self.0.write() }
}

/// Attempts to acquire exclusive access to this lock, returning whether it
Expand All @@ -57,15 +60,17 @@ impl RWLock {
/// previous method call.
#[inline]
pub unsafe fn try_write(&self) -> bool {
self.0.try_write()
// SAFETY: the caller must uphold the safety contract for `try_write`.
unsafe { self.0.try_write() }
}

/// Unlocks previously acquired shared access to this lock.
///
/// Behavior is undefined if the current thread does not have shared access.
#[inline]
pub unsafe fn read_unlock(&self) {
self.0.read_unlock()
// SAFETY: the caller must uphold the safety contract for `read_unlock`.
unsafe { self.0.read_unlock() }
}

/// Unlocks previously acquired exclusive access to this lock.
Expand All @@ -74,7 +79,8 @@ impl RWLock {
/// exclusive access.
#[inline]
pub unsafe fn write_unlock(&self) {
self.0.write_unlock()
// SAFETY: the caller must uphold the safety contract for `write_unlock`.
unsafe { self.0.write_unlock() }
}

/// Destroys OS-related resources with this RWLock.
Expand All @@ -83,6 +89,7 @@ impl RWLock {
/// lock.
#[inline]
pub unsafe fn destroy(&self) {
self.0.destroy()
// SAFETY: the caller must uphold the safety contract for `destroy`.
unsafe { self.0.destroy() }
}
}
6 changes: 5 additions & 1 deletion library/std/src/sys_common/thread_info.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
#![allow(dead_code)] // stack_guard isn't used right now on all platforms
// stack_guard isn't used right now on all platforms
#![allow(dead_code)]
// FIXME: this should be removed once `thread_local!` denies unsafe ops in unsafe
// fns. This cannot be done yet because the macro is also used in libproc_macro.
#![allow(unsafe_op_in_unsafe_fn)]

use crate::cell::RefCell;
use crate::sys::thread::guard::Guard;
Expand Down
18 changes: 11 additions & 7 deletions library/std/src/sys_common/thread_local_dtor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,25 @@ pub unsafe fn register_dtor_fallback(t: *mut u8, dtor: unsafe extern "C" fn(*mut

static DTORS: StaticKey = StaticKey::new(Some(run_dtors));
type List = Vec<(*mut u8, unsafe extern "C" fn(*mut u8))>;
if DTORS.get().is_null() {
if unsafe { DTORS.get().is_null() } {
let v: Box<List> = box Vec::new();
DTORS.set(Box::into_raw(v) as *mut u8);
unsafe { DTORS.set(Box::into_raw(v) as *mut u8) }
}
let list: &mut List = &mut *(DTORS.get() as *mut List);
let list: &mut List = unsafe { &mut *(DTORS.get() as *mut List) };
list.push((t, dtor));

unsafe extern "C" fn run_dtors(mut ptr: *mut u8) {
while !ptr.is_null() {
let list: Box<List> = Box::from_raw(ptr as *mut List);
let list: Box<List> = unsafe { Box::from_raw(ptr as *mut List) };
for (ptr, dtor) in list.into_iter() {
dtor(ptr);
unsafe {
dtor(ptr);
}
}
unsafe {
ptr = DTORS.get();
DTORS.set(ptr::null_mut());
}
ptr = DTORS.get();
DTORS.set(ptr::null_mut());
}
}
}
Loading

0 comments on commit a04a2fa

Please sign in to comment.