Skip to content

Commit

Permalink
Reduce MSRV to 1.61.0 (#234)
Browse files Browse the repository at this point in the history 
It turned out that there were only a few lines of code that required
our MSRV to be as high as it was before this commit (1.65.0), and those
lines were easy to modify to be compatible with this new MSRV.

Note that this requires introducing defensive code to
`FromZeroes::new_box_slice_zeroed` in order to sidestep a bug in
`Layout::from_size_align` that was present through 1.64.0.
  • Loading branch information
@joshlf
joshlf committed Aug 8, 2023
1 parent 311a2b0 commit a052b0d
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 75 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ all-features = true

[package.metadata.ci]
# The versions of the stable and nightly compiler toolchains to use in CI.
pinned-stable = "1.64.0"
pinned-nightly = "nightly-2022-10-17"
pinned-stable = "1.69.0"
pinned-nightly = "nightly-2023-05-25"

[features]
alloc = []
Expand Down
91 changes: 39 additions & 52 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,15 +340,23 @@ pub unsafe trait FromBytes {
where
Self: Sized,
{
let size = mem::size_of::<Self>()
.checked_mul(len)
.expect("mem::size_of::<Self>() * len overflows `usize`");
let align = mem::align_of::<Self>();
// 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,
// which can cause undefined behavior. See #64 for details.
//
// 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);
// TODO(#2): Use `Layout::repeat` when `alloc_layout_extra` is
// stabilized.
let layout = Layout::from_size_align(
mem::size_of::<Self>()
.checked_mul(len)
.expect("mem::size_of::<Self>() * len overflows `usize`"),
mem::align_of::<Self>(),
)
.expect("total allocation size overflows `isize`");
let layout =
Layout::from_size_align(size, align).expect("total allocation size overflows `isize`");

// TODO(#61): Add a "SAFETY" comment and remove this `allow`.
#[allow(clippy::undocumented_unsafe_blocks)]
Expand Down Expand Up @@ -1148,12 +1156,16 @@ impl<T> Unalign<T> {
/// If `self` does not satisfy `mem::align_of::<T>()`, then
/// `self.deref_unchecked()` may cause undefined behavior.
pub const unsafe fn deref_unchecked(&self) -> &T {
// SAFETY: `self.get_ptr()` returns a raw pointer to a valid `T` at the
// same memory location as `self`. It has no alignment guarantee, but
// the caller has promised that `self` is properly aligned, so we know
// that the pointer itself is aligned, and thus that it is sound to
// create a reference to a `T` at this memory location.
unsafe { &*self.get_ptr() }
// SAFETY: `Unalign<T>` is `repr(transparent)`, so there is a valid `T`
// at the same memory location as `self`. It has no alignment guarantee,
// but the caller has promised that `self` is properly aligned, so we
// know that it is sound to create a reference to `T` at this memory
// location.
//
// We use `mem::transmute` instead of `&*self.get_ptr()` because
// dereferencing pointers is not stable in `const` on our current MSRV
// (1.56 as of this writing).
unsafe { mem::transmute(self) }
}

/// Returns a mutable reference to the wrapped `T` without checking
Expand Down Expand Up @@ -1402,9 +1414,8 @@ macro_rules! transmute {
// `std` contexts in which `core` was not manually imported. This is
// not a problem for 2018 edition crates.
//
// This `allow` is only required on the v0.6.x branch due to
// behavior of older versions of Clippy which is no longer present
// on the MSRV we use on the main branch.
// Some older versions of Clippy have a bug in which they don't
// recognize the preceding safety comment.
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe { $crate::__real_transmute(e) }
}
Expand Down Expand Up @@ -2701,10 +2712,7 @@ mod alloc_support {
#[test]
fn test_extend_vec_zeroed() {
// Test extending when there is an existing allocation.
let mut v: Vec<u64> = Vec::with_capacity(3);
v.push(100);
v.push(200);
v.push(300);
let mut v = vec![100u64, 200, 300];
extend_vec_zeroed(&mut v, 3);
assert_eq!(v.len(), 6);
assert_eq!(&*v, &[100, 200, 300, 0, 0, 0]);
Expand All @@ -2721,10 +2729,7 @@ mod alloc_support {
#[test]
fn test_extend_vec_zeroed_zst() {
// Test extending when there is an existing (fake) allocation.
let mut v: Vec<()> = Vec::with_capacity(3);
v.push(());
v.push(());
v.push(());
let mut v = vec![(), (), ()];
extend_vec_zeroed(&mut v, 3);
assert_eq!(v.len(), 6);
assert_eq!(&*v, &[(), (), (), (), (), ()]);
Expand All @@ -2747,30 +2752,21 @@ mod alloc_support {
drop(v);

// Insert at start.
let mut v: Vec<u64> = Vec::with_capacity(3);
v.push(100);
v.push(200);
v.push(300);
let mut v = vec![100u64, 200, 300];
insert_vec_zeroed(&mut v, 0, 2);
assert_eq!(v.len(), 5);
assert_eq!(&*v, &[0, 0, 100, 200, 300]);
drop(v);

// Insert at middle.
let mut v: Vec<u64> = Vec::with_capacity(3);
v.push(100);
v.push(200);
v.push(300);
let mut v = vec![100u64, 200, 300];
insert_vec_zeroed(&mut v, 1, 1);
assert_eq!(v.len(), 4);
assert_eq!(&*v, &[100, 0, 200, 300]);
drop(v);

// Insert at end.
let mut v: Vec<u64> = Vec::with_capacity(3);
v.push(100);
v.push(200);
v.push(300);
let mut v = vec![100u64, 200, 300];
insert_vec_zeroed(&mut v, 3, 1);
assert_eq!(v.len(), 4);
assert_eq!(&*v, &[100, 200, 300, 0]);
Expand All @@ -2787,30 +2783,21 @@ mod alloc_support {
drop(v);

// Insert at start.
let mut v: Vec<()> = Vec::with_capacity(3);
v.push(());
v.push(());
v.push(());
let mut v = vec![(), (), ()];
insert_vec_zeroed(&mut v, 0, 2);
assert_eq!(v.len(), 5);
assert_eq!(&*v, &[(), (), (), (), ()]);
drop(v);

// Insert at middle.
let mut v: Vec<()> = Vec::with_capacity(3);
v.push(());
v.push(());
v.push(());
let mut v = vec![(), (), ()];
insert_vec_zeroed(&mut v, 1, 1);
assert_eq!(v.len(), 4);
assert_eq!(&*v, &[(), (), (), ()]);
drop(v);

// Insert at end.
let mut v: Vec<()> = Vec::with_capacity(3);
v.push(());
v.push(());
v.push(());
let mut v = vec![(), (), ()];
insert_vec_zeroed(&mut v, 3, 1);
assert_eq!(v.len(), 4);
assert_eq!(&*v, &[(), (), (), ()]);
Expand Down Expand Up @@ -2901,7 +2888,7 @@ mod alloc_support {
// remove this conditional compilation (and this comment) entirely.
#[rustversion::since(1.65.0)]
#[test]
#[should_panic(expected = "total allocation size overflows `isize`: LayoutError")]
#[should_panic(expected = "assertion failed: size <= max_alloc")]
fn test_new_box_slice_zeroed_panics_isize_overflow() {
// TODO: Move this to the top of the module once this test is
// compiled unconditionally. Right now, it causes an unused import
Expand Down Expand Up @@ -3736,7 +3723,7 @@ mod tests {
/// has had its bits flipped (by applying `^= 0xFF`).
///
/// `N` is the size of `t` in bytes.
fn test<const N: usize, T: FromBytes + AsBytes + Debug + Eq + ?Sized>(
fn test<T: FromBytes + AsBytes + Debug + Eq + ?Sized, const N: usize>(
t: &mut T,
bytes: &[u8],
post_mutation: &T,
Expand Down Expand Up @@ -3808,12 +3795,12 @@ mod tests {
};
let post_mutation_expected_a =
if cfg!(target_endian = "little") { 0x00_00_00_FE } else { 0xFF_00_00_01 };
test::<12, _>(
test::<_, 12>(
&mut Foo { a: 1, b: Wrapping(2), c: None },
expected_bytes.as_bytes(),
&Foo { a: post_mutation_expected_a, b: Wrapping(2), c: None },
);
test::<3, _>(
test::<_, 3>(
Unsized::from_mut_slice(&mut [1, 2, 3]),
&[1, 2, 3],
Unsized::from_mut_slice(&mut [0xFE, 2, 3]),
Expand Down
4 changes: 2 additions & 2 deletions tests/trybuild.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
// - `tests/ui-msrv` - Contains symlinks to the `.rs` files in
// `tests/ui-nightly`, and contains `.err` and `.out` files for MSRV

#[rustversion::any(nightly)]
#[rustversion::nightly]
const SOURCE_FILES_GLOB: &str = "tests/ui-nightly/*.rs";
#[rustversion::all(stable, not(stable(1.61.0)))]
#[rustversion::stable(1.69.0)]
const SOURCE_FILES_GLOB: &str = "tests/ui-stable/*.rs";
#[rustversion::stable(1.61.0)]
const SOURCE_FILES_GLOB: &str = "tests/ui-msrv/*.rs";
Expand Down
13 changes: 2 additions & 11 deletions tests/ui-nightly/transmute-illegal.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,10 @@ error[E0277]: the trait bound `*const usize: AsBytes` is not satisfied
| the trait `AsBytes` is not implemented for `*const usize`
| required by a bound introduced by this call
|
= help: the following other types implement trait `AsBytes`:
f32
f64
i128
i16
i32
i64
i8
isize
and $N others
= help: the trait `AsBytes` is implemented for `usize`
note: required by a bound in `POINTER_VALUE::transmute`
--> tests/ui-nightly/transmute-illegal.rs:10:30
|
10 | const POINTER_VALUE: usize = zerocopy::transmute!(&0usize as *const usize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `POINTER_VALUE::transmute`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `transmute`
= note: this error originates in the macro `zerocopy::transmute` (in Nightly builds, run with -Z macro-backtrace for more info)
4 changes: 2 additions & 2 deletions zerocopy-derive/tests/trybuild.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
// - `tests/ui-msrv` - Contains symlinks to the `.rs` files in
// `tests/ui-nightly`, and contains `.err` and `.out` files for MSRV

#[rustversion::any(nightly)]
#[rustversion::nightly]
const SOURCE_FILES_GLOB: &str = "tests/ui-nightly/*.rs";
#[rustversion::all(stable, not(stable(1.61.0)))]
#[rustversion::stable(1.69.0)]
const SOURCE_FILES_GLOB: &str = "tests/ui-stable/*.rs";
#[rustversion::stable(1.61.0)]
const SOURCE_FILES_GLOB: &str = "tests/ui-msrv/*.rs";
Expand Down
12 changes: 6 additions & 6 deletions zerocopy-derive/tests/ui-nightly/derive_transparent.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ note: required for `TransparentStruct<NotZerocopy>` to implement `FromBytes`
--> tests/ui-nightly/derive_transparent.rs:23:19
|
23 | #[derive(AsBytes, FromBytes, Unaligned)]
| ^^^^^^^^^
| ^^^^^^^^^ unsatisfied trait bound introduced in this `derive` macro
note: required by a bound in `_::{closure#0}::assert_impl_all`
--> tests/ui-nightly/derive_transparent.rs:33:1
|
33 | assert_impl_all!(TransparentStruct<NotZerocopy>: FromBytes);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `_::{closure#0}::assert_impl_all`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the derive macro `FromBytes` which comes from the expansion of the macro `assert_impl_all` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `NotZerocopy: AsBytes` is not satisfied
Expand All @@ -46,12 +46,12 @@ note: required for `TransparentStruct<NotZerocopy>` to implement `AsBytes`
--> tests/ui-nightly/derive_transparent.rs:23:10
|
23 | #[derive(AsBytes, FromBytes, Unaligned)]
| ^^^^^^^
| ^^^^^^^ unsatisfied trait bound introduced in this `derive` macro
note: required by a bound in `_::{closure#0}::assert_impl_all`
--> tests/ui-nightly/derive_transparent.rs:34:1
|
34 | assert_impl_all!(TransparentStruct<NotZerocopy>: AsBytes);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `_::{closure#0}::assert_impl_all`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the derive macro `AsBytes` which comes from the expansion of the macro `assert_impl_all` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: the trait bound `NotZerocopy: Unaligned` is not satisfied
Expand All @@ -74,10 +74,10 @@ note: required for `TransparentStruct<NotZerocopy>` to implement `Unaligned`
--> tests/ui-nightly/derive_transparent.rs:23:30
|
23 | #[derive(AsBytes, FromBytes, Unaligned)]
| ^^^^^^^^^
| ^^^^^^^^^ unsatisfied trait bound introduced in this `derive` macro
note: required by a bound in `_::{closure#0}::assert_impl_all`
--> tests/ui-nightly/derive_transparent.rs:35:1
|
35 | assert_impl_all!(TransparentStruct<NotZerocopy>: Unaligned);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `_::{closure#0}::assert_impl_all`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all`
= note: this error originates in the derive macro `Unaligned` which comes from the expansion of the macro `assert_impl_all` (in Nightly builds, run with -Z macro-backtrace for more info)

0 comments on commit a052b0d

Please sign in to comment.