Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 6257980

Browse files
authoredJul 15, 2024
Rollup merge of rust-lang#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.
2 parents 05614f3 + 5ff7b40 commit 6257980

File tree

18 files changed

+89
-66
lines changed

18 files changed

+89
-66
lines changed
 

‎std/src/collections/hash/map.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1018,7 +1018,7 @@ where
10181018
K: Borrow<Q>,
10191019
Q: Hash + Eq,
10201020
{
1021-
self.base.get_many_unchecked_mut(ks)
1021+
unsafe { self.base.get_many_unchecked_mut(ks) }
10221022
}
10231023

10241024
/// Returns `true` if the map contains a value for the specified key.

‎std/src/env.rs

+4-10
Original file line numberDiff line numberDiff line change
@@ -366,11 +366,8 @@ impl Error for VarError {
366366
#[rustc_deprecated_safe_2024]
367367
#[stable(feature = "env", since = "1.0.0")]
368368
pub unsafe fn set_var<K: AsRef<OsStr>, V: AsRef<OsStr>>(key: K, value: V) {
369-
_set_var(key.as_ref(), value.as_ref())
370-
}
371-
372-
unsafe fn _set_var(key: &OsStr, value: &OsStr) {
373-
os_imp::setenv(key, value).unwrap_or_else(|e| {
369+
let (key, value) = (key.as_ref(), value.as_ref());
370+
unsafe { os_imp::setenv(key, value) }.unwrap_or_else(|e| {
374371
panic!("failed to set environment variable `{key:?}` to `{value:?}`: {e}")
375372
})
376373
}
@@ -433,11 +430,8 @@ unsafe fn _set_var(key: &OsStr, value: &OsStr) {
433430
#[rustc_deprecated_safe_2024]
434431
#[stable(feature = "env", since = "1.0.0")]
435432
pub unsafe fn remove_var<K: AsRef<OsStr>>(key: K) {
436-
_remove_var(key.as_ref())
437-
}
438-
439-
unsafe fn _remove_var(key: &OsStr) {
440-
os_imp::unsetenv(key)
433+
let key = key.as_ref();
434+
unsafe { os_imp::unsetenv(key) }
441435
.unwrap_or_else(|e| panic!("failed to remove environment variable `{key:?}`: {e}"))
442436
}
443437

‎std/src/ffi/os_str.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ impl OsString {
184184
#[inline]
185185
#[stable(feature = "os_str_bytes", since = "1.74.0")]
186186
pub unsafe fn from_encoded_bytes_unchecked(bytes: Vec<u8>) -> Self {
187-
OsString { inner: Buf::from_encoded_bytes_unchecked(bytes) }
187+
OsString { inner: unsafe { Buf::from_encoded_bytes_unchecked(bytes) } }
188188
}
189189

190190
/// Converts to an [`OsStr`] slice.
@@ -813,7 +813,7 @@ impl OsStr {
813813
#[inline]
814814
#[stable(feature = "os_str_bytes", since = "1.74.0")]
815815
pub unsafe fn from_encoded_bytes_unchecked(bytes: &[u8]) -> &Self {
816-
Self::from_inner(Slice::from_encoded_bytes_unchecked(bytes))
816+
Self::from_inner(unsafe { Slice::from_encoded_bytes_unchecked(bytes) })
817817
}
818818

819819
#[inline]

‎std/src/io/buffered/bufwriter.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -433,9 +433,11 @@ impl<W: ?Sized + Write> BufWriter<W> {
433433
let old_len = self.buf.len();
434434
let buf_len = buf.len();
435435
let src = buf.as_ptr();
436-
let dst = self.buf.as_mut_ptr().add(old_len);
437-
ptr::copy_nonoverlapping(src, dst, buf_len);
438-
self.buf.set_len(old_len + buf_len);
436+
unsafe {
437+
let dst = self.buf.as_mut_ptr().add(old_len);
438+
ptr::copy_nonoverlapping(src, dst, buf_len);
439+
self.buf.set_len(old_len + buf_len);
440+
}
439441
}
440442

441443
#[inline]

‎std/src/io/cursor.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ where
482482
A: Allocator,
483483
{
484484
debug_assert!(vec.capacity() >= pos + buf.len());
485-
vec.as_mut_ptr().add(pos).copy_from(buf.as_ptr(), buf.len());
485+
unsafe { vec.as_mut_ptr().add(pos).copy_from(buf.as_ptr(), buf.len()) };
486486
pos + buf.len()
487487
}
488488

‎std/src/io/error/repr_bitpacked.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -267,11 +267,14 @@ where
267267
// Using this rather than unwrap meaningfully improves the code
268268
// for callers which only care about one variant (usually
269269
// `Custom`)
270-
core::hint::unreachable_unchecked();
270+
unsafe { core::hint::unreachable_unchecked() };
271271
});
272272
ErrorData::Simple(kind)
273273
}
274-
TAG_SIMPLE_MESSAGE => ErrorData::SimpleMessage(&*ptr.cast::<SimpleMessage>().as_ptr()),
274+
TAG_SIMPLE_MESSAGE => {
275+
// SAFETY: per tag
276+
unsafe { ErrorData::SimpleMessage(&*ptr.cast::<SimpleMessage>().as_ptr()) }
277+
}
275278
TAG_CUSTOM => {
276279
// It would be correct for us to use `ptr::byte_sub` here (see the
277280
// comment above the `wrapping_add` call in `new_custom` for why),

‎std/src/io/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -382,11 +382,11 @@ pub(crate) unsafe fn append_to_string<F>(buf: &mut String, f: F) -> Result<usize
382382
where
383383
F: FnOnce(&mut Vec<u8>) -> Result<usize>,
384384
{
385-
let mut g = Guard { len: buf.len(), buf: buf.as_mut_vec() };
385+
let mut g = Guard { len: buf.len(), buf: unsafe { buf.as_mut_vec() } };
386386
let ret = f(g.buf);
387387

388388
// SAFETY: the caller promises to only append data to `buf`
389-
let appended = g.buf.get_unchecked(g.len..);
389+
let appended = unsafe { g.buf.get_unchecked(g.len..) };
390390
if str::from_utf8(appended).is_err() {
391391
ret.and_then(|_| Err(Error::INVALID_UTF8))
392392
} else {

‎std/src/lib.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@
252252
#![allow(internal_features)]
253253
#![deny(rustc::existing_doc_keyword)]
254254
#![deny(fuzzy_provenance_casts)]
255+
#![deny(unsafe_op_in_unsafe_fn)]
255256
#![allow(rustdoc::redundant_explicit_links)]
256257
// Ensure that std can be linked against panic_abort despite compiled with `-C panic=unwind`
257258
#![deny(ffi_unwind_calls)]
@@ -664,7 +665,7 @@ pub mod alloc;
664665
mod panicking;
665666

666667
#[path = "../../backtrace/src/lib.rs"]
667-
#[allow(dead_code, unused_attributes, fuzzy_provenance_casts)]
668+
#[allow(dead_code, unused_attributes, fuzzy_provenance_casts, unsafe_op_in_unsafe_fn)]
668669
mod backtrace_rs;
669670

670671
// Re-export macros defined in core.

‎std/src/os/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
#![stable(feature = "os", since = "1.0.0")]
44
#![allow(missing_docs, nonstandard_style, missing_debug_implementations)]
5+
#![allow(unsafe_op_in_unsafe_fn)]
56

67
pub mod raw;
78

‎std/src/sync/mpmc/array.rs

+13-9
Original file line numberDiff line numberDiff line change
@@ -200,11 +200,12 @@ impl<T> Channel<T> {
200200
return Err(msg);
201201
}
202202

203-
let slot: &Slot<T> = &*(token.array.slot as *const Slot<T>);
204-
205203
// Write the message into the slot and update the stamp.
206-
slot.msg.get().write(MaybeUninit::new(msg));
207-
slot.stamp.store(token.array.stamp, Ordering::Release);
204+
unsafe {
205+
let slot: &Slot<T> = &*(token.array.slot as *const Slot<T>);
206+
slot.msg.get().write(MaybeUninit::new(msg));
207+
slot.stamp.store(token.array.stamp, Ordering::Release);
208+
}
208209

209210
// Wake a sleeping receiver.
210211
self.receivers.notify();
@@ -291,11 +292,14 @@ impl<T> Channel<T> {
291292
return Err(());
292293
}
293294

294-
let slot: &Slot<T> = &*(token.array.slot as *const Slot<T>);
295-
296295
// Read the message from the slot and update the stamp.
297-
let msg = slot.msg.get().read().assume_init();
298-
slot.stamp.store(token.array.stamp, Ordering::Release);
296+
let msg = unsafe {
297+
let slot: &Slot<T> = &*(token.array.slot as *const Slot<T>);
298+
299+
let msg = slot.msg.get().read().assume_init();
300+
slot.stamp.store(token.array.stamp, Ordering::Release);
301+
msg
302+
};
299303

300304
// Wake a sleeping sender.
301305
self.senders.notify();
@@ -471,7 +475,7 @@ impl<T> Channel<T> {
471475
false
472476
};
473477

474-
self.discard_all_messages(tail);
478+
unsafe { self.discard_all_messages(tail) };
475479
disconnected
476480
}
477481

‎std/src/sync/mpmc/counter.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ impl<C> Sender<C> {
6363
disconnect(&self.counter().chan);
6464

6565
if self.counter().destroy.swap(true, Ordering::AcqRel) {
66-
drop(Box::from_raw(self.counter));
66+
drop(unsafe { Box::from_raw(self.counter) });
6767
}
6868
}
6969
}
@@ -116,7 +116,7 @@ impl<C> Receiver<C> {
116116
disconnect(&self.counter().chan);
117117

118118
if self.counter().destroy.swap(true, Ordering::AcqRel) {
119-
drop(Box::from_raw(self.counter));
119+
drop(unsafe { Box::from_raw(self.counter) });
120120
}
121121
}
122122
}

‎std/src/sync/mpmc/list.rs

+21-17
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ impl<T> Block<T> {
9191
// It is not necessary to set the `DESTROY` bit in the last slot because that slot has
9292
// begun destruction of the block.
9393
for i in start..BLOCK_CAP - 1 {
94-
let slot = (*this).slots.get_unchecked(i);
94+
let slot = unsafe { (*this).slots.get_unchecked(i) };
9595

9696
// Mark the `DESTROY` bit if a thread is still using the slot.
9797
if slot.state.load(Ordering::Acquire) & READ == 0
@@ -103,7 +103,7 @@ impl<T> Block<T> {
103103
}
104104

105105
// No thread is using the block, now it is safe to destroy it.
106-
drop(Box::from_raw(this));
106+
drop(unsafe { Box::from_raw(this) });
107107
}
108108
}
109109

@@ -265,9 +265,11 @@ impl<T> Channel<T> {
265265
// Write the message into the slot.
266266
let block = token.list.block as *mut Block<T>;
267267
let offset = token.list.offset;
268-
let slot = (*block).slots.get_unchecked(offset);
269-
slot.msg.get().write(MaybeUninit::new(msg));
270-
slot.state.fetch_or(WRITE, Ordering::Release);
268+
unsafe {
269+
let slot = (*block).slots.get_unchecked(offset);
270+
slot.msg.get().write(MaybeUninit::new(msg));
271+
slot.state.fetch_or(WRITE, Ordering::Release);
272+
}
271273

272274
// Wake a sleeping receiver.
273275
self.receivers.notify();
@@ -369,19 +371,21 @@ impl<T> Channel<T> {
369371
// Read the message.
370372
let block = token.list.block as *mut Block<T>;
371373
let offset = token.list.offset;
372-
let slot = (*block).slots.get_unchecked(offset);
373-
slot.wait_write();
374-
let msg = slot.msg.get().read().assume_init();
375-
376-
// Destroy the block if we've reached the end, or if another thread wanted to destroy but
377-
// couldn't because we were busy reading from the slot.
378-
if offset + 1 == BLOCK_CAP {
379-
Block::destroy(block, 0);
380-
} else if slot.state.fetch_or(READ, Ordering::AcqRel) & DESTROY != 0 {
381-
Block::destroy(block, offset + 1);
382-
}
374+
unsafe {
375+
let slot = (*block).slots.get_unchecked(offset);
376+
slot.wait_write();
377+
let msg = slot.msg.get().read().assume_init();
378+
379+
// Destroy the block if we've reached the end, or if another thread wanted to destroy but
380+
// couldn't because we were busy reading from the slot.
381+
if offset + 1 == BLOCK_CAP {
382+
Block::destroy(block, 0);
383+
} else if slot.state.fetch_or(READ, Ordering::AcqRel) & DESTROY != 0 {
384+
Block::destroy(block, offset + 1);
385+
}
383386

384-
Ok(msg)
387+
Ok(msg)
388+
}
385389
}
386390

387391
/// Attempts to send a message into the channel.

‎std/src/sync/mpmc/zero.rs

+12-8
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,11 @@ impl<T> Channel<T> {
103103
return Err(msg);
104104
}
105105

106-
let packet = &*(token.zero.0 as *const Packet<T>);
107-
packet.msg.get().write(Some(msg));
108-
packet.ready.store(true, Ordering::Release);
106+
unsafe {
107+
let packet = &*(token.zero.0 as *const Packet<T>);
108+
packet.msg.get().write(Some(msg));
109+
packet.ready.store(true, Ordering::Release);
110+
}
109111
Ok(())
110112
}
111113

@@ -116,22 +118,24 @@ impl<T> Channel<T> {
116118
return Err(());
117119
}
118120

119-
let packet = &*(token.zero.0 as *const Packet<T>);
121+
let packet = unsafe { &*(token.zero.0 as *const Packet<T>) };
120122

121123
if packet.on_stack {
122124
// The message has been in the packet from the beginning, so there is no need to wait
123125
// for it. However, after reading the message, we need to set `ready` to `true` in
124126
// order to signal that the packet can be destroyed.
125-
let msg = packet.msg.get().replace(None).unwrap();
127+
let msg = unsafe { packet.msg.get().replace(None) }.unwrap();
126128
packet.ready.store(true, Ordering::Release);
127129
Ok(msg)
128130
} else {
129131
// Wait until the message becomes available, then read it and destroy the
130132
// heap-allocated packet.
131133
packet.wait_ready();
132-
let msg = packet.msg.get().replace(None).unwrap();
133-
drop(Box::from_raw(token.zero.0 as *mut Packet<T>));
134-
Ok(msg)
134+
unsafe {
135+
let msg = packet.msg.get().replace(None).unwrap();
136+
drop(Box::from_raw(token.zero.0 as *mut Packet<T>));
137+
Ok(msg)
138+
}
135139
}
136140
}
137141

‎std/src/sync/once_lock.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ impl<T> OnceLock<T> {
502502
#[inline]
503503
unsafe fn get_unchecked(&self) -> &T {
504504
debug_assert!(self.is_initialized());
505-
(&*self.value.get()).assume_init_ref()
505+
unsafe { (&*self.value.get()).assume_init_ref() }
506506
}
507507

508508
/// # Safety
@@ -511,7 +511,7 @@ impl<T> OnceLock<T> {
511511
#[inline]
512512
unsafe fn get_unchecked_mut(&mut self) -> &mut T {
513513
debug_assert!(self.is_initialized());
514-
(&mut *self.value.get()).assume_init_mut()
514+
unsafe { (&mut *self.value.get()).assume_init_mut() }
515515
}
516516
}
517517

‎std/src/sync/reentrant_lock.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,9 @@ impl<T: ?Sized> ReentrantLock<T> {
244244
}
245245

246246
unsafe fn increment_lock_count(&self) -> Option<()> {
247-
*self.lock_count.get() = (*self.lock_count.get()).checked_add(1)?;
247+
unsafe {
248+
*self.lock_count.get() = (*self.lock_count.get()).checked_add(1)?;
249+
}
248250
Some(())
249251
}
250252
}

‎std/src/sync/rwlock.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ impl<'rwlock, T: ?Sized> RwLockReadGuard<'rwlock, T> {
578578
// successfully called from the same thread before instantiating this object.
579579
unsafe fn new(lock: &'rwlock RwLock<T>) -> LockResult<RwLockReadGuard<'rwlock, T>> {
580580
poison::map_result(lock.poison.borrow(), |()| RwLockReadGuard {
581-
data: NonNull::new_unchecked(lock.data.get()),
581+
data: unsafe { NonNull::new_unchecked(lock.data.get()) },
582582
inner_lock: &lock.inner,
583583
})
584584
}

‎std/src/sys/mod.rs

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

‎std/src/sys_common/wtf8.rs

+10-4
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,8 @@ impl Wtf8 {
602602
/// marked unsafe.
603603
#[inline]
604604
pub unsafe fn from_bytes_unchecked(value: &[u8]) -> &Wtf8 {
605-
mem::transmute(value)
605+
// SAFETY: start with &[u8], end with fancy &[u8]
606+
unsafe { &*(value as *const [u8] as *const Wtf8) }
606607
}
607608

608609
/// Creates a mutable WTF-8 slice from a mutable WTF-8 byte slice.
@@ -611,7 +612,8 @@ impl Wtf8 {
611612
/// marked unsafe.
612613
#[inline]
613614
unsafe fn from_mut_bytes_unchecked(value: &mut [u8]) -> &mut Wtf8 {
614-
mem::transmute(value)
615+
// SAFETY: start with &mut [u8], end with fancy &mut [u8]
616+
unsafe { &mut *(value as *mut [u8] as *mut Wtf8) }
615617
}
616618

617619
/// Returns the length, in WTF-8 bytes.
@@ -942,8 +944,12 @@ pub fn check_utf8_boundary(slice: &Wtf8, index: usize) {
942944
/// Copied from core::str::raw::slice_unchecked
943945
#[inline]
944946
pub unsafe fn slice_unchecked(s: &Wtf8, begin: usize, end: usize) -> &Wtf8 {
945-
// memory layout of a &[u8] and &Wtf8 are the same
946-
Wtf8::from_bytes_unchecked(slice::from_raw_parts(s.bytes.as_ptr().add(begin), end - begin))
947+
// SAFETY: memory layout of a &[u8] and &Wtf8 are the same
948+
unsafe {
949+
let len = end - begin;
950+
let start = s.as_bytes().as_ptr().add(begin);
951+
Wtf8::from_bytes_unchecked(slice::from_raw_parts(start, len))
952+
}
947953
}
948954

949955
/// Copied from core::str::raw::slice_error_fail

0 commit comments

Comments
 (0)
Failed to load comments.