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

FromZeros boxed slice method supports slice DSTs #1478

Merged
merged 1 commit into from
Sep 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,20 @@ impl<Src, Dst: ?Sized + TryFromBytes> TryReadError<Src, Dst> {
}
}

/// The error type of a failed allocation.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to document that this will, in the future, be aliased with core::alloc::AllocError. Otherwise, replacing this type with an alias will be a breaking change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

///
/// This type is intended to be deprecated in favor of the standard library's
/// [`AllocError`] type once it is stabilized. When that happens, this type will
/// be replaced by a type alias to the standard library type. We do not intend
/// to treat this as a breaking change; users who wish to avoid breakage should
/// avoid writing code which assumes that this is *not* such an alias. For
/// example, implementing the same trait for both types will result in an impl
/// conflict once this type is an alias.
///
/// [`AllocError`]: https://doc.rust-lang.org/alloc/alloc/struct.AllocError.html
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub struct AllocError;

#[cfg(test)]
mod tests {
use super::*;
Expand Down
2 changes: 1 addition & 1 deletion src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,7 @@ mod tests {
impl<T: FromBytes> TryFromBytesTestable for T {
fn with_passing_test_cases<F: Fn(Box<Self>)>(f: F) {
// Test with a zeroed value.
f(Self::new_box_zeroed());
f(Self::new_box_zeroed().unwrap());

let ffs = {
let mut t = Self::new_zeroed();
Expand Down
165 changes: 105 additions & 60 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2113,14 +2113,15 @@ pub unsafe trait FromZeros: TryFromBytes {
/// Note that `Box<Self>` can be converted to `Arc<Self>` and other
/// container types without reallocation.
///
/// # Panics
/// # Errors
///
/// Panics if allocation of `size_of::<Self>()` bytes fails.
/// Returns an error on allocation failure. Allocation failure is guaranteed
/// never to cause a panic or an abort.
#[must_use = "has no side effects (other than allocation)"]
#[cfg(any(feature = "alloc", test))]
#[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))]
#[inline]
fn new_box_zeroed() -> Box<Self>
fn new_box_zeroed() -> Result<Box<Self>, AllocError>
where
Self: Sized,
{
Expand All @@ -2146,22 +2147,18 @@ pub unsafe trait FromZeros: TryFromBytes {
// [2] Per https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.dangling:
//
// Creates a new `NonNull` that is dangling, but well-aligned.
unsafe {
return Box::from_raw(NonNull::dangling().as_ptr());
}
return Ok(unsafe { Box::from_raw(NonNull::dangling().as_ptr()) });
}

// TODO(#429): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
let ptr = unsafe { alloc::alloc::alloc_zeroed(layout).cast::<Self>() };
if ptr.is_null() {
alloc::alloc::handle_alloc_error(layout);
return Err(AllocError);
}
// TODO(#429): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
Box::from_raw(ptr)
}
Ok(unsafe { Box::from_raw(ptr) })
}

/// Creates a `Box<[Self]>` (a boxed slice) from zeroed bytes.
Expand All @@ -2181,22 +2178,24 @@ pub unsafe trait FromZeros: TryFromBytes {
/// actual information, but its `len()` property will report the correct
/// value.
///
/// # Panics
/// # Errors
///
/// * Panics if `size_of::<Self>() * len` overflows.
/// * Panics if allocation of `size_of::<Self>() * len` bytes fails.
/// Returns an error on allocation failure. Allocation failure is
/// guaranteed never to cause a panic or an abort.
#[must_use = "has no side effects (other than allocation)"]
#[cfg(feature = "alloc")]
#[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))]
#[inline]
fn new_box_slice_zeroed(len: usize) -> Box<[Self]>
fn new_box_zeroed_with_elems(count: usize) -> Result<Box<Self>, AllocError>
where
Self: Sized,
Self: KnownLayout<PointerMetadata = usize>,
{
let size = mem::size_of::<Self>()
.checked_mul(len)
.expect("mem::size_of::<Self>() * len overflows `usize`");
let align = mem::align_of::<Self>();
let size = match count.size_for_metadata(Self::LAYOUT) {
Some(size) => size,
None => return Err(AllocError),
};

let align = Self::LAYOUT.align.get();
// On stable Rust versions <= 1.64.0, `Layout::from_size_align` has a
// bug in which sufficiently-large allocations (those which, when
// rounded up to the alignment, overflow `isize`) are not rejected,
Expand All @@ -2205,32 +2204,74 @@ pub unsafe trait FromZeros: TryFromBytes {
// TODO(#67): Once our MSRV is > 1.64.0, remove this assertion.
#[allow(clippy::as_conversions)]
let max_alloc = (isize::MAX as usize).saturating_sub(align);
assert!(size <= max_alloc);
if size > max_alloc {
return Err(AllocError);
}

// TODO(https://github.com/rust-lang/rust/issues/55724): Use
// `Layout::repeat` once it's stabilized.
let layout =
Layout::from_size_align(size, align).expect("total allocation size overflows `isize`");
let layout = Layout::from_size_align(size, align).or(Err(AllocError))?;

let ptr = if layout.size() != 0 {
// TODO(#429): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
let ptr = unsafe { alloc::alloc::alloc_zeroed(layout).cast::<Self>() };
if ptr.is_null() {
alloc::alloc::handle_alloc_error(layout);
let ptr = unsafe { alloc::alloc::alloc_zeroed(layout) };
match NonNull::new(ptr) {
Some(ptr) => ptr,
None => return Err(AllocError),
}
ptr
} else {
let align = Self::LAYOUT.align.get();
// We use `transmute` instead of an `as` cast since Miri (with
// strict provenance enabled) notices and complains that an `as`
// cast creates a pointer with no provenance. Miri isn't smart
// enough to realize that we're only executing this branch when
// we're constructing a zero-sized `Box`, which doesn't require
// provenance.
//
// SAFETY: any initialized bit sequence is a bit-valid `*mut u8`.
// All bits of a `usize` are initialized.
#[allow(clippy::useless_transmute)]
let dangling = unsafe { mem::transmute::<usize, *mut u8>(align) };
// SAFETY: `dangling` is constructed from `Self::LAYOUT.align`,
// which is a `NonZeroUsize`, which is guaranteed to be non-zero.
//
// `Box<[T]>` does not allocate when `T` is zero-sized or when `len`
// is zero, but it does require a non-null dangling pointer for its
// allocation.
NonNull::<Self>::dangling().as_ptr()
//
// TODO(https://github.com/rust-lang/rust/issues/95228): Use
// `std::ptr::without_provenance` once it's stable. That may
// optimize better. As written, Rust may assume that this consumes
// "exposed" provenance, and thus Rust may have to assume that this
// may consume provenance from any pointer whose provenance has been
// exposed.
#[allow(fuzzy_provenance_casts)]
unsafe {
NonNull::new_unchecked(dangling)
}
};

// TODO(#429): Add a "SAFETY" comment and remove this `allow`.
let ptr = Self::raw_from_ptr_len(ptr, count);

// TODO(#429): Add a "SAFETY" comment and remove this `allow`. Make sure
// to include a justification that `ptr.as_ptr()` is validly-aligned in
// the ZST case (in which we manually construct a dangling pointer).
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
Box::from_raw(slice::from_raw_parts_mut(ptr, len))
}
Ok(unsafe { Box::from_raw(ptr.as_ptr()) })
}

#[deprecated(since = "0.8.0", note = "renamed to `FromZeros::new_box_zeroed_with_elems`")]
#[doc(hidden)]
#[cfg(feature = "alloc")]
#[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))]
#[must_use = "has no side effects (other than allocation)"]
#[inline(always)]
fn new_box_slice_zeroed(len: usize) -> Result<Box<[Self]>, AllocError>
where
Self: Sized,
{
<[Self]>::new_box_zeroed_with_elems(len)
}

/// Creates a `Vec<Self>` from zeroed bytes.
Expand All @@ -2249,19 +2290,19 @@ pub unsafe trait FromZeros: TryFromBytes {
/// actual information, but its `len()` property will report the correct
/// value.
///
/// # Panics
/// # Errors
///
/// * Panics if `size_of::<Self>() * len` overflows.
/// * Panics if allocation of `size_of::<Self>() * len` bytes fails.
/// Returns an error on allocation failure. Allocation failure is
/// guaranteed never to cause a panic or an abort.
#[must_use = "has no side effects (other than allocation)"]
#[cfg(feature = "alloc")]
#[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))]
#[inline(always)]
fn new_vec_zeroed(len: usize) -> Vec<Self>
fn new_vec_zeroed(len: usize) -> Result<Vec<Self>, AllocError>
where
Self: Sized,
{
Self::new_box_slice_zeroed(len).into()
<[Self]>::new_box_zeroed_with_elems(len).map(Into::into)
}
}

Expand Down Expand Up @@ -4972,7 +5013,7 @@ mod alloc_support {

#[test]
fn test_new_box_zeroed() {
assert_eq!(*u64::new_box_zeroed(), 0);
assert_eq!(u64::new_box_zeroed(), Ok(Box::new(0)));
}

#[test]
Expand All @@ -4986,28 +5027,28 @@ mod alloc_support {
// when running under Miri.
#[allow(clippy::unit_cmp)]
{
assert_eq!(*<()>::new_box_zeroed(), ());
assert_eq!(<()>::new_box_zeroed(), Ok(Box::new(())));
}
}

#[test]
fn test_new_box_slice_zeroed() {
let mut s: Box<[u64]> = u64::new_box_slice_zeroed(3);
fn test_new_box_zeroed_with_elems() {
let mut s: Box<[u64]> = <[u64]>::new_box_zeroed_with_elems(3).unwrap();
assert_eq!(s.len(), 3);
assert_eq!(&*s, &[0, 0, 0]);
s[1] = 3;
assert_eq!(&*s, &[0, 3, 0]);
}

#[test]
fn test_new_box_slice_zeroed_empty() {
let s: Box<[u64]> = u64::new_box_slice_zeroed(0);
fn test_new_box_zeroed_with_elems_empty() {
let s: Box<[u64]> = <[u64]>::new_box_zeroed_with_elems(0).unwrap();
assert_eq!(s.len(), 0);
}

#[test]
fn test_new_box_slice_zeroed_zst() {
let mut s: Box<[()]> = <()>::new_box_slice_zeroed(3);
fn test_new_box_zeroed_with_elems_zst() {
let mut s: Box<[()]> = <[()]>::new_box_zeroed_with_elems(3).unwrap();
assert_eq!(s.len(), 3);
assert!(s.get(10).is_none());
// This test exists in order to exercise unsafe code, especially
Expand All @@ -5020,22 +5061,20 @@ mod alloc_support {
}

#[test]
fn test_new_box_slice_zeroed_zst_empty() {
let s: Box<[()]> = <()>::new_box_slice_zeroed(0);
fn test_new_box_zeroed_with_elems_zst_empty() {
let s: Box<[()]> = <[()]>::new_box_zeroed_with_elems(0).unwrap();
assert_eq!(s.len(), 0);
}

#[test]
#[should_panic(expected = "mem::size_of::<Self>() * len overflows `usize`")]
fn test_new_box_slice_zeroed_panics_mul_overflow() {
let _ = u16::new_box_slice_zeroed(usize::MAX);
}
fn new_box_zeroed_with_elems_errors() {
assert_eq!(<[u16]>::new_box_zeroed_with_elems(usize::MAX), Err(AllocError));

#[test]
#[should_panic(expected = "assertion failed: size <= max_alloc")]
fn test_new_box_slice_zeroed_panics_isize_overflow() {
let max = usize::try_from(isize::MAX).unwrap();
let _ = u16::new_box_slice_zeroed((max / mem::size_of::<u16>()) + 1);
assert_eq!(
<[u16]>::new_box_zeroed_with_elems((max / mem::size_of::<u16>()) + 1),
Err(AllocError)
);
}
}
}
Expand Down Expand Up @@ -5525,14 +5564,20 @@ mod tests {

#[cfg(feature = "alloc")]
{
assert_eq!(bool::new_box_zeroed(), Box::new(false));
assert_eq!(char::new_box_zeroed(), Box::new('\0'));
assert_eq!(bool::new_box_zeroed(), Ok(Box::new(false)));
assert_eq!(char::new_box_zeroed(), Ok(Box::new('\0')));

assert_eq!(bool::new_box_slice_zeroed(3).as_ref(), [false, false, false]);
assert_eq!(char::new_box_slice_zeroed(3).as_ref(), ['\0', '\0', '\0']);
assert_eq!(
<[bool]>::new_box_zeroed_with_elems(3).unwrap().as_ref(),
[false, false, false]
);
assert_eq!(
<[char]>::new_box_zeroed_with_elems(3).unwrap().as_ref(),
['\0', '\0', '\0']
);

assert_eq!(bool::new_vec_zeroed(3).as_ref(), [false, false, false]);
assert_eq!(char::new_vec_zeroed(3).as_ref(), ['\0', '\0', '\0']);
assert_eq!(bool::new_vec_zeroed(3).unwrap().as_ref(), [false, false, false]);
assert_eq!(char::new_vec_zeroed(3).unwrap().as_ref(), ['\0', '\0', '\0']);
}

let mut string = "hello".to_string();
Expand Down
Loading