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

Make too-large byte sizes a validity error in Layout #99134

Closed
Closed
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
13 changes: 8 additions & 5 deletions library/core/src/alloc/layout.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::cmp;
use crate::fmt;
use crate::mem::{self, ValidAlign};
use crate::mem::{self, ValidAlign, ValidSize};
use crate::ptr::NonNull;

// While this function is used in one place and its implementation
Expand Down Expand Up @@ -30,7 +30,7 @@ const fn size_align<T>() -> (usize, usize) {
#[lang = "alloc_layout"]
pub struct Layout {
// size of the requested block of memory, measured in bytes.
size: usize,
size: ValidSize,

// alignment of the requested block of memory, measured in bytes.
// we ensure that this is always a power-of-two, because API's
Expand Down Expand Up @@ -96,8 +96,11 @@ impl Layout {
#[must_use]
#[inline]
pub const unsafe fn from_size_align_unchecked(size: usize, align: usize) -> Self {
// SAFETY: the caller must ensure that `align` is a power of two.
Layout { size, align: unsafe { ValidAlign::new_unchecked(align) } }
// SAFETY: the caller must ensure that `align` is a power of two and
// that `size` is no more than `isize::MAX`.
unsafe {
Layout { size: ValidSize::new_unchecked(size), align: ValidAlign::new_unchecked(align) }
}
}

/// The minimum size in bytes for a memory block of this layout.
Expand All @@ -106,7 +109,7 @@ impl Layout {
#[must_use]
#[inline]
pub const fn size(&self) -> usize {
self.size
self.size.as_usize()
}

/// The minimum byte alignment for a memory block of this layout.
Expand Down
3 changes: 3 additions & 0 deletions library/core/src/mem/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ mod valid_align;
// alignment as a parameter, such as `Layout::padding_needed_for`.
pub(crate) use valid_align::ValidAlign;

mod valid_size;
pub(crate) use valid_size::ValidSize;

#[stable(feature = "rust1", since = "1.0.0")]
#[doc(inline)]
pub use crate::intrinsics::transmute;
Expand Down
8 changes: 5 additions & 3 deletions library/core/src/mem/valid_align.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::convert::TryFrom;
use crate::intrinsics::assert_unsafe_precondition;
use crate::num::NonZeroUsize;
use crate::{cmp, fmt, hash, mem, num};

Expand Down Expand Up @@ -26,11 +27,12 @@ impl ValidAlign {
/// It must *not* be zero.
#[inline]
pub(crate) const unsafe fn new_unchecked(align: usize) -> Self {
debug_assert!(align.is_power_of_two());

// SAFETY: By precondition, this must be a power of two, and
// our variants encompass all possible powers of two.
unsafe { mem::transmute::<usize, ValidAlign>(align) }
unsafe {
assert_unsafe_precondition!(align.is_power_of_two());
mem::transmute::<usize, ValidAlign>(align)
}
}

#[inline]
Expand Down
64 changes: 64 additions & 0 deletions library/core/src/mem/valid_size.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
use crate::convert::TryFrom;
use crate::intrinsics::assert_unsafe_precondition;
use crate::{fmt, num};

/// A type storing a possible object size (in bytes) in the rust abstract machine.
///
/// This can be thought of as a positive `isize`, or `usize` without the high bit
/// set. This is important because [`pointer::offset`] is UB for *byte* sizes
/// too large for an `isize`, and there's a corresponding language limit on the
/// size of any allocated object.
///
/// Note that particularly large sizes, while representable in this type, are
/// likely not to be supported by actual allocators and machines.
#[derive(Copy, Clone, Hash, Eq, PartialEq, Ord, PartialOrd)]
#[repr(transparent)]
#[cfg_attr(target_pointer_width = "16", rustc_layout_scalar_valid_range_end(0x7FFF))]
#[cfg_attr(target_pointer_width = "32", rustc_layout_scalar_valid_range_end(0x7FFF_FFFF))]
#[cfg_attr(target_pointer_width = "64", rustc_layout_scalar_valid_range_end(0x7FFF_FFFF_FFFF_FFFF))]
pub(crate) struct ValidSize(usize);

const MAX_SIZE: usize = isize::MAX as usize;

impl ValidSize {
/// Creates a `ValidSize` from a `usize` that fits in an `isize`.
///
/// # Safety
///
/// `size` must be less than or equal to `isize::MAX`.
///
/// Equivalently, it must not have its high bit set.
#[inline]
pub(crate) const unsafe fn new_unchecked(size: usize) -> Self {
// SAFETY: By precondition, this must be within our validity invariant.
unsafe {
assert_unsafe_precondition!(size <= MAX_SIZE);
ValidSize(size)
}
}

#[inline]
pub(crate) const fn as_usize(self) -> usize {
self.0
}
}

impl TryFrom<usize> for ValidSize {
type Error = num::TryFromIntError;

#[inline]
fn try_from(size: usize) -> Result<ValidSize, Self::Error> {
if size <= MAX_SIZE {
// SAFETY: Just checked it's within our validity invariant.
unsafe { Ok(ValidSize(size)) }
} else {
Err(num::TryFromIntError(()))
}
}
}

impl fmt::Debug for ValidSize {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.as_usize().fmt(f)
}
}
16 changes: 16 additions & 0 deletions library/core/tests/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ fn layout_debug_shows_log2_of_alignment() {
assert_eq!(s, "Layout { size: 24576, align: 8192 (1 << 13) }");
}

#[test]
fn layout_rejects_invalid_combinations() {
assert!(Layout::from_size_align(3, 3).is_err()); // bad align
assert!(Layout::from_size_align(1 << (usize::BITS - 1), 1).is_err()); // bad size
assert!(Layout::from_size_align(isize::MAX as usize, 2).is_err()); // fails round-up
assert!(Layout::from_size_align(1, 1 << (usize::BITS - 1)).is_err()); // fails round-up
}

// Running this normally doesn't do much, but it's also run in Miri, which
// will double-check that these are allowed by the validity invariants.
#[test]
Expand All @@ -29,3 +37,11 @@ fn layout_accepts_all_valid_alignments() {
assert_eq!(layout.align(), 1_usize << align);
}
}

#[test]
fn layout_accepts_various_valid_sizes() {
for shift in 1..usize::BITS {
let layout = Layout::from_size_align(usize::MAX >> shift, 1).unwrap();
assert_eq!(layout.size(), usize::MAX >> shift);
}
}
24 changes: 23 additions & 1 deletion src/test/ui/consts/std/alloc.32bit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,28 @@ LL | const LAYOUT_INVALID_THREE: Layout = unsafe { Layout::from_size_align_unche
09 00 00 00 03 00 00 00 │ ........
}

error: aborting due to 2 previous errors
error[E0080]: it is undefined behavior to use this value
--> $DIR/alloc.rs:17:1
|
LL | const LAYOUT_SIZE_NEGATIVE_ONE: Layout = unsafe { Layout::from_size_align_unchecked(-1 as _, 1) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .size: encountered 4294967295, but expected something less or equal to 2147483647
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 8, align: 4) {
ff ff ff ff 01 00 00 00 │ ........
}

error[E0080]: it is undefined behavior to use this value
--> $DIR/alloc.rs:21:1
|
LL | const LAYOUT_SIZE_HIGH_BIT: Layout = unsafe { Layout::from_size_align_unchecked(SIZE_MAX + 1, 1) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .size: encountered 2147483648, but expected something less or equal to 2147483647
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 8, align: 4) {
00 00 00 80 01 00 00 00 │ ........
}

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0080`.
24 changes: 23 additions & 1 deletion src/test/ui/consts/std/alloc.64bit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,28 @@ LL | const LAYOUT_INVALID_THREE: Layout = unsafe { Layout::from_size_align_unche
09 00 00 00 00 00 00 00 03 00 00 00 00 00 00 00 │ ................
}

error: aborting due to 2 previous errors
error[E0080]: it is undefined behavior to use this value
--> $DIR/alloc.rs:17:1
|
LL | const LAYOUT_SIZE_NEGATIVE_ONE: Layout = unsafe { Layout::from_size_align_unchecked(-1 as _, 1) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .size: encountered 18446744073709551615, but expected something less or equal to 9223372036854775807
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 16, align: 8) {
ff ff ff ff ff ff ff ff 01 00 00 00 00 00 00 00 │ ................
}

error[E0080]: it is undefined behavior to use this value
--> $DIR/alloc.rs:21:1
|
LL | const LAYOUT_SIZE_HIGH_BIT: Layout = unsafe { Layout::from_size_align_unchecked(SIZE_MAX + 1, 1) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value at .size: encountered 9223372036854775808, but expected something less or equal to 9223372036854775807
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 16, align: 8) {
00 00 00 00 00 00 00 80 01 00 00 00 00 00 00 00 │ ................
}

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0080`.
10 changes: 10 additions & 0 deletions src/test/ui/consts/std/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,14 @@ const LAYOUT_INVALID_ZERO: Layout = unsafe { Layout::from_size_align_unchecked(0
const LAYOUT_INVALID_THREE: Layout = unsafe { Layout::from_size_align_unchecked(9, 3) };
//~^ ERROR it is undefined behavior to use this value

// not ok, since size needs to be no more than `isize::MAX`
const LAYOUT_SIZE_NEGATIVE_ONE: Layout = unsafe { Layout::from_size_align_unchecked(-1 as _, 1) };
//~^ ERROR it is undefined behavior to use this value

// not ok, since size needs to be no more than `isize::MAX`
const LAYOUT_SIZE_HIGH_BIT: Layout = unsafe { Layout::from_size_align_unchecked(SIZE_MAX + 1, 1) };
//~^ ERROR it is undefined behavior to use this value

const SIZE_MAX: usize = isize::MAX as usize;

fn main() {}