Skip to content

Commit

Permalink
Rollup merge of rust-lang#73622 - LeSeulArtichaut:unsafe-libcore, r=n…
Browse files Browse the repository at this point in the history
…ikomatsakis

Deny unsafe ops in unsafe fns in libcore

After `liballoc`, It's time for `libcore` :D

I planned to do this bit by bit to avoid having a big chunk of diffs, so to make reviews easier, and to make the unsafe blocks narrower and take the time to document them properly.

r? @nikomatsakis cc @RalfJung
  • Loading branch information
Manishearth authored Jul 2, 2020
2 parents 1c68bb6 + 6a7a652 commit 500634b
Show file tree
Hide file tree
Showing 38 changed files with 877 additions and 436 deletions.
22 changes: 16 additions & 6 deletions src/libcore/alloc/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,12 @@ pub unsafe trait GlobalAlloc {
#[stable(feature = "global_alloc", since = "1.28.0")]
unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
let size = layout.size();
let ptr = self.alloc(layout);
// SAFETY: the safety contract for `alloc` must be upheld by the caller.
let ptr = unsafe { self.alloc(layout) };
if !ptr.is_null() {
ptr::write_bytes(ptr, 0, size);
// SAFETY: as allocation succeeded, the region from `ptr`
// of size `size` is guaranteed to be valid for writes.
unsafe { ptr::write_bytes(ptr, 0, size) };
}
ptr
}
Expand Down Expand Up @@ -187,11 +190,18 @@ pub unsafe trait GlobalAlloc {
/// [`handle_alloc_error`]: ../../alloc/alloc/fn.handle_alloc_error.html
#[stable(feature = "global_alloc", since = "1.28.0")]
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
let new_ptr = self.alloc(new_layout);
// SAFETY: the caller must ensure that the `new_size` does not overflow.
// `layout.align()` comes from a `Layout` and is thus guaranteed to be valid.
let new_layout = unsafe { Layout::from_size_align_unchecked(new_size, layout.align()) };
// SAFETY: the caller must ensure that `new_layout` is greater than zero.
let new_ptr = unsafe { self.alloc(new_layout) };
if !new_ptr.is_null() {
ptr::copy_nonoverlapping(ptr, new_ptr, cmp::min(layout.size(), new_size));
self.dealloc(ptr, layout);
// SAFETY: the previously allocated block cannot overlap the newly allocated block.
// The safety contract for `dealloc` must be upheld by the caller.
unsafe {
ptr::copy_nonoverlapping(ptr, new_ptr, cmp::min(layout.size(), new_size));
self.dealloc(ptr, layout);
}
}
new_ptr
}
Expand Down
3 changes: 2 additions & 1 deletion src/libcore/alloc/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ impl Layout {
#[rustc_const_stable(feature = "alloc_layout", since = "1.28.0")]
#[inline]
pub const unsafe fn from_size_align_unchecked(size: usize, align: usize) -> Self {
Layout { size_: size, align_: NonZeroUsize::new_unchecked(align) }
// SAFETY: the caller must ensure that `align` is greater than zero.
Layout { size_: size, align_: unsafe { NonZeroUsize::new_unchecked(align) } }
}

/// The minimum size in bytes for a memory block of this layout.
Expand Down
58 changes: 45 additions & 13 deletions src/libcore/alloc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ impl AllocInit {
#[inline]
#[unstable(feature = "allocator_api", issue = "32838")]
pub unsafe fn init(self, memory: MemoryBlock) {
self.init_offset(memory, 0)
// SAFETY: the safety contract for `init_offset` must be
// upheld by the caller.
unsafe { self.init_offset(memory, 0) }
}

/// Initialize the memory block like specified by `init` at the specified `offset`.
Expand All @@ -78,7 +80,10 @@ impl AllocInit {
match self {
AllocInit::Uninitialized => (),
AllocInit::Zeroed => {
memory.ptr.as_ptr().add(offset).write_bytes(0, memory.size - offset)
// SAFETY: the caller must guarantee that `offset` is smaller than or equal to `memory.size`,
// so the memory from `memory.ptr + offset` of length `memory.size - offset`
// is guaranteed to be contaned in `memory` and thus valid for writes.
unsafe { memory.ptr.as_ptr().add(offset).write_bytes(0, memory.size - offset) }
}
}
}
Expand Down Expand Up @@ -281,11 +286,23 @@ pub unsafe trait AllocRef {
return Ok(MemoryBlock { ptr, size });
}

let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
let new_layout =
// SAFETY: the caller must ensure that the `new_size` does not overflow.
// `layout.align()` comes from a `Layout` and is thus guaranteed to be valid for a Layout.
// The caller must ensure that `new_size` is greater than zero.
unsafe { Layout::from_size_align_unchecked(new_size, layout.align()) };
let new_memory = self.alloc(new_layout, init)?;
ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), size);
self.dealloc(ptr, layout);
Ok(new_memory)

// SAFETY: because `new_size` must be greater than or equal to `size`, both the old and new
// memory allocation are valid for reads and writes for `size` bytes. Also, because the old
// allocation wasn't yet deallocated, it cannot overlap `new_memory`. Thus, the call to
// `copy_nonoverlapping` is safe.
// The safety contract for `dealloc` must be upheld by the caller.
unsafe {
ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), size);
self.dealloc(ptr, layout);
Ok(new_memory)
}
}
}
}
Expand Down Expand Up @@ -356,11 +373,23 @@ pub unsafe trait AllocRef {
return Ok(MemoryBlock { ptr, size });
}

let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
let new_layout =
// SAFETY: the caller must ensure that the `new_size` does not overflow.
// `layout.align()` comes from a `Layout` and is thus guaranteed to be valid for a Layout.
// The caller must ensure that `new_size` is greater than zero.
unsafe { Layout::from_size_align_unchecked(new_size, layout.align()) };
let new_memory = self.alloc(new_layout, AllocInit::Uninitialized)?;
ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), new_size);
self.dealloc(ptr, layout);
Ok(new_memory)

// SAFETY: because `new_size` must be lower than or equal to `size`, both the old and new
// memory allocation are valid for reads and writes for `new_size` bytes. Also, because the
// old allocation wasn't yet deallocated, it cannot overlap `new_memory`. Thus, the call to
// `copy_nonoverlapping` is safe.
// The safety contract for `dealloc` must be upheld by the caller.
unsafe {
ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), new_size);
self.dealloc(ptr, layout);
Ok(new_memory)
}
}
}
}
Expand All @@ -386,7 +415,8 @@ where

#[inline]
unsafe fn dealloc(&mut self, ptr: NonNull<u8>, layout: Layout) {
(**self).dealloc(ptr, layout)
// SAFETY: the safety contract must be upheld by the caller
unsafe { (**self).dealloc(ptr, layout) }
}

#[inline]
Expand All @@ -398,7 +428,8 @@ where
placement: ReallocPlacement,
init: AllocInit,
) -> Result<MemoryBlock, AllocErr> {
(**self).grow(ptr, layout, new_size, placement, init)
// SAFETY: the safety contract must be upheld by the caller
unsafe { (**self).grow(ptr, layout, new_size, placement, init) }
}

#[inline]
Expand All @@ -409,6 +440,7 @@ where
new_size: usize,
placement: ReallocPlacement,
) -> Result<MemoryBlock, AllocErr> {
(**self).shrink(ptr, layout, new_size, placement)
// SAFETY: the safety contract must be upheld by the caller
unsafe { (**self).shrink(ptr, layout, new_size, placement) }
}
}
7 changes: 6 additions & 1 deletion src/libcore/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,12 @@ impl<T: ?Sized> RefCell<T> {
#[inline]
pub unsafe fn try_borrow_unguarded(&self) -> Result<&T, BorrowError> {
if !is_writing(self.borrow.get()) {
Ok(&*self.value.get())
// SAFETY: We check that nobody is actively writing now, but it is
// the caller's responsibility to ensure that nobody writes until
// the returned reference is no longer in use.
// Also, `self.value.get()` refers to the value owned by `self`
// and is thus guaranteed to be valid for the lifetime of `self`.
Ok(unsafe { &*self.value.get() })
} else {
Err(BorrowError { _private: () })
}
Expand Down
3 changes: 2 additions & 1 deletion src/libcore/char/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ pub fn from_u32(i: u32) -> Option<char> {
#[inline]
#[stable(feature = "char_from_unchecked", since = "1.5.0")]
pub unsafe fn from_u32_unchecked(i: u32) -> char {
if cfg!(debug_assertions) { char::from_u32(i).unwrap() } else { transmute(i) }
// SAFETY: the caller must guarantee that `i` is a valid char value.
if cfg!(debug_assertions) { char::from_u32(i).unwrap() } else { unsafe { transmute(i) } }
}

#[stable(feature = "char_convert", since = "1.13.0")]
Expand Down
3 changes: 2 additions & 1 deletion src/libcore/char/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ impl char {
#[unstable(feature = "assoc_char_funcs", reason = "recently added", issue = "71763")]
#[inline]
pub unsafe fn from_u32_unchecked(i: u32) -> char {
super::convert::from_u32_unchecked(i)
// SAFETY: the safety contract must be upheld by the caller.
unsafe { super::convert::from_u32_unchecked(i) }
}

/// Converts a digit in the given radix to a `char`.
Expand Down
3 changes: 2 additions & 1 deletion src/libcore/convert/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ macro_rules! impl_float_to_int {
#[doc(hidden)]
#[inline]
unsafe fn to_int_unchecked(self) -> $Int {
crate::intrinsics::float_to_int_unchecked(self)
// SAFETY: the safety contract must be upheld by the caller.
unsafe { crate::intrinsics::float_to_int_unchecked(self) }
}
}
)+
Expand Down
8 changes: 6 additions & 2 deletions src/libcore/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,8 @@ impl<'f> VaListImpl<'f> {
/// Advance to the next arg.
#[inline]
pub unsafe fn arg<T: sealed_trait::VaArgSafe>(&mut self) -> T {
va_arg(self)
// SAFETY: the caller must uphold the safety contract for `va_arg`.
unsafe { va_arg(self) }
}

/// Copies the `va_list` at the current location.
Expand All @@ -343,7 +344,10 @@ impl<'f> VaListImpl<'f> {
{
let mut ap = self.clone();
let ret = f(ap.as_va_list());
va_end(&mut ap);
// SAFETY: the caller must uphold the safety contract for `va_end`.
unsafe {
va_end(&mut ap);
}
ret
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/libcore/future/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,7 @@ where
#[unstable(feature = "gen_future", issue = "50547")]
#[inline]
pub unsafe fn get_context<'a, 'b>(cx: ResumeTy) -> &'a mut Context<'b> {
&mut *cx.0.as_ptr().cast()
// SAFETY: the caller must guarantee that `cx.0` is a valid pointer
// that fulfills all the requirements for a mutable reference.
unsafe { &mut *cx.0.as_ptr().cast() }
}
10 changes: 7 additions & 3 deletions src/libcore/hash/sip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,19 @@ unsafe fn u8to64_le(buf: &[u8], start: usize, len: usize) -> u64 {
let mut i = 0; // current byte index (from LSB) in the output u64
let mut out = 0;
if i + 3 < len {
out = load_int_le!(buf, start + i, u32) as u64;
// SAFETY: `i` cannot be greater than `len`, and the caller must guarantee
// that the index start..start+len is in bounds.
out = unsafe { load_int_le!(buf, start + i, u32) } as u64;
i += 4;
}
if i + 1 < len {
out |= (load_int_le!(buf, start + i, u16) as u64) << (i * 8);
// SAFETY: same as above.
out |= (unsafe { load_int_le!(buf, start + i, u16) } as u64) << (i * 8);
i += 2
}
if i < len {
out |= (*buf.get_unchecked(start + i) as u64) << (i * 8);
// SAFETY: same as above.
out |= (unsafe { *buf.get_unchecked(start + i) } as u64) << (i * 8);
i += 1;
}
debug_assert_eq!(i, len);
Expand Down
4 changes: 3 additions & 1 deletion src/libcore/hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ use crate::intrinsics;
#[inline]
#[stable(feature = "unreachable", since = "1.27.0")]
pub unsafe fn unreachable_unchecked() -> ! {
intrinsics::unreachable()
// SAFETY: the safety contract for `intrinsics::unreachable` must
// be upheld by the caller.
unsafe { intrinsics::unreachable() }
}

/// Emits a machine instruction hinting to the processor that it is running in busy-wait
Expand Down
13 changes: 10 additions & 3 deletions src/libcore/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2099,7 +2099,10 @@ pub unsafe fn copy_nonoverlapping<T>(src: *const T, dst: *mut T, count: usize) {
// Not panicking to keep codegen impact smaller.
abort();
}
copy_nonoverlapping(src, dst, count)

// SAFETY: the safety contract for `copy_nonoverlapping` must be
// upheld by the caller.
unsafe { copy_nonoverlapping(src, dst, count) }
}

/// Copies `count * size_of::<T>()` bytes from `src` to `dst`. The source
Expand Down Expand Up @@ -2165,7 +2168,9 @@ pub unsafe fn copy<T>(src: *const T, dst: *mut T, count: usize) {
// Not panicking to keep codegen impact smaller.
abort();
}
copy(src, dst, count)

// SAFETY: the safety contract for `copy` must be upheld by the caller.
unsafe { copy(src, dst, count) }
}

/// Sets `count * size_of::<T>()` bytes of memory starting at `dst` to
Expand Down Expand Up @@ -2248,5 +2253,7 @@ pub unsafe fn write_bytes<T>(dst: *mut T, val: u8, count: usize) {
}

debug_assert!(is_aligned_and_not_null(dst), "attempt to write to unaligned or null pointer");
write_bytes(dst, val, count)

// SAFETY: the safety contract for `write_bytes` must be upheld by the caller.
unsafe { write_bytes(dst, val, count) }
}
5 changes: 3 additions & 2 deletions src/libcore/iter/adapters/fuse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,10 @@ where
{
unsafe fn get_unchecked(&mut self, i: usize) -> I::Item {
match self.iter {
Some(ref mut iter) => iter.get_unchecked(i),
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
Some(ref mut iter) => unsafe { iter.get_unchecked(i) },
// SAFETY: the caller asserts there is an item at `i`, so we're not exhausted.
None => intrinsics::unreachable(),
None => unsafe { intrinsics::unreachable() },
}
}

Expand Down
15 changes: 10 additions & 5 deletions src/libcore/iter/adapters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,8 @@ where
T: Copy,
{
unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
*self.it.get_unchecked(i)
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
unsafe { *self.it.get_unchecked(i) }
}

#[inline]
Expand Down Expand Up @@ -402,7 +403,8 @@ where
T: Clone,
{
default unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
self.it.get_unchecked(i).clone()
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
unsafe { self.it.get_unchecked(i) }.clone()
}

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

#[inline]
Expand Down Expand Up @@ -930,7 +933,8 @@ where
F: FnMut(I::Item) -> B,
{
unsafe fn get_unchecked(&mut self, i: usize) -> Self::Item {
(self.f)(self.iter.get_unchecked(i))
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
(self.f)(unsafe { self.iter.get_unchecked(i) })
}
#[inline]
fn may_have_side_effect() -> bool {
Expand Down Expand Up @@ -1392,7 +1396,8 @@ where
I: TrustedRandomAccess,
{
unsafe fn get_unchecked(&mut self, i: usize) -> (usize, I::Item) {
(self.count + i, self.iter.get_unchecked(i))
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
(self.count + i, unsafe { self.iter.get_unchecked(i) })
}

fn may_have_side_effect() -> bool {
Expand Down
3 changes: 2 additions & 1 deletion src/libcore/iter/adapters/zip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ where
B: TrustedRandomAccess,
{
unsafe fn get_unchecked(&mut self, i: usize) -> (A::Item, B::Item) {
(self.a.get_unchecked(i), self.b.get_unchecked(i))
// SAFETY: the caller must uphold the contract for `TrustedRandomAccess::get_unchecked`.
unsafe { (self.a.get_unchecked(i), self.b.get_unchecked(i)) }
}

fn may_have_side_effect() -> bool {
Expand Down
Loading

0 comments on commit 500634b

Please sign in to comment.