From cfd9ece60722c57c5456db28632ae0b87665a15b Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 10 Jul 2022 15:37:31 -0700 Subject: [PATCH 1/2] Make too-large byte sizes a validity error in `Layout` --- library/core/src/alloc/layout.rs | 13 +++-- library/core/src/mem/mod.rs | 3 ++ library/core/src/mem/valid_size.rs | 64 +++++++++++++++++++++++ library/core/tests/alloc.rs | 16 ++++++ src/test/ui/consts/std/alloc.32bit.stderr | 24 ++++++++- src/test/ui/consts/std/alloc.64bit.stderr | 24 ++++++++- src/test/ui/consts/std/alloc.rs | 8 +++ 7 files changed, 145 insertions(+), 7 deletions(-) create mode 100644 library/core/src/mem/valid_size.rs diff --git a/library/core/src/alloc/layout.rs b/library/core/src/alloc/layout.rs index 39bccdb854c30..564735addc011 100644 --- a/library/core/src/alloc/layout.rs +++ b/library/core/src/alloc/layout.rs @@ -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 @@ -30,7 +30,7 @@ const fn size_align() -> (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 @@ -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. @@ -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. diff --git a/library/core/src/mem/mod.rs b/library/core/src/mem/mod.rs index ecd2b75ae4427..00688907de05d 100644 --- a/library/core/src/mem/mod.rs +++ b/library/core/src/mem/mod.rs @@ -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; diff --git a/library/core/src/mem/valid_size.rs b/library/core/src/mem/valid_size.rs new file mode 100644 index 0000000000000..f0064ab0415a6 --- /dev/null +++ b/library/core/src/mem/valid_size.rs @@ -0,0 +1,64 @@ +use crate::convert::TryFrom; +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; + +const _: () = unsafe { ValidSize::new_unchecked(MAX_SIZE); }; + +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 { + debug_assert!(size <= MAX_SIZE); + + // SAFETY: By precondition, this must be within our validity invariant. + unsafe { ValidSize(size) } + } + + #[inline] + pub(crate) const fn as_usize(self) -> usize { + self.0 + } +} + +impl TryFrom for ValidSize { + type Error = num::TryFromIntError; + + #[inline] + fn try_from(size: usize) -> Result { + 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) + } +} diff --git a/library/core/tests/alloc.rs b/library/core/tests/alloc.rs index 8a5a06b3440f8..5215ba91b5e08 100644 --- a/library/core/tests/alloc.rs +++ b/library/core/tests/alloc.rs @@ -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] @@ -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); + } +} diff --git a/src/test/ui/consts/std/alloc.32bit.stderr b/src/test/ui/consts/std/alloc.32bit.stderr index 79efcd3f62eed..2d130de458df6 100644 --- a/src/test/ui/consts/std/alloc.32bit.stderr +++ b/src/test/ui/consts/std/alloc.32bit.stderr @@ -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((isize::MAX as usize) + 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`. diff --git a/src/test/ui/consts/std/alloc.64bit.stderr b/src/test/ui/consts/std/alloc.64bit.stderr index cb477b72b3121..d7208c3450c1c 100644 --- a/src/test/ui/consts/std/alloc.64bit.stderr +++ b/src/test/ui/consts/std/alloc.64bit.stderr @@ -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((isize::MAX as usize) + 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`. diff --git a/src/test/ui/consts/std/alloc.rs b/src/test/ui/consts/std/alloc.rs index 708b954e84aea..11bf8344267eb 100644 --- a/src/test/ui/consts/std/alloc.rs +++ b/src/test/ui/consts/std/alloc.rs @@ -13,4 +13,12 @@ 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((isize::MAX as usize) + 1, 1) }; +//~^ ERROR it is undefined behavior to use this value + fn main() {} From b34f5b9f0f81ec9da4155dad15b8005561182edd Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 10 Jul 2022 16:45:40 -0700 Subject: [PATCH 2/2] fix CI; use assert_unsafe_precondition --- library/core/src/mem/valid_align.rs | 8 +++++--- library/core/src/mem/valid_size.rs | 10 +++++----- src/test/ui/consts/std/alloc.32bit.stderr | 2 +- src/test/ui/consts/std/alloc.64bit.stderr | 2 +- src/test/ui/consts/std/alloc.rs | 4 +++- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/library/core/src/mem/valid_align.rs b/library/core/src/mem/valid_align.rs index fcfa95120df21..9152eef07b1ce 100644 --- a/library/core/src/mem/valid_align.rs +++ b/library/core/src/mem/valid_align.rs @@ -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}; @@ -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::(align) } + unsafe { + assert_unsafe_precondition!(align.is_power_of_two()); + mem::transmute::(align) + } } #[inline] diff --git a/library/core/src/mem/valid_size.rs b/library/core/src/mem/valid_size.rs index f0064ab0415a6..f9e7a851714be 100644 --- a/library/core/src/mem/valid_size.rs +++ b/library/core/src/mem/valid_size.rs @@ -1,4 +1,5 @@ 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. @@ -19,8 +20,6 @@ pub(crate) struct ValidSize(usize); const MAX_SIZE: usize = isize::MAX as usize; -const _: () = unsafe { ValidSize::new_unchecked(MAX_SIZE); }; - impl ValidSize { /// Creates a `ValidSize` from a `usize` that fits in an `isize`. /// @@ -31,10 +30,11 @@ impl ValidSize { /// Equivalently, it must not have its high bit set. #[inline] pub(crate) const unsafe fn new_unchecked(size: usize) -> Self { - debug_assert!(size <= MAX_SIZE); - // SAFETY: By precondition, this must be within our validity invariant. - unsafe { ValidSize(size) } + unsafe { + assert_unsafe_precondition!(size <= MAX_SIZE); + ValidSize(size) + } } #[inline] diff --git a/src/test/ui/consts/std/alloc.32bit.stderr b/src/test/ui/consts/std/alloc.32bit.stderr index 2d130de458df6..7031306955c69 100644 --- a/src/test/ui/consts/std/alloc.32bit.stderr +++ b/src/test/ui/consts/std/alloc.32bit.stderr @@ -34,7 +34,7 @@ LL | const LAYOUT_SIZE_NEGATIVE_ONE: Layout = unsafe { Layout::from_size_align_u 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((isize::MAX as usize) + 1, 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. diff --git a/src/test/ui/consts/std/alloc.64bit.stderr b/src/test/ui/consts/std/alloc.64bit.stderr index d7208c3450c1c..0d443b845f966 100644 --- a/src/test/ui/consts/std/alloc.64bit.stderr +++ b/src/test/ui/consts/std/alloc.64bit.stderr @@ -34,7 +34,7 @@ LL | const LAYOUT_SIZE_NEGATIVE_ONE: Layout = unsafe { Layout::from_size_align_u 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((isize::MAX as usize) + 1, 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. diff --git a/src/test/ui/consts/std/alloc.rs b/src/test/ui/consts/std/alloc.rs index 11bf8344267eb..c68fd677d3867 100644 --- a/src/test/ui/consts/std/alloc.rs +++ b/src/test/ui/consts/std/alloc.rs @@ -18,7 +18,9 @@ const LAYOUT_SIZE_NEGATIVE_ONE: Layout = unsafe { Layout::from_size_align_unchec //~^ 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((isize::MAX as usize) + 1, 1) }; +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() {}