Skip to content

Commit be11c3d

Browse files
authored
Rollup merge of rust-lang#73622 - LeSeulArtichaut:unsafe-libcore, r=nikomatsakis
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
2 parents 6e57524 + 6a7a652 commit be11c3d

38 files changed

+877
-436
lines changed

src/libcore/alloc/global.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,12 @@ pub unsafe trait GlobalAlloc {
127127
#[stable(feature = "global_alloc", since = "1.28.0")]
128128
unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
129129
let size = layout.size();
130-
let ptr = self.alloc(layout);
130+
// SAFETY: the safety contract for `alloc` must be upheld by the caller.
131+
let ptr = unsafe { self.alloc(layout) };
131132
if !ptr.is_null() {
132-
ptr::write_bytes(ptr, 0, size);
133+
// SAFETY: as allocation succeeded, the region from `ptr`
134+
// of size `size` is guaranteed to be valid for writes.
135+
unsafe { ptr::write_bytes(ptr, 0, size) };
133136
}
134137
ptr
135138
}
@@ -187,11 +190,18 @@ pub unsafe trait GlobalAlloc {
187190
/// [`handle_alloc_error`]: ../../alloc/alloc/fn.handle_alloc_error.html
188191
#[stable(feature = "global_alloc", since = "1.28.0")]
189192
unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
190-
let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
191-
let new_ptr = self.alloc(new_layout);
193+
// SAFETY: the caller must ensure that the `new_size` does not overflow.
194+
// `layout.align()` comes from a `Layout` and is thus guaranteed to be valid.
195+
let new_layout = unsafe { Layout::from_size_align_unchecked(new_size, layout.align()) };
196+
// SAFETY: the caller must ensure that `new_layout` is greater than zero.
197+
let new_ptr = unsafe { self.alloc(new_layout) };
192198
if !new_ptr.is_null() {
193-
ptr::copy_nonoverlapping(ptr, new_ptr, cmp::min(layout.size(), new_size));
194-
self.dealloc(ptr, layout);
199+
// SAFETY: the previously allocated block cannot overlap the newly allocated block.
200+
// The safety contract for `dealloc` must be upheld by the caller.
201+
unsafe {
202+
ptr::copy_nonoverlapping(ptr, new_ptr, cmp::min(layout.size(), new_size));
203+
self.dealloc(ptr, layout);
204+
}
195205
}
196206
new_ptr
197207
}

src/libcore/alloc/layout.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,8 @@ impl Layout {
9090
#[rustc_const_stable(feature = "alloc_layout", since = "1.28.0")]
9191
#[inline]
9292
pub const unsafe fn from_size_align_unchecked(size: usize, align: usize) -> Self {
93-
Layout { size_: size, align_: NonZeroUsize::new_unchecked(align) }
93+
// SAFETY: the caller must ensure that `align` is greater than zero.
94+
Layout { size_: size, align_: unsafe { NonZeroUsize::new_unchecked(align) } }
9495
}
9596

9697
/// The minimum size in bytes for a memory block of this layout.

src/libcore/alloc/mod.rs

Lines changed: 45 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ impl AllocInit {
5454
#[inline]
5555
#[unstable(feature = "allocator_api", issue = "32838")]
5656
pub unsafe fn init(self, memory: MemoryBlock) {
57-
self.init_offset(memory, 0)
57+
// SAFETY: the safety contract for `init_offset` must be
58+
// upheld by the caller.
59+
unsafe { self.init_offset(memory, 0) }
5860
}
5961

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

284-
let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
289+
let new_layout =
290+
// SAFETY: the caller must ensure that the `new_size` does not overflow.
291+
// `layout.align()` comes from a `Layout` and is thus guaranteed to be valid for a Layout.
292+
// The caller must ensure that `new_size` is greater than zero.
293+
unsafe { Layout::from_size_align_unchecked(new_size, layout.align()) };
285294
let new_memory = self.alloc(new_layout, init)?;
286-
ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), size);
287-
self.dealloc(ptr, layout);
288-
Ok(new_memory)
295+
296+
// SAFETY: because `new_size` must be greater than or equal to `size`, both the old and new
297+
// memory allocation are valid for reads and writes for `size` bytes. Also, because the old
298+
// allocation wasn't yet deallocated, it cannot overlap `new_memory`. Thus, the call to
299+
// `copy_nonoverlapping` is safe.
300+
// The safety contract for `dealloc` must be upheld by the caller.
301+
unsafe {
302+
ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), size);
303+
self.dealloc(ptr, layout);
304+
Ok(new_memory)
305+
}
289306
}
290307
}
291308
}
@@ -356,11 +373,23 @@ pub unsafe trait AllocRef {
356373
return Ok(MemoryBlock { ptr, size });
357374
}
358375

359-
let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
376+
let new_layout =
377+
// SAFETY: the caller must ensure that the `new_size` does not overflow.
378+
// `layout.align()` comes from a `Layout` and is thus guaranteed to be valid for a Layout.
379+
// The caller must ensure that `new_size` is greater than zero.
380+
unsafe { Layout::from_size_align_unchecked(new_size, layout.align()) };
360381
let new_memory = self.alloc(new_layout, AllocInit::Uninitialized)?;
361-
ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), new_size);
362-
self.dealloc(ptr, layout);
363-
Ok(new_memory)
382+
383+
// SAFETY: because `new_size` must be lower than or equal to `size`, both the old and new
384+
// memory allocation are valid for reads and writes for `new_size` bytes. Also, because the
385+
// old allocation wasn't yet deallocated, it cannot overlap `new_memory`. Thus, the call to
386+
// `copy_nonoverlapping` is safe.
387+
// The safety contract for `dealloc` must be upheld by the caller.
388+
unsafe {
389+
ptr::copy_nonoverlapping(ptr.as_ptr(), new_memory.ptr.as_ptr(), new_size);
390+
self.dealloc(ptr, layout);
391+
Ok(new_memory)
392+
}
364393
}
365394
}
366395
}
@@ -386,7 +415,8 @@ where
386415

387416
#[inline]
388417
unsafe fn dealloc(&mut self, ptr: NonNull<u8>, layout: Layout) {
389-
(**self).dealloc(ptr, layout)
418+
// SAFETY: the safety contract must be upheld by the caller
419+
unsafe { (**self).dealloc(ptr, layout) }
390420
}
391421

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

404435
#[inline]
@@ -409,6 +440,7 @@ where
409440
new_size: usize,
410441
placement: ReallocPlacement,
411442
) -> Result<MemoryBlock, AllocErr> {
412-
(**self).shrink(ptr, layout, new_size, placement)
443+
// SAFETY: the safety contract must be upheld by the caller
444+
unsafe { (**self).shrink(ptr, layout, new_size, placement) }
413445
}
414446
}

src/libcore/cell.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,12 @@ impl<T: ?Sized> RefCell<T> {
10051005
#[inline]
10061006
pub unsafe fn try_borrow_unguarded(&self) -> Result<&T, BorrowError> {
10071007
if !is_writing(self.borrow.get()) {
1008-
Ok(&*self.value.get())
1008+
// SAFETY: We check that nobody is actively writing now, but it is
1009+
// the caller's responsibility to ensure that nobody writes until
1010+
// the returned reference is no longer in use.
1011+
// Also, `self.value.get()` refers to the value owned by `self`
1012+
// and is thus guaranteed to be valid for the lifetime of `self`.
1013+
Ok(unsafe { &*self.value.get() })
10091014
} else {
10101015
Err(BorrowError { _private: () })
10111016
}

src/libcore/char/convert.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@ pub fn from_u32(i: u32) -> Option<char> {
9999
#[inline]
100100
#[stable(feature = "char_from_unchecked", since = "1.5.0")]
101101
pub unsafe fn from_u32_unchecked(i: u32) -> char {
102-
if cfg!(debug_assertions) { char::from_u32(i).unwrap() } else { transmute(i) }
102+
// SAFETY: the caller must guarantee that `i` is a valid char value.
103+
if cfg!(debug_assertions) { char::from_u32(i).unwrap() } else { unsafe { transmute(i) } }
103104
}
104105

105106
#[stable(feature = "char_convert", since = "1.13.0")]

src/libcore/char/methods.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,8 @@ impl char {
183183
#[unstable(feature = "assoc_char_funcs", reason = "recently added", issue = "71763")]
184184
#[inline]
185185
pub unsafe fn from_u32_unchecked(i: u32) -> char {
186-
super::convert::from_u32_unchecked(i)
186+
// SAFETY: the safety contract must be upheld by the caller.
187+
unsafe { super::convert::from_u32_unchecked(i) }
187188
}
188189

189190
/// Converts a digit in the given radix to a `char`.

src/libcore/convert/num.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ macro_rules! impl_float_to_int {
2828
#[doc(hidden)]
2929
#[inline]
3030
unsafe fn to_int_unchecked(self) -> $Int {
31-
crate::intrinsics::float_to_int_unchecked(self)
31+
// SAFETY: the safety contract must be upheld by the caller.
32+
unsafe { crate::intrinsics::float_to_int_unchecked(self) }
3233
}
3334
}
3435
)+

src/libcore/ffi.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,8 @@ impl<'f> VaListImpl<'f> {
333333
/// Advance to the next arg.
334334
#[inline]
335335
pub unsafe fn arg<T: sealed_trait::VaArgSafe>(&mut self) -> T {
336-
va_arg(self)
336+
// SAFETY: the caller must uphold the safety contract for `va_arg`.
337+
unsafe { va_arg(self) }
337338
}
338339

339340
/// Copies the `va_list` at the current location.
@@ -343,7 +344,10 @@ impl<'f> VaListImpl<'f> {
343344
{
344345
let mut ap = self.clone();
345346
let ret = f(ap.as_va_list());
346-
va_end(&mut ap);
347+
// SAFETY: the caller must uphold the safety contract for `va_end`.
348+
unsafe {
349+
va_end(&mut ap);
350+
}
347351
ret
348352
}
349353
}

src/libcore/future/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,7 @@ where
8585
#[unstable(feature = "gen_future", issue = "50547")]
8686
#[inline]
8787
pub unsafe fn get_context<'a, 'b>(cx: ResumeTy) -> &'a mut Context<'b> {
88-
&mut *cx.0.as_ptr().cast()
88+
// SAFETY: the caller must guarantee that `cx.0` is a valid pointer
89+
// that fulfills all the requirements for a mutable reference.
90+
unsafe { &mut *cx.0.as_ptr().cast() }
8991
}

src/libcore/hash/sip.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,15 +130,19 @@ unsafe fn u8to64_le(buf: &[u8], start: usize, len: usize) -> u64 {
130130
let mut i = 0; // current byte index (from LSB) in the output u64
131131
let mut out = 0;
132132
if i + 3 < len {
133-
out = load_int_le!(buf, start + i, u32) as u64;
133+
// SAFETY: `i` cannot be greater than `len`, and the caller must guarantee
134+
// that the index start..start+len is in bounds.
135+
out = unsafe { load_int_le!(buf, start + i, u32) } as u64;
134136
i += 4;
135137
}
136138
if i + 1 < len {
137-
out |= (load_int_le!(buf, start + i, u16) as u64) << (i * 8);
139+
// SAFETY: same as above.
140+
out |= (unsafe { load_int_le!(buf, start + i, u16) } as u64) << (i * 8);
138141
i += 2
139142
}
140143
if i < len {
141-
out |= (*buf.get_unchecked(start + i) as u64) << (i * 8);
144+
// SAFETY: same as above.
145+
out |= (unsafe { *buf.get_unchecked(start + i) } as u64) << (i * 8);
142146
i += 1;
143147
}
144148
debug_assert_eq!(i, len);

0 commit comments

Comments
 (0)