Skip to content

Commit 8a515e9

Browse files
LeSeulArtichautLeSeulArtichaut
committed
Deny unsafe ops in unsafe fns, part 2
1 parent 8ee1dec commit 8a515e9

File tree

9 files changed

+103
-37
lines changed

9 files changed

+103
-37
lines changed

‎src/libcore/hash/sip.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! An implementation of SipHash.
22
33
#![allow(deprecated)] // the types in this module are deprecated
4+
#![deny(unsafe_op_in_unsafe_fn)]
45

56
use crate::cmp;
67
use crate::marker::PhantomData;
@@ -130,15 +131,19 @@ unsafe fn u8to64_le(buf: &[u8], start: usize, len: usize) -> u64 {
130131
let mut i = 0; // current byte index (from LSB) in the output u64
131132
let mut out = 0;
132133
if i + 3 < len {
133-
out = load_int_le!(buf, start + i, u32) as u64;
134+
// SAFETY: `i` cannot be greater than `len`, and the caller must guarantee
135+
// that the index start..start+len is in bounds.
136+
out = unsafe { load_int_le!(buf, start + i, u32) } as u64;
134137
i += 4;
135138
}
136139
if i + 1 < len {
137-
out |= (load_int_le!(buf, start + i, u16) as u64) << (i * 8);
140+
// SAFETY: same as above.
141+
out |= (unsafe { load_int_le!(buf, start + i, u16) } as u64) << (i * 8);
138142
i += 2
139143
}
140144
if i < len {
141-
out |= (*buf.get_unchecked(start + i) as u64) << (i * 8);
145+
// SAFETY: same as above.
146+
out |= (unsafe { *buf.get_unchecked(start + i) } as u64) << (i * 8);
142147
i += 1;
143148
}
144149
debug_assert_eq!(i, len);

‎src/libcore/iter/adapters/fuse.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,10 @@ where
178178
{
179179
unsafe fn get_unchecked(&mut self, i: usize) -> I::Item {
180180
match self.iter {
181-
Some(ref mut iter) => iter.get_unchecked(i),
181+
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
182+
Some(ref mut iter) => unsafe { iter.get_unchecked(i) },
182183
// SAFETY: the caller asserts there is an item at `i`, so we're not exhausted.
183-
None => intrinsics::unreachable(),
184+
None => unsafe { intrinsics::unreachable() },
184185
}
185186
}
186187

‎src/libcore/iter/adapters/mod.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,8 @@ where
272272
T: Copy,
273273
{
274274
unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
275-
*self.it.get_unchecked(i)
275+
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
276+
unsafe { *self.it.get_unchecked(i) }
276277
}
277278

278279
#[inline]
@@ -402,7 +403,8 @@ where
402403
T: Clone,
403404
{
404405
default unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
405-
self.it.get_unchecked(i).clone()
406+
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
407+
unsafe { self.it.get_unchecked(i) }.clone()
406408
}
407409

408410
#[inline]
@@ -418,7 +420,8 @@ where
418420
T: Copy,
419421
{
420422
unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
421-
*self.it.get_unchecked(i)
423+
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
424+
unsafe { *self.it.get_unchecked(i) }
422425
}
423426

424427
#[inline]
@@ -930,7 +933,8 @@ where
930933
F: FnMut(I::Item) -> B,
931934
{
932935
unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
933-
(self.f)(self.iter.get_unchecked(i))
936+
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
937+
(self.f)(unsafe { self.iter.get_unchecked(i) })
934938
}
935939
#[inline]
936940
fn may_have_side_effect() -> bool {
@@ -1392,7 +1396,8 @@ where
13921396
I: TrustedRandomAccess,
13931397
{
13941398
unsafe fn get_unchecked(&mut self, i: usize) -> (usize, I::Item) {
1395-
(self.count + i, self.iter.get_unchecked(i))
1399+
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
1400+
(self.count + i, unsafe { self.iter.get_unchecked(i) })
13961401
}
13971402

13981403
fn may_have_side_effect() -> bool {

‎src/libcore/iter/adapters/zip.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,8 @@ where
271271
B: TrustedRandomAccess,
272272
{
273273
unsafe fn get_unchecked(&mut self, i: usize) -> (A::Item, B::Item) {
274-
(self.a.get_unchecked(i), self.b.get_unchecked(i))
274+
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
275+
unsafe { (self.a.get_unchecked(i), self.b.get_unchecked(i)) }
275276
}
276277

277278
fn may_have_side_effect() -> bool {

‎src/libcore/iter/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,7 @@
309309
//! [`min`]: trait.Iterator.html#method.min
310310
311311
#![stable(feature = "rust1", since = "1.0.0")]
312+
#![deny(unsafe_op_in_unsafe_fn)]
312313

313314
use crate::ops::Try;
314315

‎src/libcore/iter/range.rs

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -189,12 +189,14 @@ macro_rules! step_identical_methods {
189189
() => {
190190
#[inline]
191191
unsafe fn forward_unchecked(start: Self, n: usize) -> Self {
192-
start.unchecked_add(n as Self)
192+
// SAFETY: the caller has to guarantee that `start + n` doesn't overflow.
193+
unsafe { start.unchecked_add(n as Self) }
193194
}
194195

195196
#[inline]
196197
unsafe fn backward_unchecked(start: Self, n: usize) -> Self {
197-
start.unchecked_sub(n as Self)
198+
// SAFETY: the caller has to guarantee that `start - n` doesn't overflow.
199+
unsafe { start.unchecked_sub(n as Self) }
198200
}
199201

200202
#[inline]
@@ -450,21 +452,33 @@ unsafe impl Step for char {
450452
#[inline]
451453
unsafe fn forward_unchecked(start: char, count: usize) -> char {
452454
let start = start as u32;
453-
let mut res = Step::forward_unchecked(start, count);
455+
// SAFETY: the caller must guarantee that this doesn't overflow
456+
// the range of values for a char.
457+
let mut res = unsafe { Step::forward_unchecked(start, count) };
454458
if start < 0xD800 && 0xD800 <= res {
455-
res = Step::forward_unchecked(res, 0x800);
459+
// SAFETY: the caller must guarantee that this doesn't overflow
460+
// the range of values for a char.
461+
res = unsafe { Step::forward_unchecked(res, 0x800) };
456462
}
457-
char::from_u32_unchecked(res)
463+
// SAFETY: because of the previous contract, this is guaranteed
464+
// by the caller to be a valid char.
465+
unsafe { char::from_u32_unchecked(res) }
458466
}
459467

460468
#[inline]
461469
unsafe fn backward_unchecked(start: char, count: usize) -> char {
462470
let start = start as u32;
463-
let mut res = Step::backward_unchecked(start, count);
471+
// SAFETY: the caller must guarantee that this doesn't overflow
472+
// the range of values for a char.
473+
let mut res = unsafe { Step::backward_unchecked(start, count) };
464474
if start >= 0xE000 && 0xE000 > res {
465-
res = Step::backward_unchecked(res, 0x800);
475+
// SAFETY: the caller must guarantee that this doesn't overflow
476+
// the range of values for a char.
477+
res = unsafe { Step::backward_unchecked(res, 0x800) };
466478
}
467-
char::from_u32_unchecked(res)
479+
// SAFETY: because of the previous contract, this is guaranteed
480+
// by the caller to be a valid char.
481+
unsafe { char::from_u32_unchecked(res) }
468482
}
469483
}
470484

‎src/libcore/mem/manually_drop.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ impl<T> ManuallyDrop<T> {
122122
#[stable(feature = "manually_drop_take", since = "1.42.0")]
123123
#[inline]
124124
pub unsafe fn take(slot: &mut ManuallyDrop<T>) -> T {
125-
ptr::read(&slot.value)
125+
// SAFETY: we are reading from a reference, which is guaranteed
126+
// to be valid for reads.
127+
unsafe { ptr::read(&slot.value) }
126128
}
127129
}
128130

@@ -152,7 +154,10 @@ impl<T: ?Sized> ManuallyDrop<T> {
152154
#[stable(feature = "manually_drop", since = "1.20.0")]
153155
#[inline]
154156
pub unsafe fn drop(slot: &mut ManuallyDrop<T>) {
155-
ptr::drop_in_place(&mut slot.value)
157+
// SAFETY: we are dropping the value pointed to by a mutable reference
158+
// which is guaranteed to be valid for writes.
159+
// It is up to the caller to make sure that `slot` isn't dropped again.
160+
unsafe { ptr::drop_in_place(&mut slot.value) }
156161
}
157162
}
158163

‎src/libcore/mem/maybe_uninit.rs

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -494,8 +494,12 @@ impl<T> MaybeUninit<T> {
494494
#[inline(always)]
495495
#[rustc_diagnostic_item = "assume_init"]
496496
pub unsafe fn assume_init(self) -> T {
497-
intrinsics::assert_inhabited::<T>();
498-
ManuallyDrop::into_inner(self.value)
497+
// SAFETY: the caller must guarantee that `self` is initialized.
498+
// This also means that `self` must be a `value` variant.
499+
unsafe {
500+
intrinsics::assert_inhabited::<T>();
501+
ManuallyDrop::into_inner(self.value)
502+
}
499503
}
500504

501505
/// Reads the value from the `MaybeUninit<T>` container. The resulting `T` is subject
@@ -558,8 +562,12 @@ impl<T> MaybeUninit<T> {
558562
#[unstable(feature = "maybe_uninit_extra", issue = "63567")]
559563
#[inline(always)]
560564
pub unsafe fn read(&self) -> T {
561-
intrinsics::assert_inhabited::<T>();
562-
self.as_ptr().read()
565+
// SAFETY: the caller must guarantee that `self` is initialized.
566+
// Reading from `self.as_ptr()` is safe since `self` should be initialized.
567+
unsafe {
568+
intrinsics::assert_inhabited::<T>();
569+
self.as_ptr().read()
570+
}
563571
}
564572

565573
/// Gets a shared reference to the contained value.
@@ -620,8 +628,12 @@ impl<T> MaybeUninit<T> {
620628
#[unstable(feature = "maybe_uninit_ref", issue = "63568")]
621629
#[inline(always)]
622630
pub unsafe fn get_ref(&self) -> &T {
623-
intrinsics::assert_inhabited::<T>();
624-
&*self.value
631+
// SAFETY: the caller must guarantee that `self` is initialized.
632+
// This also means that `self` must be a `value` variant.
633+
unsafe {
634+
intrinsics::assert_inhabited::<T>();
635+
&*self.value
636+
}
625637
}
626638

627639
/// Gets a mutable (unique) reference to the contained value.
@@ -738,8 +750,12 @@ impl<T> MaybeUninit<T> {
738750
#[unstable(feature = "maybe_uninit_ref", issue = "63568")]
739751
#[inline(always)]
740752
pub unsafe fn get_mut(&mut self) -> &mut T {
741-
intrinsics::assert_inhabited::<T>();
742-
&mut *self.value
753+
// SAFETY: the caller must guarantee that `self` is initialized.
754+
// This also means that `self` must be a `value` variant.
755+
unsafe {
756+
intrinsics::assert_inhabited::<T>();
757+
&mut *self.value
758+
}
743759
}
744760

745761
/// Assuming all the elements are initialized, get a slice to them.
@@ -752,7 +768,11 @@ impl<T> MaybeUninit<T> {
752768
#[unstable(feature = "maybe_uninit_slice_assume_init", issue = "none")]
753769
#[inline(always)]
754770
pub unsafe fn slice_get_ref(slice: &[Self]) -> &[T] {
755-
&*(slice as *const [Self] as *const [T])
771+
// SAFETY: casting slice to a `*const [T]` is safe since the caller guarantees that
772+
// `slice` is initialized, and`MaybeUninit` is guaranteed to have the same layout as `T`.
773+
// The pointer obtained is valid since it refers to memory owned by `slice` which is a
774+
// reference and thus guaranteed to be valid for reads.
775+
unsafe { &*(slice as *const [Self] as *const [T]) }
756776
}
757777

758778
/// Assuming all the elements are initialized, get a mutable slice to them.
@@ -765,7 +785,9 @@ impl<T> MaybeUninit<T> {
765785
#[unstable(feature = "maybe_uninit_slice_assume_init", issue = "none")]
766786
#[inline(always)]
767787
pub unsafe fn slice_get_mut(slice: &mut [Self]) -> &mut [T] {
768-
&mut *(slice as *mut [Self] as *mut [T])
788+
// SAFETY: similar to safety notes for `slice_get_ref`, but we have a
789+
// mutable reference which is also guaranteed to be valid for writes.
790+
unsafe { &mut *(slice as *mut [Self] as *mut [T]) }
769791
}
770792

771793
/// Gets a pointer to the first element of the array.

‎src/libcore/mem/mod.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! types, initializing and manipulating memory.
55
66
#![stable(feature = "rust1", since = "1.0.0")]
7+
#![deny(unsafe_op_in_unsafe_fn)]
78

89
use crate::clone;
910
use crate::cmp;
@@ -623,8 +624,11 @@ pub const fn needs_drop<T>() -> bool {
623624
#[allow(deprecated)]
624625
#[rustc_diagnostic_item = "mem_zeroed"]
625626
pub unsafe fn zeroed<T>() -> T {
626-
intrinsics::assert_zero_valid::<T>();
627-
MaybeUninit::zeroed().assume_init()
627+
// SAFETY: the caller must guarantee that an all-zero value is valid for `T`.
628+
unsafe {
629+
intrinsics::assert_zero_valid::<T>();
630+
MaybeUninit::zeroed().assume_init()
631+
}
628632
}
629633

630634
/// Bypasses Rust's normal memory-initialization checks by pretending to
@@ -656,8 +660,11 @@ pub unsafe fn zeroed<T>() -> T {
656660
#[allow(deprecated)]
657661
#[rustc_diagnostic_item = "mem_uninitialized"]
658662
pub unsafe fn uninitialized<T>() -> T {
659-
intrinsics::assert_uninit_valid::<T>();
660-
MaybeUninit::uninit().assume_init()
663+
// SAFETY: the caller must guarantee that an unitialized value is valid for `T`.
664+
unsafe {
665+
intrinsics::assert_uninit_valid::<T>();
666+
MaybeUninit::uninit().assume_init()
667+
}
661668
}
662669

663670
/// Swaps the values at two mutable locations, without deinitializing either one.
@@ -922,9 +929,14 @@ pub fn drop<T>(_x: T) {}
922929
pub unsafe fn transmute_copy<T, U>(src: &T) -> U {
923930
// If U has a higher alignment requirement, src may not be suitably aligned.
924931
if align_of::<U>() > align_of::<T>() {
925-
ptr::read_unaligned(src as *const T as *const U)
932+
// SAFETY: `src` is a reference which is guaranteed to be valid for reads.
933+
// The caller must guarantee that the actual transmutation is safe.
934+
unsafe { ptr::read_unaligned(src as *const T as *const U) }
926935
} else {
927-
ptr::read(src as *const T as *const U)
936+
// SAFETY: `src` is a reference which is guaranteed to be valid for reads.
937+
// We just checked that `src as *const U` was properly aligned.
938+
// The caller must guarantee that the actual transmutation is safe.
939+
unsafe { ptr::read(src as *const T as *const U) }
928940
}
929941
}
930942

0 commit comments

Comments
 (0)