Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul of the AllocRef trait to match allocator-wg's latest consens; Take 2 #70362

Merged
merged 23 commits into from
Apr 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
56cbf2f
Overhaul of the `AllocRef` trait to match allocator-wg's latest consens
TimDiekmann Mar 24, 2020
2526acc
Fix issues from review and unsoundness of `RawVec::into_box`
TimDiekmann Mar 26, 2020
2f215b6
Use `NonNull` instead of `Unique` in `MemoryBlock`
TimDiekmann Mar 25, 2020
d9d35cc
Add comment to `AllocRef` implementation for `System`
TimDiekmann Mar 25, 2020
c1fa023
Fix ZST handling for `RawVec`
TimDiekmann Mar 25, 2020
42a8547
Fix comment in `RawVec::into_box()`
TimDiekmann Mar 25, 2020
ba26a9e
Fix assertion in `shrink` to use `capacity()` instead
TimDiekmann Mar 25, 2020
aae3c52
Remove the note on the internal capacity field in `RawVec`
TimDiekmann Mar 25, 2020
ad7de67
Refine docs for `RawVec::from_raw_parts(_in)`
TimDiekmann Mar 25, 2020
b02e53f
Remove check for ZST in `RawVec::needs_to_grow`
TimDiekmann Mar 25, 2020
cbbdca0
Fix wording in `RawVec::from_raw_parts(_in)`
TimDiekmann Mar 25, 2020
fed3d6e
Fix safety section of `RawVec::into_box`
TimDiekmann Mar 25, 2020
bfbdb5f
Remove unused import from libcore/alloc
TimDiekmann Mar 25, 2020
03b055b
Remove alignment from `MemoryBlock`
TimDiekmann Mar 25, 2020
717e0c7
Apply suggestions from code review
TimDiekmann Mar 26, 2020
db15fe6
Mark `Layout::dangling` inline
TimDiekmann Mar 26, 2020
bf6a46d
Make fields in `MemoryBlock` public
TimDiekmann Mar 28, 2020
3ade8ae
Implement `init` and `init_offset` on `AllocInit` and mark it unsafe
TimDiekmann Mar 29, 2020
4309f6d
Minor doc fixes in `AllocInit::init`
TimDiekmann Mar 29, 2020
d241db2
Fix links for `AllocInit` methods
TimDiekmann Mar 29, 2020
fcd7092
Revert "Fix links for `AllocInit` methods"
TimDiekmann Mar 29, 2020
c49f280
Fix links for `AllocInit` methods
TimDiekmann Mar 29, 2020
89ed59d
Add missing allocation guard in `RawVec::grow`
TimDiekmann Apr 1, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 76 additions & 36 deletions src/liballoc/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#![stable(feature = "alloc_module", since = "1.28.0")]

use core::intrinsics::{min_align_of_val, size_of_val};
use core::intrinsics::{self, min_align_of_val, size_of_val};
use core::ptr::{NonNull, Unique};
use core::usize;

Expand Down Expand Up @@ -165,11 +165,19 @@ pub unsafe fn alloc_zeroed(layout: Layout) -> *mut u8 {
#[unstable(feature = "allocator_api", issue = "32838")]
unsafe impl AllocRef for Global {
#[inline]
fn alloc(&mut self, layout: Layout) -> Result<(NonNull<u8>, usize), AllocErr> {
if layout.size() == 0 {
Ok((layout.dangling(), 0))
} else {
unsafe { NonNull::new(alloc(layout)).ok_or(AllocErr).map(|p| (p, layout.size())) }
fn alloc(&mut self, layout: Layout, init: AllocInit) -> Result<MemoryBlock, AllocErr> {
unsafe {
let size = layout.size();
if size == 0 {
Ok(MemoryBlock { ptr: layout.dangling(), size: 0 })
} else {
let raw_ptr = match init {
AllocInit::Uninitialized => alloc(layout),
AllocInit::Zeroed => alloc_zeroed(layout),
};
let ptr = NonNull::new(raw_ptr).ok_or(AllocErr)?;
Ok(MemoryBlock { ptr, size })
}
}
}

Expand All @@ -181,32 +189,71 @@ unsafe impl AllocRef for Global {
}

#[inline]
unsafe fn realloc(
unsafe fn grow(
&mut self,
ptr: NonNull<u8>,
layout: Layout,
new_size: usize,
) -> Result<(NonNull<u8>, usize), AllocErr> {
match (layout.size(), new_size) {
(0, 0) => Ok((layout.dangling(), 0)),
(0, _) => self.alloc(Layout::from_size_align_unchecked(new_size, layout.align())),
(_, 0) => {
self.dealloc(ptr, layout);
Ok((layout.dangling(), 0))
placement: ReallocPlacement,
init: AllocInit,
) -> Result<MemoryBlock, AllocErr> {
let size = layout.size();
debug_assert!(
new_size >= size,
"`new_size` must be greater than or equal to `memory.size()`"
);

if size == new_size {
return Ok(MemoryBlock { ptr, size });
}

match placement {
ReallocPlacement::InPlace => Err(AllocErr),
ReallocPlacement::MayMove if layout.size() == 0 => {
let new_layout = Layout::from_size_align_unchecked(new_size, layout.align());
self.alloc(new_layout, init)
}
ReallocPlacement::MayMove => {
// `realloc` probably checks for `new_size > size` or something similar.
intrinsics::assume(new_size > size);
let ptr = realloc(ptr.as_ptr(), layout, new_size);
let memory =
MemoryBlock { ptr: NonNull::new(ptr).ok_or(AllocErr)?, size: new_size };
init.init_offset(memory, size);
Ok(memory)
}
(_, _) => NonNull::new(realloc(ptr.as_ptr(), layout, new_size))
.ok_or(AllocErr)
.map(|p| (p, new_size)),
}
}

#[inline]
fn alloc_zeroed(&mut self, layout: Layout) -> Result<(NonNull<u8>, usize), AllocErr> {
if layout.size() == 0 {
Ok((layout.dangling(), 0))
} else {
unsafe {
NonNull::new(alloc_zeroed(layout)).ok_or(AllocErr).map(|p| (p, layout.size()))
unsafe fn shrink(
&mut self,
ptr: NonNull<u8>,
layout: Layout,
new_size: usize,
placement: ReallocPlacement,
) -> Result<MemoryBlock, AllocErr> {
let size = layout.size();
debug_assert!(
new_size <= size,
"`new_size` must be smaller than or equal to `memory.size()`"
);

if size == new_size {
return Ok(MemoryBlock { ptr, size });
}

match placement {
ReallocPlacement::InPlace => Err(AllocErr),
ReallocPlacement::MayMove if new_size == 0 => {
self.dealloc(ptr, layout);
Ok(MemoryBlock { ptr: layout.dangling(), size: 0 })
}
ReallocPlacement::MayMove => {
// `realloc` probably checks for `new_size < size` or something similar.
intrinsics::assume(new_size < size);
let ptr = realloc(ptr.as_ptr(), layout, new_size);
Ok(MemoryBlock { ptr: NonNull::new(ptr).ok_or(AllocErr)?, size: new_size })
}
}
}
Expand All @@ -218,14 +265,10 @@ unsafe impl AllocRef for Global {
#[lang = "exchange_malloc"]
#[inline]
unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 {
if size == 0 {
align as *mut u8
} else {
let layout = Layout::from_size_align_unchecked(size, align);
match Global.alloc(layout) {
Ok((ptr, _)) => ptr.as_ptr(),
Err(_) => handle_alloc_error(layout),
}
let layout = Layout::from_size_align_unchecked(size, align);
match Global.alloc(layout, AllocInit::Uninitialized) {
Ok(memory) => memory.ptr.as_ptr(),
Err(_) => handle_alloc_error(layout),
}
}

Expand All @@ -239,11 +282,8 @@ unsafe fn exchange_malloc(size: usize, align: usize) -> *mut u8 {
pub(crate) unsafe fn box_free<T: ?Sized>(ptr: Unique<T>) {
let size = size_of_val(ptr.as_ref());
let align = min_align_of_val(ptr.as_ref());
// We do not allocate for Box<T> when T is ZST, so deallocation is also not necessary.
if size != 0 {
let layout = Layout::from_size_align_unchecked(size, align);
Global.dealloc(ptr.cast().into(), layout);
}
let layout = Layout::from_size_align_unchecked(size, align);
Global.dealloc(ptr.cast().into(), layout)
}

/// Abort on memory allocation error or failure.
Expand Down
9 changes: 5 additions & 4 deletions src/liballoc/alloc/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,17 @@ use test::Bencher;
fn allocate_zeroed() {
unsafe {
let layout = Layout::from_size_align(1024, 1).unwrap();
let (ptr, _) =
Global.alloc_zeroed(layout.clone()).unwrap_or_else(|_| handle_alloc_error(layout));
let memory = Global
.alloc(layout.clone(), AllocInit::Zeroed)
.unwrap_or_else(|_| handle_alloc_error(layout));

let mut i = ptr.cast::<u8>().as_ptr();
let mut i = memory.ptr.cast::<u8>().as_ptr();
let end = i.add(layout.size());
while i < end {
assert_eq!(*i, 0);
i = i.offset(1);
}
Global.dealloc(ptr, layout);
Global.dealloc(memory.ptr, layout);
}
}

Expand Down
41 changes: 16 additions & 25 deletions src/liballoc/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,9 @@ use core::ops::{
};
use core::pin::Pin;
use core::ptr::{self, NonNull, Unique};
use core::slice;
use core::task::{Context, Poll};

use crate::alloc::{self, AllocRef, Global};
use crate::alloc::{self, AllocInit, AllocRef, Global};
use crate::raw_vec::RawVec;
use crate::str::from_boxed_utf8_unchecked;
use crate::vec::Vec;
Expand Down Expand Up @@ -196,14 +195,12 @@ impl<T> Box<T> {
#[unstable(feature = "new_uninit", issue = "63291")]
pub fn new_uninit() -> Box<mem::MaybeUninit<T>> {
let layout = alloc::Layout::new::<mem::MaybeUninit<T>>();
unsafe {
let ptr = if layout.size() == 0 {
NonNull::dangling()
} else {
Global.alloc(layout).unwrap_or_else(|_| alloc::handle_alloc_error(layout)).0.cast()
};
Box::from_raw(ptr.as_ptr())
}
let ptr = Global
.alloc(layout, AllocInit::Uninitialized)
.unwrap_or_else(|_| alloc::handle_alloc_error(layout))
.ptr
.cast();
unsafe { Box::from_raw(ptr.as_ptr()) }
}

/// Constructs a new `Box` with uninitialized contents, with the memory
Expand All @@ -226,11 +223,13 @@ impl<T> Box<T> {
/// [zeroed]: ../../std/mem/union.MaybeUninit.html#method.zeroed
#[unstable(feature = "new_uninit", issue = "63291")]
pub fn new_zeroed() -> Box<mem::MaybeUninit<T>> {
unsafe {
let mut uninit = Self::new_uninit();
ptr::write_bytes::<T>(uninit.as_mut_ptr(), 0, 1);
uninit
}
let layout = alloc::Layout::new::<mem::MaybeUninit<T>>();
let ptr = Global
.alloc(layout, AllocInit::Zeroed)
.unwrap_or_else(|_| alloc::handle_alloc_error(layout))
.ptr
.cast();
unsafe { Box::from_raw(ptr.as_ptr()) }
}

/// Constructs a new `Pin<Box<T>>`. If `T` does not implement `Unpin`, then
Expand Down Expand Up @@ -265,15 +264,7 @@ impl<T> Box<[T]> {
/// ```
#[unstable(feature = "new_uninit", issue = "63291")]
pub fn new_uninit_slice(len: usize) -> Box<[mem::MaybeUninit<T>]> {
let layout = alloc::Layout::array::<mem::MaybeUninit<T>>(len).unwrap();
unsafe {
let ptr = if layout.size() == 0 {
NonNull::dangling()
} else {
Global.alloc(layout).unwrap_or_else(|_| alloc::handle_alloc_error(layout)).0.cast()
};
Box::from_raw(slice::from_raw_parts_mut(ptr.as_ptr(), len))
}
unsafe { RawVec::with_capacity(len).into_box(len) }
}
}

Expand Down Expand Up @@ -778,7 +769,7 @@ impl<T: Copy> From<&[T]> for Box<[T]> {
let buf = RawVec::with_capacity(len);
unsafe {
ptr::copy_nonoverlapping(slice.as_ptr(), buf.ptr(), len);
buf.into_box()
buf.into_box(slice.len()).assume_init()
}
}
}
Expand Down
9 changes: 5 additions & 4 deletions src/liballoc/collections/btree/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1142,7 +1142,7 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::

(*left_node.as_leaf_mut()).len += right_len as u16 + 1;

if self.node.height > 1 {
let layout = if self.node.height > 1 {
ptr::copy_nonoverlapping(
right_node.cast_unchecked().as_internal().edges.as_ptr(),
left_node
Expand All @@ -1159,10 +1159,11 @@ impl<'a, K, V> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::
.correct_parent_link();
}

Global.dealloc(right_node.node.cast(), Layout::new::<InternalNode<K, V>>());
Layout::new::<InternalNode<K, V>>()
} else {
Global.dealloc(right_node.node.cast(), Layout::new::<LeafNode<K, V>>());
}
Layout::new::<LeafNode<K, V>>()
};
Global.dealloc(right_node.node.cast(), layout);

Handle::new_edge(self.node, self.idx)
}
Expand Down
1 change: 1 addition & 0 deletions src/liballoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
#![feature(lang_items)]
#![feature(libc)]
#![cfg_attr(not(bootstrap), feature(negative_impls))]
#![feature(new_uninit)]
#![feature(nll)]
#![feature(optin_builtin_traits)]
#![feature(pattern)]
Expand Down
Loading