Rollup merge of #127744 - workingjubilee:deny-unsafe-op-in-std, r=jhpratt

std: `#![deny(unsafe_op_in_unsafe_fn)]` in platform-independent code

This applies the `unsafe_op_in_unsafe_fn` lint in all places in std that _do not have platform-specific cfg in their code_. For all such places, the lint remains allowed, because they need further work to address the relevant concerns. This list includes:

- `std::backtrace_rs` (internal-only)
- `std::sys` (internal-only)
- `std::os`

Notably this eliminates all "unwrapped" unsafe operations in `std::io` and `std::sync`, which will make them much more auditable in the future. Such has *also* been left for future work. While I made a few safety comments along the way on interfaces I have grown sufficiently familiar with, in most cases I had no context, nor particular confidence the unsafety was correct.

In the cases where I was able to determine the unsafety was correct without having prior context, it was obviously redundant. For example, an unsafe function calling another unsafe function that has the exact same contract, forwarding its caller's requirements just as it forwards its actual call.
This commit is contained in:
Jubilee 2024-07-15 02:28:44 -07:00 committed by GitHub
commit 99c5302d9f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
18 changed files with 88 additions and 65 deletions

View File

@ -1018,7 +1018,7 @@ where
K: Borrow<Q>, K: Borrow<Q>,
Q: Hash + Eq, Q: Hash + Eq,
{ {
self.base.get_many_unchecked_mut(ks) unsafe { self.base.get_many_unchecked_mut(ks) }
} }
/// Returns `true` if the map contains a value for the specified key. /// Returns `true` if the map contains a value for the specified key.

View File

@ -366,11 +366,8 @@ impl Error for VarError {
#[rustc_deprecated_safe_2024] #[rustc_deprecated_safe_2024]
#[stable(feature = "env", since = "1.0.0")] #[stable(feature = "env", since = "1.0.0")]
pub unsafe fn set_var<K: AsRef<OsStr>, V: AsRef<OsStr>>(key: K, value: V) { pub unsafe fn set_var<K: AsRef<OsStr>, V: AsRef<OsStr>>(key: K, value: V) {
_set_var(key.as_ref(), value.as_ref()) let (key, value) = (key.as_ref(), value.as_ref());
} unsafe { os_imp::setenv(key, value) }.unwrap_or_else(|e| {
unsafe fn _set_var(key: &OsStr, value: &OsStr) {
os_imp::setenv(key, value).unwrap_or_else(|e| {
panic!("failed to set environment variable `{key:?}` to `{value:?}`: {e}") panic!("failed to set environment variable `{key:?}` to `{value:?}`: {e}")
}) })
} }
@ -433,11 +430,8 @@ unsafe fn _set_var(key: &OsStr, value: &OsStr) {
#[rustc_deprecated_safe_2024] #[rustc_deprecated_safe_2024]
#[stable(feature = "env", since = "1.0.0")] #[stable(feature = "env", since = "1.0.0")]
pub unsafe fn remove_var<K: AsRef<OsStr>>(key: K) { pub unsafe fn remove_var<K: AsRef<OsStr>>(key: K) {
_remove_var(key.as_ref()) let key = key.as_ref();
} unsafe { os_imp::unsetenv(key) }
unsafe fn _remove_var(key: &OsStr) {
os_imp::unsetenv(key)
.unwrap_or_else(|e| panic!("failed to remove environment variable `{key:?}`: {e}")) .unwrap_or_else(|e| panic!("failed to remove environment variable `{key:?}`: {e}"))
} }

View File

@ -184,7 +184,7 @@ impl OsString {
#[inline] #[inline]
#[stable(feature = "os_str_bytes", since = "1.74.0")] #[stable(feature = "os_str_bytes", since = "1.74.0")]
pub unsafe fn from_encoded_bytes_unchecked(bytes: Vec<u8>) -> Self { pub unsafe fn from_encoded_bytes_unchecked(bytes: Vec<u8>) -> Self {
OsString { inner: Buf::from_encoded_bytes_unchecked(bytes) } OsString { inner: unsafe { Buf::from_encoded_bytes_unchecked(bytes) } }
} }
/// Converts to an [`OsStr`] slice. /// Converts to an [`OsStr`] slice.
@ -813,7 +813,7 @@ impl OsStr {
#[inline] #[inline]
#[stable(feature = "os_str_bytes", since = "1.74.0")] #[stable(feature = "os_str_bytes", since = "1.74.0")]
pub unsafe fn from_encoded_bytes_unchecked(bytes: &[u8]) -> &Self { pub unsafe fn from_encoded_bytes_unchecked(bytes: &[u8]) -> &Self {
Self::from_inner(Slice::from_encoded_bytes_unchecked(bytes)) Self::from_inner(unsafe { Slice::from_encoded_bytes_unchecked(bytes) })
} }
#[inline] #[inline]

View File

@ -433,10 +433,12 @@ impl<W: ?Sized + Write> BufWriter<W> {
let old_len = self.buf.len(); let old_len = self.buf.len();
let buf_len = buf.len(); let buf_len = buf.len();
let src = buf.as_ptr(); let src = buf.as_ptr();
unsafe {
let dst = self.buf.as_mut_ptr().add(old_len); let dst = self.buf.as_mut_ptr().add(old_len);
ptr::copy_nonoverlapping(src, dst, buf_len); ptr::copy_nonoverlapping(src, dst, buf_len);
self.buf.set_len(old_len + buf_len); self.buf.set_len(old_len + buf_len);
} }
}
#[inline] #[inline]
fn spare_capacity(&self) -> usize { fn spare_capacity(&self) -> usize {

View File

@ -482,7 +482,7 @@ where
A: Allocator, A: Allocator,
{ {
debug_assert!(vec.capacity() >= pos + buf.len()); debug_assert!(vec.capacity() >= pos + buf.len());
vec.as_mut_ptr().add(pos).copy_from(buf.as_ptr(), buf.len()); unsafe { vec.as_mut_ptr().add(pos).copy_from(buf.as_ptr(), buf.len()) };
pos + buf.len() pos + buf.len()
} }

View File

@ -267,11 +267,14 @@ where
// Using this rather than unwrap meaningfully improves the code // Using this rather than unwrap meaningfully improves the code
// for callers which only care about one variant (usually // for callers which only care about one variant (usually
// `Custom`) // `Custom`)
core::hint::unreachable_unchecked(); unsafe { core::hint::unreachable_unchecked() };
}); });
ErrorData::Simple(kind) ErrorData::Simple(kind)
} }
TAG_SIMPLE_MESSAGE => ErrorData::SimpleMessage(&*ptr.cast::<SimpleMessage>().as_ptr()), TAG_SIMPLE_MESSAGE => {
// SAFETY: per tag
unsafe { ErrorData::SimpleMessage(&*ptr.cast::<SimpleMessage>().as_ptr()) }
}
TAG_CUSTOM => { TAG_CUSTOM => {
// It would be correct for us to use `ptr::byte_sub` here (see the // It would be correct for us to use `ptr::byte_sub` here (see the
// comment above the `wrapping_add` call in `new_custom` for why), // comment above the `wrapping_add` call in `new_custom` for why),

View File

@ -382,11 +382,11 @@ pub(crate) unsafe fn append_to_string<F>(buf: &mut String, f: F) -> Result<usize
where where
F: FnOnce(&mut Vec<u8>) -> Result<usize>, F: FnOnce(&mut Vec<u8>) -> Result<usize>,
{ {
let mut g = Guard { len: buf.len(), buf: buf.as_mut_vec() }; let mut g = Guard { len: buf.len(), buf: unsafe { buf.as_mut_vec() } };
let ret = f(g.buf); let ret = f(g.buf);
// SAFETY: the caller promises to only append data to `buf` // SAFETY: the caller promises to only append data to `buf`
let appended = g.buf.get_unchecked(g.len..); let appended = unsafe { g.buf.get_unchecked(g.len..) };
if str::from_utf8(appended).is_err() { if str::from_utf8(appended).is_err() {
ret.and_then(|_| Err(Error::INVALID_UTF8)) ret.and_then(|_| Err(Error::INVALID_UTF8))
} else { } else {

View File

@ -252,6 +252,7 @@
#![allow(internal_features)] #![allow(internal_features)]
#![deny(rustc::existing_doc_keyword)] #![deny(rustc::existing_doc_keyword)]
#![deny(fuzzy_provenance_casts)] #![deny(fuzzy_provenance_casts)]
#![deny(unsafe_op_in_unsafe_fn)]
#![allow(rustdoc::redundant_explicit_links)] #![allow(rustdoc::redundant_explicit_links)]
// Ensure that std can be linked against panic_abort despite compiled with `-C panic=unwind` // Ensure that std can be linked against panic_abort despite compiled with `-C panic=unwind`
#![deny(ffi_unwind_calls)] #![deny(ffi_unwind_calls)]
@ -664,7 +665,7 @@ pub mod alloc;
mod panicking; mod panicking;
#[path = "../../backtrace/src/lib.rs"] #[path = "../../backtrace/src/lib.rs"]
#[allow(dead_code, unused_attributes, fuzzy_provenance_casts)] #[allow(dead_code, unused_attributes, fuzzy_provenance_casts, unsafe_op_in_unsafe_fn)]
mod backtrace_rs; mod backtrace_rs;
// Re-export macros defined in core. // Re-export macros defined in core.

View File

@ -2,6 +2,7 @@
#![stable(feature = "os", since = "1.0.0")] #![stable(feature = "os", since = "1.0.0")]
#![allow(missing_docs, nonstandard_style, missing_debug_implementations)] #![allow(missing_docs, nonstandard_style, missing_debug_implementations)]
#![allow(unsafe_op_in_unsafe_fn)]
pub mod raw; pub mod raw;

View File

@ -200,11 +200,12 @@ impl<T> Channel<T> {
return Err(msg); return Err(msg);
} }
let slot: &Slot<T> = &*(token.array.slot as *const Slot<T>);
// Write the message into the slot and update the stamp. // Write the message into the slot and update the stamp.
unsafe {
let slot: &Slot<T> = &*(token.array.slot as *const Slot<T>);
slot.msg.get().write(MaybeUninit::new(msg)); slot.msg.get().write(MaybeUninit::new(msg));
slot.stamp.store(token.array.stamp, Ordering::Release); slot.stamp.store(token.array.stamp, Ordering::Release);
}
// Wake a sleeping receiver. // Wake a sleeping receiver.
self.receivers.notify(); self.receivers.notify();
@ -291,11 +292,14 @@ impl<T> Channel<T> {
return Err(()); return Err(());
} }
// Read the message from the slot and update the stamp.
let msg = unsafe {
let slot: &Slot<T> = &*(token.array.slot as *const Slot<T>); let slot: &Slot<T> = &*(token.array.slot as *const Slot<T>);
// Read the message from the slot and update the stamp.
let msg = slot.msg.get().read().assume_init(); let msg = slot.msg.get().read().assume_init();
slot.stamp.store(token.array.stamp, Ordering::Release); slot.stamp.store(token.array.stamp, Ordering::Release);
msg
};
// Wake a sleeping sender. // Wake a sleeping sender.
self.senders.notify(); self.senders.notify();
@ -471,7 +475,7 @@ impl<T> Channel<T> {
false false
}; };
self.discard_all_messages(tail); unsafe { self.discard_all_messages(tail) };
disconnected disconnected
} }

View File

@ -63,7 +63,7 @@ impl<C> Sender<C> {
disconnect(&self.counter().chan); disconnect(&self.counter().chan);
if self.counter().destroy.swap(true, Ordering::AcqRel) { if self.counter().destroy.swap(true, Ordering::AcqRel) {
drop(Box::from_raw(self.counter)); drop(unsafe { Box::from_raw(self.counter) });
} }
} }
} }
@ -116,7 +116,7 @@ impl<C> Receiver<C> {
disconnect(&self.counter().chan); disconnect(&self.counter().chan);
if self.counter().destroy.swap(true, Ordering::AcqRel) { if self.counter().destroy.swap(true, Ordering::AcqRel) {
drop(Box::from_raw(self.counter)); drop(unsafe { Box::from_raw(self.counter) });
} }
} }
} }

View File

@ -91,7 +91,7 @@ impl<T> Block<T> {
// It is not necessary to set the `DESTROY` bit in the last slot because that slot has // It is not necessary to set the `DESTROY` bit in the last slot because that slot has
// begun destruction of the block. // begun destruction of the block.
for i in start..BLOCK_CAP - 1 { for i in start..BLOCK_CAP - 1 {
let slot = (*this).slots.get_unchecked(i); let slot = unsafe { (*this).slots.get_unchecked(i) };
// Mark the `DESTROY` bit if a thread is still using the slot. // Mark the `DESTROY` bit if a thread is still using the slot.
if slot.state.load(Ordering::Acquire) & READ == 0 if slot.state.load(Ordering::Acquire) & READ == 0
@ -103,7 +103,7 @@ impl<T> Block<T> {
} }
// No thread is using the block, now it is safe to destroy it. // No thread is using the block, now it is safe to destroy it.
drop(Box::from_raw(this)); drop(unsafe { Box::from_raw(this) });
} }
} }
@ -265,9 +265,11 @@ impl<T> Channel<T> {
// Write the message into the slot. // Write the message into the slot.
let block = token.list.block as *mut Block<T>; let block = token.list.block as *mut Block<T>;
let offset = token.list.offset; let offset = token.list.offset;
unsafe {
let slot = (*block).slots.get_unchecked(offset); let slot = (*block).slots.get_unchecked(offset);
slot.msg.get().write(MaybeUninit::new(msg)); slot.msg.get().write(MaybeUninit::new(msg));
slot.state.fetch_or(WRITE, Ordering::Release); slot.state.fetch_or(WRITE, Ordering::Release);
}
// Wake a sleeping receiver. // Wake a sleeping receiver.
self.receivers.notify(); self.receivers.notify();
@ -369,6 +371,7 @@ impl<T> Channel<T> {
// Read the message. // Read the message.
let block = token.list.block as *mut Block<T>; let block = token.list.block as *mut Block<T>;
let offset = token.list.offset; let offset = token.list.offset;
unsafe {
let slot = (*block).slots.get_unchecked(offset); let slot = (*block).slots.get_unchecked(offset);
slot.wait_write(); slot.wait_write();
let msg = slot.msg.get().read().assume_init(); let msg = slot.msg.get().read().assume_init();
@ -383,6 +386,7 @@ impl<T> Channel<T> {
Ok(msg) Ok(msg)
} }
}
/// Attempts to send a message into the channel. /// Attempts to send a message into the channel.
pub(crate) fn try_send(&self, msg: T) -> Result<(), TrySendError<T>> { pub(crate) fn try_send(&self, msg: T) -> Result<(), TrySendError<T>> {

View File

@ -103,9 +103,11 @@ impl<T> Channel<T> {
return Err(msg); return Err(msg);
} }
unsafe {
let packet = &*(token.zero.0 as *const Packet<T>); let packet = &*(token.zero.0 as *const Packet<T>);
packet.msg.get().write(Some(msg)); packet.msg.get().write(Some(msg));
packet.ready.store(true, Ordering::Release); packet.ready.store(true, Ordering::Release);
}
Ok(()) Ok(())
} }
@ -116,24 +118,26 @@ impl<T> Channel<T> {
return Err(()); return Err(());
} }
let packet = &*(token.zero.0 as *const Packet<T>); let packet = unsafe { &*(token.zero.0 as *const Packet<T>) };
if packet.on_stack { if packet.on_stack {
// The message has been in the packet from the beginning, so there is no need to wait // The message has been in the packet from the beginning, so there is no need to wait
// for it. However, after reading the message, we need to set `ready` to `true` in // for it. However, after reading the message, we need to set `ready` to `true` in
// order to signal that the packet can be destroyed. // order to signal that the packet can be destroyed.
let msg = packet.msg.get().replace(None).unwrap(); let msg = unsafe { packet.msg.get().replace(None) }.unwrap();
packet.ready.store(true, Ordering::Release); packet.ready.store(true, Ordering::Release);
Ok(msg) Ok(msg)
} else { } else {
// Wait until the message becomes available, then read it and destroy the // Wait until the message becomes available, then read it and destroy the
// heap-allocated packet. // heap-allocated packet.
packet.wait_ready(); packet.wait_ready();
unsafe {
let msg = packet.msg.get().replace(None).unwrap(); let msg = packet.msg.get().replace(None).unwrap();
drop(Box::from_raw(token.zero.0 as *mut Packet<T>)); drop(Box::from_raw(token.zero.0 as *mut Packet<T>));
Ok(msg) Ok(msg)
} }
} }
}
/// Attempts to send a message into the channel. /// Attempts to send a message into the channel.
pub(crate) fn try_send(&self, msg: T) -> Result<(), TrySendError<T>> { pub(crate) fn try_send(&self, msg: T) -> Result<(), TrySendError<T>> {

View File

@ -502,7 +502,7 @@ impl<T> OnceLock<T> {
#[inline] #[inline]
unsafe fn get_unchecked(&self) -> &T { unsafe fn get_unchecked(&self) -> &T {
debug_assert!(self.is_initialized()); debug_assert!(self.is_initialized());
(&*self.value.get()).assume_init_ref() unsafe { (&*self.value.get()).assume_init_ref() }
} }
/// # Safety /// # Safety
@ -511,7 +511,7 @@ impl<T> OnceLock<T> {
#[inline] #[inline]
unsafe fn get_unchecked_mut(&mut self) -> &mut T { unsafe fn get_unchecked_mut(&mut self) -> &mut T {
debug_assert!(self.is_initialized()); debug_assert!(self.is_initialized());
(&mut *self.value.get()).assume_init_mut() unsafe { (&mut *self.value.get()).assume_init_mut() }
} }
} }

View File

@ -244,7 +244,9 @@ impl<T: ?Sized> ReentrantLock<T> {
} }
unsafe fn increment_lock_count(&self) -> Option<()> { unsafe fn increment_lock_count(&self) -> Option<()> {
unsafe {
*self.lock_count.get() = (*self.lock_count.get()).checked_add(1)?; *self.lock_count.get() = (*self.lock_count.get()).checked_add(1)?;
}
Some(()) Some(())
} }
} }

View File

@ -578,7 +578,7 @@ impl<'rwlock, T: ?Sized> RwLockReadGuard<'rwlock, T> {
// successfully called from the same thread before instantiating this object. // successfully called from the same thread before instantiating this object.
unsafe fn new(lock: &'rwlock RwLock<T>) -> LockResult<RwLockReadGuard<'rwlock, T>> { unsafe fn new(lock: &'rwlock RwLock<T>) -> LockResult<RwLockReadGuard<'rwlock, T>> {
poison::map_result(lock.poison.borrow(), |()| RwLockReadGuard { poison::map_result(lock.poison.borrow(), |()| RwLockReadGuard {
data: NonNull::new_unchecked(lock.data.get()), data: unsafe { NonNull::new_unchecked(lock.data.get()) },
inner_lock: &lock.inner, inner_lock: &lock.inner,
}) })
} }

View File

@ -1,3 +1,5 @@
#![allow(unsafe_op_in_unsafe_fn)]
/// The PAL (platform abstraction layer) contains platform-specific abstractions /// The PAL (platform abstraction layer) contains platform-specific abstractions
/// for implementing the features in the other submodules, e.g. UNIX file /// for implementing the features in the other submodules, e.g. UNIX file
/// descriptors. /// descriptors.

View File

@ -602,7 +602,8 @@ impl Wtf8 {
/// marked unsafe. /// marked unsafe.
#[inline] #[inline]
pub unsafe fn from_bytes_unchecked(value: &[u8]) -> &Wtf8 { pub unsafe fn from_bytes_unchecked(value: &[u8]) -> &Wtf8 {
mem::transmute(value) // SAFETY: start with &[u8], end with fancy &[u8]
unsafe { &*(value as *const [u8] as *const Wtf8) }
} }
/// Creates a mutable WTF-8 slice from a mutable WTF-8 byte slice. /// Creates a mutable WTF-8 slice from a mutable WTF-8 byte slice.
@ -611,7 +612,8 @@ impl Wtf8 {
/// marked unsafe. /// marked unsafe.
#[inline] #[inline]
unsafe fn from_mut_bytes_unchecked(value: &mut [u8]) -> &mut Wtf8 { unsafe fn from_mut_bytes_unchecked(value: &mut [u8]) -> &mut Wtf8 {
mem::transmute(value) // SAFETY: start with &mut [u8], end with fancy &mut [u8]
unsafe { &mut *(value as *mut [u8] as *mut Wtf8) }
} }
/// Returns the length, in WTF-8 bytes. /// Returns the length, in WTF-8 bytes.
@ -942,8 +944,12 @@ pub fn check_utf8_boundary(slice: &Wtf8, index: usize) {
/// Copied from core::str::raw::slice_unchecked /// Copied from core::str::raw::slice_unchecked
#[inline] #[inline]
pub unsafe fn slice_unchecked(s: &Wtf8, begin: usize, end: usize) -> &Wtf8 { pub unsafe fn slice_unchecked(s: &Wtf8, begin: usize, end: usize) -> &Wtf8 {
// memory layout of a &[u8] and &Wtf8 are the same // SAFETY: memory layout of a &[u8] and &Wtf8 are the same
Wtf8::from_bytes_unchecked(slice::from_raw_parts(s.bytes.as_ptr().add(begin), end - begin)) unsafe {
let len = end - begin;
let start = s.as_bytes().as_ptr().add(begin);
Wtf8::from_bytes_unchecked(slice::from_raw_parts(start, len))
}
} }
/// Copied from core::str::raw::slice_error_fail /// Copied from core::str::raw::slice_error_fail