Skip to content

Commit

Permalink
Rollup merge of rust-lang#69026 - TimDiekmann:common-usage, r=Amanieu
Browse files Browse the repository at this point in the history 
Remove common usage pattern from `AllocRef`

This removes the common usage patterns from `AllocRef`:
- `alloc_one`
- `dealloc_one`
- `alloc_array`
- `realloc_array`
- `dealloc_array`

Actually, they add nothing to `AllocRef` except a [convenience wrapper around `Layout` and other methods in this trait](https://doc.rust-lang.org/1.41.0/src/core/alloc.rs.html#1076-1240) but have a major flaw: The documentation of `AllocRefs` notes, that

> some higher-level allocation methods (`alloc_one`, `alloc_array`) are well-defined on zero-sized types and can optionally support them: it is left up to the implementor whether to return `Err`, or to return `Ok` with some pointer.

With the current API, `GlobalAlloc` does not have those methods, so they cannot be overridden for `liballoc::Global`, which means that even if the global allocator would support zero-sized allocations, `alloc_one`, `alloc_array`, and `realloc_array` for `liballoc::Global` will error, while calling `alloc` with a zeroed-size `Layout` could succeed. Even worse: allocating with `alloc` and deallocating with `dealloc_{one,array}` could end up with not calling `dealloc` at all!

For the full discussion please see rust-lang/wg-allocators#18

r? @Amanieu
  • Loading branch information
@JohnTitor
JohnTitor authored Feb 12, 2020
2 parents 9b51a52 + 25de80a commit 9bc003d
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 223 deletions.
13 changes: 7 additions & 6 deletions src/liballoc/raw_vec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ impl<T, A: AllocRef> RawVec<T, A> {
// 0, getting to here necessarily means the `RawVec` is overfull.
assert!(elem_size != 0, "capacity overflow");

let (new_cap, uniq) = match self.current_layout() {
let (new_cap, ptr) = match self.current_layout() {
Some(cur) => {
// Since we guarantee that we never allocate more than
// `isize::MAX` bytes, `elem_size * self.cap <= isize::MAX` as
Expand All @@ -297,7 +297,7 @@ impl<T, A: AllocRef> RawVec<T, A> {
alloc_guard(new_size).unwrap_or_else(|_| capacity_overflow());
let ptr_res = self.a.realloc(NonNull::from(self.ptr).cast(), cur, new_size);
match ptr_res {
Ok(ptr) => (new_cap, ptr.cast().into()),
Ok(ptr) => (new_cap, ptr),
Err(_) => handle_alloc_error(Layout::from_size_align_unchecked(
new_size,
cur.align(),
Expand All @@ -308,13 +308,14 @@ impl<T, A: AllocRef> RawVec<T, A> {
// Skip to 4 because tiny `Vec`'s are dumb; but not if that
// would cause overflow.
let new_cap = if elem_size > (!0) / 8 { 1 } else { 4 };
match self.a.alloc_array::<T>(new_cap) {
Ok(ptr) => (new_cap, ptr.into()),
Err(_) => handle_alloc_error(Layout::array::<T>(new_cap).unwrap()),
let layout = Layout::array::<T>(new_cap).unwrap();
match self.a.alloc(layout) {
Ok(ptr) => (new_cap, ptr),
Err(_) => handle_alloc_error(layout),
}
}
};
self.ptr = uniq;
self.ptr = ptr.cast().into();
self.cap = new_cap;
}
}
Expand Down
201 changes: 2 additions & 199 deletions src/libcore/alloc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -593,9 +593,8 @@ pub unsafe trait GlobalAlloc {
///
/// * the starting address for that memory block was previously
/// returned by a previous call to an allocation method (`alloc`,
/// `alloc_zeroed`, `alloc_excess`, `alloc_one`, `alloc_array`) or
/// reallocation method (`realloc`, `realloc_excess`, or
/// `realloc_array`), and
/// `alloc_zeroed`, `alloc_excess`) or reallocation method
/// (`realloc`, `realloc_excess`), and
///
/// * the memory block has not been subsequently deallocated, where
/// blocks are deallocated either by being passed to a deallocation
Expand All @@ -606,11 +605,6 @@ pub unsafe trait GlobalAlloc {
/// methods in the `AllocRef` trait state that allocation requests
/// must be non-zero size, or else undefined behavior can result.
///
/// * However, some higher-level allocation methods (`alloc_one`,
/// `alloc_array`) are well-defined on zero-sized types and can
/// optionally support them: it is left up to the implementor
/// whether to return `Err`, or to return `Ok` with some pointer.
///
/// * If an `AllocRef` implementation chooses to return `Ok` in this
/// case (i.e., the pointer denotes a zero-sized inaccessible block)
/// then that returned pointer must be considered "currently
Expand Down Expand Up @@ -1035,195 +1029,4 @@ pub unsafe trait AllocRef {
// new_layout.size() <= layout.size() [required by this method]
if l <= new_size { Ok(()) } else { Err(CannotReallocInPlace) }
}

// == COMMON USAGE PATTERNS ==
// alloc_one, dealloc_one, alloc_array, realloc_array. dealloc_array

/// Allocates a block suitable for holding an instance of `T`.
///
/// Captures a common usage pattern for allocators.
///
/// The returned block is suitable for passing to the
/// `realloc`/`dealloc` methods of this allocator.
///
/// Note to implementors: If this returns `Ok(ptr)`, then `ptr`
/// must be considered "currently allocated" and must be
/// acceptable input to methods such as `realloc` or `dealloc`,
/// *even if* `T` is a zero-sized type. In other words, if your
/// `AllocRef` implementation overrides this method in a manner
/// that can return a zero-sized `ptr`, then all reallocation and
/// deallocation methods need to be similarly overridden to accept
/// such values as input.
///
/// # Errors
///
/// Returning `Err` indicates that either memory is exhausted or
/// `T` does not meet allocator's size or alignment constraints.
///
/// For zero-sized `T`, may return either of `Ok` or `Err`, but
/// will *not* yield undefined behavior.
///
/// Clients wishing to abort computation in response to an
/// allocation error are encouraged to call the [`handle_alloc_error`] function,
/// rather than directly invoking `panic!` or similar.
///
/// [`handle_alloc_error`]: ../../alloc/alloc/fn.handle_alloc_error.html
fn alloc_one<T>(&mut self) -> Result<NonNull<T>, AllocErr>
where
Self: Sized,
{
let k = Layout::new::<T>();
if k.size() > 0 { unsafe { self.alloc(k).map(|p| p.cast()) } } else { Err(AllocErr) }
}

/// Deallocates a block suitable for holding an instance of `T`.
///
/// The given block must have been produced by this allocator,
/// and must be suitable for storing a `T` (in terms of alignment
/// as well as minimum and maximum size); otherwise yields
/// undefined behavior.
///
/// Captures a common usage pattern for allocators.
///
/// # Safety
///
/// This function is unsafe because undefined behavior can result
/// if the caller does not ensure both:
///
/// * `ptr` must denote a block of memory currently allocated via this allocator
///
/// * the layout of `T` must *fit* that block of memory.
unsafe fn dealloc_one<T>(&mut self, ptr: NonNull<T>)
where
Self: Sized,
{
let k = Layout::new::<T>();
if k.size() > 0 {
self.dealloc(ptr.cast(), k);
}
}

/// Allocates a block suitable for holding `n` instances of `T`.
///
/// Captures a common usage pattern for allocators.
///
/// The returned block is suitable for passing to the
/// `realloc`/`dealloc` methods of this allocator.
///
/// Note to implementors: If this returns `Ok(ptr)`, then `ptr`
/// must be considered "currently allocated" and must be
/// acceptable input to methods such as `realloc` or `dealloc`,
/// *even if* `T` is a zero-sized type. In other words, if your
/// `AllocRef` implementation overrides this method in a manner
/// that can return a zero-sized `ptr`, then all reallocation and
/// deallocation methods need to be similarly overridden to accept
/// such values as input.
///
/// # Errors
///
/// Returning `Err` indicates that either memory is exhausted or
/// `[T; n]` does not meet allocator's size or alignment
/// constraints.
///
/// For zero-sized `T` or `n == 0`, may return either of `Ok` or
/// `Err`, but will *not* yield undefined behavior.
///
/// Always returns `Err` on arithmetic overflow.
///
/// Clients wishing to abort computation in response to an
/// allocation error are encouraged to call the [`handle_alloc_error`] function,
/// rather than directly invoking `panic!` or similar.
///
/// [`handle_alloc_error`]: ../../alloc/alloc/fn.handle_alloc_error.html
fn alloc_array<T>(&mut self, n: usize) -> Result<NonNull<T>, AllocErr>
where
Self: Sized,
{
match Layout::array::<T>(n) {
Ok(layout) if layout.size() > 0 => unsafe { self.alloc(layout).map(|p| p.cast()) },
_ => Err(AllocErr),
}
}

/// Reallocates a block previously suitable for holding `n_old`
/// instances of `T`, returning a block suitable for holding
/// `n_new` instances of `T`.
///
/// Captures a common usage pattern for allocators.
///
/// The returned block is suitable for passing to the
/// `realloc`/`dealloc` methods of this allocator.
///
/// # Safety
///
/// This function is unsafe because undefined behavior can result
/// if the caller does not ensure all of the following:
///
/// * `ptr` must be currently allocated via this allocator,
///
/// * the layout of `[T; n_old]` must *fit* that block of memory.
///
/// # Errors
///
/// Returning `Err` indicates that either memory is exhausted or
/// `[T; n_new]` does not meet allocator's size or alignment
/// constraints.
///
/// For zero-sized `T` or `n_new == 0`, may return either of `Ok` or
/// `Err`, but will *not* yield undefined behavior.
///
/// Always returns `Err` on arithmetic overflow.
///
/// Clients wishing to abort computation in response to a
/// reallocation error are encouraged to call the [`handle_alloc_error`] function,
/// rather than directly invoking `panic!` or similar.
///
/// [`handle_alloc_error`]: ../../alloc/alloc/fn.handle_alloc_error.html
unsafe fn realloc_array<T>(
&mut self,
ptr: NonNull<T>,
n_old: usize,
n_new: usize,
) -> Result<NonNull<T>, AllocErr>
where
Self: Sized,
{
match (Layout::array::<T>(n_old), Layout::array::<T>(n_new)) {
(Ok(k_old), Ok(k_new)) if k_old.size() > 0 && k_new.size() > 0 => {
debug_assert!(k_old.align() == k_new.align());
self.realloc(ptr.cast(), k_old, k_new.size()).map(NonNull::cast)
}
_ => Err(AllocErr),
}
}

/// Deallocates a block suitable for holding `n` instances of `T`.
///
/// Captures a common usage pattern for allocators.
///
/// # Safety
///
/// This function is unsafe because undefined behavior can result
/// if the caller does not ensure both:
///
/// * `ptr` must denote a block of memory currently allocated via this allocator
///
/// * the layout of `[T; n]` must *fit* that block of memory.
///
/// # Errors
///
/// Returning `Err` indicates that either `[T; n]` or the given
/// memory block does not meet allocator's size or alignment
/// constraints.
///
/// Always returns `Err` on arithmetic overflow.
unsafe fn dealloc_array<T>(&mut self, ptr: NonNull<T>, n: usize) -> Result<(), AllocErr>
where
Self: Sized,
{
match Layout::array::<T>(n) {
Ok(k) if k.size() > 0 => Ok(self.dealloc(ptr.cast(), k)),
_ => Err(AllocErr),
}
}
}
18 changes: 0 additions & 18 deletions src/test/ui/allocator-alloc-one.rs
View file

This file was deleted.

0 comments on commit 9bc003d

Please sign in to comment.