From 1eb1ca62b0028a560a59a075b47a3c391eecf1cf Mon Sep 17 00:00:00 2001 From: Joshua Liebow-Feeser Date: Sun, 15 Sep 2024 09:28:18 -0400 Subject: [PATCH] Make extend_vec_zeroed, insert_vec_zeroed fallible They now return `AllocError` on allocation failure. `insert_vec_zeroed` still panics if its `position` argument is out-of-bounds. This requires using `Vec::try_reserve`, which is unstable before 1.57.0. Since our MSRV is currently 1.56.0, these functions are now conditionally compiled and only available on 1.57.0 and later. Closes #1653 --- .github/workflows/ci.yml | 6 ++-- Cargo.toml | 4 +-- src/layout.rs | 2 +- src/lib.rs | 65 ++++++++++++++++++++++++---------------- src/util/macros.rs | 16 +++++----- src/util/mod.rs | 4 +-- 6 files changed, 56 insertions(+), 41 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a9567ab548f..50b01797908 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,7 +57,7 @@ jobs: "zerocopy-generic-bounds-in-const-fn", "zerocopy-target-has-atomics", "zerocopy-aarch64-simd", - "zerocopy-panic-in-const" + "zerocopy-panic-in-const-and-vec-try-reserve" ] target: [ "i686-unknown-linux-gnu", @@ -93,7 +93,7 @@ jobs: features: "--all-features" - toolchain: "zerocopy-aarch64-simd" features: "--all-features" - - toolchain: "zerocopy-panic-in-const" + - toolchain: "zerocopy-panic-in-const-and-vec-try-reserve" features: "--all-features" # Exclude any combination for the zerocopy-derive crate which # uses zerocopy features. @@ -114,7 +114,7 @@ jobs: - crate: "zerocopy-derive" toolchain: "zerocopy-aarch64-simd" - crate: "zerocopy-derive" - toolchain: "zerocopy-panic-in-const" + toolchain: "zerocopy-panic-in-const-and-vec-try-reserve" # Exclude non-aarch64 targets from the `zerocopy-aarch64-simd` # toolchain. - toolchain: "zerocopy-aarch64-simd" diff --git a/Cargo.toml b/Cargo.toml index 2031bd2be60..5650239c624 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,8 +44,8 @@ zerocopy-target-has-atomics = "1.60.0" # versions, these types require the "simd-nightly" feature. zerocopy-aarch64-simd = "1.59.0" -# Permit panicking in `const fn`s. -zerocopy-panic-in-const = "1.57.0" +# Permit panicking in `const fn`s and calling `Vec::try_reserve`. +zerocopy-panic-in-const-and-vec-try-reserve = "1.57.0" [package.metadata.ci] # The versions of the stable and nightly compiler toolchains to use in CI. diff --git a/src/layout.rs b/src/layout.rs index 26b6ef11ed9..7811c8672b4 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -880,7 +880,7 @@ mod tests { layout(size_info, align).validate_cast_and_convert_metadata(addr, bytes_len, cast_type) }).map_err(|d| { let msg = d.downcast::<&'static str>().ok().map(|s| *s.as_ref()); - assert!(msg.is_some() || cfg!(not(zerocopy_panic_in_const)), "non-string panic messages are not permitted when `--cfg zerocopy_panic_in_const` is set"); + assert!(msg.is_some() || cfg!(not(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)), "non-string panic messages are not permitted when `--cfg zerocopy_zerocopy_panic_in_const_and_vec_try_reserve` is set"); msg }); std::panic::set_hook(previous_hook); diff --git a/src/lib.rs b/src/lib.rs index 1521d7ca02b..b0e9121d99d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4371,31 +4371,38 @@ pub unsafe trait Unaligned { #[cfg(feature = "alloc")] #[cfg_attr(doc_cfg, doc(cfg(feature = "alloc")))] mod alloc_support { + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] use super::*; /// Extends a `Vec` by pushing `additional` new items onto the end of the /// vector. The new items are initialized with zeros. - /// - /// # Panics - /// - /// Panics if `Vec::reserve(additional)` fails to reserve enough memory. + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] #[inline(always)] - pub fn extend_vec_zeroed(v: &mut Vec, additional: usize) { - insert_vec_zeroed(v, v.len(), additional); + pub fn extend_vec_zeroed( + v: &mut Vec, + additional: usize, + ) -> Result<(), AllocError> { + // PANICS: We pass `v.len()` for `position`, so the `position > v.len()` + // panic condition is not satisfied. + insert_vec_zeroed(v, v.len(), additional) } - /// Inserts `additional` new items into `Vec` at `position`. - /// The new items are initialized with zeros. + /// Inserts `additional` new items into `Vec` at `position`. The new + /// items are initialized with zeros. /// /// # Panics /// - /// * Panics if `position > v.len()`. - /// * Panics if `Vec::reserve(additional)` fails to reserve enough memory. + /// Panics if `position > v.len()`. + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] #[inline] - pub fn insert_vec_zeroed(v: &mut Vec, position: usize, additional: usize) { + pub fn insert_vec_zeroed( + v: &mut Vec, + position: usize, + additional: usize, + ) -> Result<(), AllocError> { assert!(position <= v.len()); - v.reserve(additional); - // SAFETY: The `reserve` call guarantees that these cannot overflow: + v.try_reserve(additional).map_err(|_| AllocError)?; + // SAFETY: The `try_reserve` call guarantees that these cannot overflow: // * `ptr.add(position)` // * `position + additional` // * `v.len() + additional` @@ -4411,104 +4418,112 @@ mod alloc_support { #[allow(clippy::arithmetic_side_effects)] v.set_len(v.len() + additional); } + + Ok(()) } #[cfg(test)] mod tests { use core::convert::TryFrom as _; + use super::super::*; + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] use super::*; + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] #[test] fn test_extend_vec_zeroed() { // Test extending when there is an existing allocation. let mut v = vec![100u64, 200, 300]; - extend_vec_zeroed(&mut v, 3); + extend_vec_zeroed(&mut v, 3).unwrap(); assert_eq!(v.len(), 6); assert_eq!(&*v, &[100, 200, 300, 0, 0, 0]); drop(v); // Test extending when there is no existing allocation. let mut v: Vec = Vec::new(); - extend_vec_zeroed(&mut v, 3); + extend_vec_zeroed(&mut v, 3).unwrap(); assert_eq!(v.len(), 3); assert_eq!(&*v, &[0, 0, 0]); drop(v); } + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] #[test] fn test_extend_vec_zeroed_zst() { // Test extending when there is an existing (fake) allocation. let mut v = vec![(), (), ()]; - extend_vec_zeroed(&mut v, 3); + extend_vec_zeroed(&mut v, 3).unwrap(); assert_eq!(v.len(), 6); assert_eq!(&*v, &[(), (), (), (), (), ()]); drop(v); // Test extending when there is no existing (fake) allocation. let mut v: Vec<()> = Vec::new(); - extend_vec_zeroed(&mut v, 3); + extend_vec_zeroed(&mut v, 3).unwrap(); assert_eq!(&*v, &[(), (), ()]); drop(v); } + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] #[test] fn test_insert_vec_zeroed() { // Insert at start (no existing allocation). let mut v: Vec = Vec::new(); - insert_vec_zeroed(&mut v, 0, 2); + insert_vec_zeroed(&mut v, 0, 2).unwrap(); assert_eq!(v.len(), 2); assert_eq!(&*v, &[0, 0]); drop(v); // Insert at start. let mut v = vec![100u64, 200, 300]; - insert_vec_zeroed(&mut v, 0, 2); + insert_vec_zeroed(&mut v, 0, 2).unwrap(); assert_eq!(v.len(), 5); assert_eq!(&*v, &[0, 0, 100, 200, 300]); drop(v); // Insert at middle. let mut v = vec![100u64, 200, 300]; - insert_vec_zeroed(&mut v, 1, 1); + insert_vec_zeroed(&mut v, 1, 1).unwrap(); assert_eq!(v.len(), 4); assert_eq!(&*v, &[100, 0, 200, 300]); drop(v); // Insert at end. let mut v = vec![100u64, 200, 300]; - insert_vec_zeroed(&mut v, 3, 1); + insert_vec_zeroed(&mut v, 3, 1).unwrap(); assert_eq!(v.len(), 4); assert_eq!(&*v, &[100, 200, 300, 0]); drop(v); } + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] #[test] fn test_insert_vec_zeroed_zst() { // Insert at start (no existing fake allocation). let mut v: Vec<()> = Vec::new(); - insert_vec_zeroed(&mut v, 0, 2); + insert_vec_zeroed(&mut v, 0, 2).unwrap(); assert_eq!(v.len(), 2); assert_eq!(&*v, &[(), ()]); drop(v); // Insert at start. let mut v = vec![(), (), ()]; - insert_vec_zeroed(&mut v, 0, 2); + insert_vec_zeroed(&mut v, 0, 2).unwrap(); assert_eq!(v.len(), 5); assert_eq!(&*v, &[(), (), (), (), ()]); drop(v); // Insert at middle. let mut v = vec![(), (), ()]; - insert_vec_zeroed(&mut v, 1, 1); + insert_vec_zeroed(&mut v, 1, 1).unwrap(); assert_eq!(v.len(), 4); assert_eq!(&*v, &[(), (), (), ()]); drop(v); // Insert at end. let mut v = vec![(), (), ()]; - insert_vec_zeroed(&mut v, 3, 1); + insert_vec_zeroed(&mut v, 3, 1).unwrap(); assert_eq!(v.len(), 4); assert_eq!(&*v, &[(), (), (), ()]); drop(v); diff --git a/src/util/macros.rs b/src/util/macros.rs index abeeb9ed8ba..4a817ca10f4 100644 --- a/src/util/macros.rs +++ b/src/util/macros.rs @@ -640,9 +640,9 @@ macro_rules! maybe_const_trait_bounded_fn { /// non-panicking desugaring will fail to compile. macro_rules! const_panic { ($fmt:literal) => {{ - #[cfg(zerocopy_panic_in_const)] + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] panic!($fmt); - #[cfg(not(zerocopy_panic_in_const))] + #[cfg(not(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve))] const_panic!(@non_panic $fmt) }}; (@non_panic $fmt:expr) => {{ @@ -661,9 +661,9 @@ macro_rules! const_panic { /// accommodate old toolchains. macro_rules! const_assert { ($e:expr) => {{ - #[cfg(zerocopy_panic_in_const)] + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] assert!($e); - #[cfg(not(zerocopy_panic_in_const))] + #[cfg(not(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve))] { let e = $e; if !e { @@ -676,9 +676,9 @@ macro_rules! const_assert { /// Like `const_assert!`, but relative to `debug_assert!`. macro_rules! const_debug_assert { ($e:expr $(, $msg:expr)?) => {{ - #[cfg(zerocopy_panic_in_const)] + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] debug_assert!($e $(, $msg)?); - #[cfg(not(zerocopy_panic_in_const))] + #[cfg(not(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve))] { // Use this (rather than `#[cfg(debug_assertions)]`) to ensure that // `$e` is always compiled even if it will never be evaluated at @@ -697,10 +697,10 @@ macro_rules! const_debug_assert { /// toolchain supports panicking in `const fn`. macro_rules! const_unreachable { () => {{ - #[cfg(zerocopy_panic_in_const)] + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] unreachable!(); - #[cfg(not(zerocopy_panic_in_const))] + #[cfg(not(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve))] loop {} }}; } diff --git a/src/util/mod.rs b/src/util/mod.rs index 3879e981f18..49504c6f671 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -628,7 +628,7 @@ pub(crate) const fn round_down_to_next_multiple_of_alignment( align: NonZeroUsize, ) -> usize { let align = align.get(); - #[cfg(zerocopy_panic_in_const)] + #[cfg(zerocopy_zerocopy_panic_in_const_and_vec_try_reserve)] debug_assert!(align.is_power_of_two()); // Subtraction can't underflow because `align.get() >= 1`. @@ -865,7 +865,7 @@ mod tests { #[rustversion::since(1.57.0)] #[test] #[should_panic] - fn test_round_down_to_next_multiple_of_alignment_panic_in_const() { + fn test_round_down_to_next_multiple_of_alignment_zerocopy_panic_in_const_and_vec_try_reserve() { round_down_to_next_multiple_of_alignment(0, NonZeroUsize::new(3).unwrap()); } }