From b77d9ea9494e68ee8dde11140c1a83c87b0b7071 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Sat, 11 Dec 2021 10:28:41 +0100 Subject: [PATCH 01/13] Added a pipeline to deny 'no_global_oom_handling' Added try_reserve to VecWriter, Vec, VecDeque and HashMap Made the OOM test succeed on the latest nightly Made the OOM check work on nightly Disabled stable checks for now Removed feature that is stable in current nightly Fixed issue where Box<[T]> would not do the size limit check Added a drop guard to `impl Decode for Vec` to make sure memory is not leaked when T::Decode panics Made the implementation of `VecWriter` use less lines of unsfe Fixed errors while merging --- .github/workflows/rust.yml | 32 ++- Cargo.toml | 3 + src/error.rs | 57 +++++ src/features/impl_alloc.rs | 444 ++++++++++++++++++++++++------------- src/features/impl_std.rs | 9 +- src/lib.rs | 4 + 6 files changed, 386 insertions(+), 163 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 53af7bb0..5d958b37 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -48,7 +48,7 @@ "uses": "actions-rs/cargo@v1", "with": { "command": "check", - "args": "--all-features" + "args": "--features std,derive" }, "name": "Run `cargo check`" }, @@ -128,6 +128,32 @@ fi", } ] }, + "no_oom": { + "name": "Strict OOM checks", + "runs-on": "ubuntu-latest", + "steps": [ + { + "uses": "actions/checkout@v2", + "name": "Checkout" + }, + { + "uses": "actions-rs/toolchain@v1", + "with": { + "profile": "minimal", + "toolchain": "nightly", + "components": "rust-src", + "override": true + }, + "name": "Install Rust nightly" + }, + { + "run": "cargo build --no-default-features --features unstable-strict-oom-checks -Z build-std=core,alloc --target x86_64-unknown-linux-gnu", + "env": { + "RUSTFLAGS": "--cfg no_global_oom_handling" + } + } + ] + }, "lints": { "name": "Lints", "runs-on": "ubuntu-latest", @@ -158,7 +184,7 @@ fi", "uses": "actions-rs/cargo@v1", "with": { "command": "clippy", - "args": "--all-features -- -D warnings" + "args": "--features std,derive -- -D warnings" }, "name": "Run `cargo clippy`" } @@ -213,7 +239,7 @@ fi", "uses": "actions-rs/tarpaulin@v0.1", "with": { "version": "0.19.1", - "args": "--all --all-features" + "args": "--all --features std,derive" } }, { diff --git a/Cargo.toml b/Cargo.toml index e0f48513..8b49146b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,9 @@ std = ["alloc", "serde?/std"] alloc = ["serde?/alloc"] derive = ["bincode_derive"] +# experimental strict OOM checks, requires nightly features +unstable-strict-oom-checks = ["alloc"] + [dependencies] bincode_derive = { path = "derive", version = "2.0.0-rc.3", optional = true } serde = { version = "1.0", default-features = false, optional = true } diff --git a/src/error.rs b/src/error.rs index 7fb9c5c0..d3e51ec6 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,5 +1,10 @@ //! Errors that can be encounting by Encoding and Decoding. +#[cfg(feature = "alloc")] +use alloc::collections::TryReserveError; +#[cfg(all(feature = "alloc", feature = "unstable-strict-oom-checks"))] +use core::alloc::AllocError; + /// Errors that can be encountered by encoding a type #[non_exhaustive] #[derive(Debug)] @@ -51,6 +56,10 @@ pub enum EncodeError { time: std::boxed::Box, }, + /// bincode failed to allocate enough memory + #[cfg(feature = "alloc")] + OutOfMemory(OutOfMemory), + #[cfg(feature = "serde")] /// A serde-specific error that occurred while decoding. Serde(crate::features::serde::EncodeError), @@ -188,6 +197,54 @@ pub enum DecodeError { #[cfg(feature = "serde")] /// A serde-specific error that occurred while decoding. Serde(crate::features::serde::DecodeError), + + /// bincode failed to allocate enough memory + #[cfg(feature = "alloc")] + OutOfMemory(OutOfMemory), +} + +/// A wrapper to make all the out of memory errors consistent +#[cfg(feature = "alloc")] +#[derive(Debug, PartialEq)] +pub enum OutOfMemory { + /// Failed to reserve an entry + TryReserve(TryReserveError), + #[cfg(feature = "unstable-strict-oom-checks")] + /// Failed to allocate memory + Alloc(AllocError), +} + +impl From for DecodeError { + fn from(e: TryReserveError) -> Self { + Self::OutOfMemory(OutOfMemory::TryReserve(e)) + } +} + +impl From for EncodeError { + fn from(e: TryReserveError) -> Self { + Self::OutOfMemory(OutOfMemory::TryReserve(e)) + } +} + +#[cfg(all(feature = "alloc", feature = "unstable-strict-oom-checks"))] +impl From for DecodeError { + fn from(e: AllocError) -> Self { + Self::OutOfMemory(OutOfMemory::Alloc(e)) + } +} + +#[cfg(all(feature = "alloc", feature = "unstable-strict-oom-checks"))] +impl From for EncodeError { + fn from(e: AllocError) -> Self { + Self::OutOfMemory(OutOfMemory::Alloc(e)) + } +} + +#[cfg(all(feature = "alloc", feature = "unstable-strict-oom-checks"))] +impl From for DecodeError { + fn from(e: AllocError) -> Self { + Self::OutOfMemory(OutOfMemory::Alloc(e)) + } } impl core::fmt::Display for DecodeError { diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs index f5186a76..b4195260 100644 --- a/src/features/impl_alloc.rs +++ b/src/features/impl_alloc.rs @@ -11,7 +11,6 @@ use crate::{ use alloc::{ borrow::{Cow, ToOwned}, boxed::Box, - collections::*, rc::Rc, string::String, vec::Vec, @@ -42,7 +41,21 @@ impl VecWriter { impl enc::write::Writer for VecWriter { #[inline(always)] fn write(&mut self, bytes: &[u8]) -> Result<(), EncodeError> { - self.inner.extend_from_slice(bytes); + self.inner.try_reserve(bytes.len())?; + + let start = self.inner.len(); + + // Get a slice of `&mut [MaybeUninit]` of the remaining capacity + let remaining = &mut self.inner.spare_capacity_mut()[..bytes.len()]; + for (i, b) in bytes.iter().copied().enumerate() { + // TODO: is there a better way to copy from `&mut [MaybeUninit]` to `&[u8]`? + remaining[i].write(b); + } + + unsafe { + // Safety: We reserved enough bytes, and the bytes have values written to them + self.inner.set_len(start + bytes.len()); + } Ok(()) } } @@ -63,188 +76,195 @@ pub fn encode_to_vec(val: E, config: C) -> Result Decode for BinaryHeap -where - T: Decode + Ord, -{ - fn decode(decoder: &mut D) -> Result { - Ok(Vec::::decode(decoder)?.into()) +// TODO: these collections straight up don't exist with `no_global_oom_handling` +#[cfg(not(feature = "unstable-strict-oom-checks"))] +mod collections { + use super::*; + use alloc::collections::{BTreeMap, BTreeSet, BinaryHeap, VecDeque}; + + impl Decode for BinaryHeap + where + T: Decode + Ord, + { + fn decode(decoder: &mut D) -> Result { + Ok(Vec::::decode(decoder)?.into()) + } } -} -impl<'de, T> BorrowDecode<'de> for BinaryHeap -where - T: BorrowDecode<'de> + Ord, -{ - fn borrow_decode>(decoder: &mut D) -> Result { - Ok(Vec::::borrow_decode(decoder)?.into()) + impl<'de, T> BorrowDecode<'de> for BinaryHeap + where + T: BorrowDecode<'de> + Ord, + { + fn borrow_decode>(decoder: &mut D) -> Result { + Ok(Vec::::borrow_decode(decoder)?.into()) + } } -} -impl Encode for BinaryHeap -where - T: Encode + Ord, -{ - fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { - // BLOCKEDTODO(https://github.com/rust-lang/rust/issues/83659): we can u8 optimize this with `.as_slice()` - crate::enc::encode_slice_len(encoder, self.len())?; - for val in self.iter() { - val.encode(encoder)?; + impl Encode for BinaryHeap + where + T: Encode + Ord, + { + fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { + // BLOCKEDTODO(https://github.com/rust-lang/rust/issues/83659): we can u8 optimize this with `.as_slice()` + crate::enc::encode_slice_len(encoder, self.len())?; + for val in self.iter() { + val.encode(encoder)?; + } + Ok(()) } - Ok(()) } -} -impl Decode for BTreeMap -where - K: Decode + Ord, - V: Decode, -{ - fn decode(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::<(K, V)>(len)?; + impl Decode for BTreeMap + where + K: Decode + Ord, + V: Decode, + { + fn decode(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::<(K, V)>(len)?; - let mut map = BTreeMap::new(); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); + let mut map = BTreeMap::new(); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); - let key = K::decode(decoder)?; - let value = V::decode(decoder)?; - map.insert(key, value); + let key = K::decode(decoder)?; + let value = V::decode(decoder)?; + map.insert(key, value); + } + Ok(map) } - Ok(map) } -} -impl<'de, K, V> BorrowDecode<'de> for BTreeMap -where - K: BorrowDecode<'de> + Ord, - V: BorrowDecode<'de>, -{ - fn borrow_decode>(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::<(K, V)>(len)?; + impl<'de, K, V> BorrowDecode<'de> for BTreeMap + where + K: BorrowDecode<'de> + Ord, + V: BorrowDecode<'de>, + { + fn borrow_decode>(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::<(K, V)>(len)?; - let mut map = BTreeMap::new(); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); + let mut map = BTreeMap::new(); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); - let key = K::borrow_decode(decoder)?; - let value = V::borrow_decode(decoder)?; - map.insert(key, value); + let key = K::borrow_decode(decoder)?; + let value = V::borrow_decode(decoder)?; + map.insert(key, value); + } + Ok(map) } - Ok(map) } -} -impl Encode for BTreeMap -where - K: Encode + Ord, - V: Encode, -{ - fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { - crate::enc::encode_slice_len(encoder, self.len())?; - for (key, val) in self.iter() { - key.encode(encoder)?; - val.encode(encoder)?; + impl Encode for BTreeMap + where + K: Encode + Ord, + V: Encode, + { + fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { + crate::enc::encode_slice_len(encoder, self.len())?; + for (key, val) in self.iter() { + key.encode(encoder)?; + val.encode(encoder)?; + } + Ok(()) } - Ok(()) } -} -impl Decode for BTreeSet -where - T: Decode + Ord, -{ - fn decode(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; + impl Decode for BTreeSet + where + T: Decode + Ord, + { + fn decode(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::(len)?; - let mut map = BTreeSet::new(); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); + let mut map = BTreeSet::new(); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); - let key = T::decode(decoder)?; - map.insert(key); + let key = T::decode(decoder)?; + map.insert(key); + } + Ok(map) } - Ok(map) } -} -impl<'de, T> BorrowDecode<'de> for BTreeSet -where - T: BorrowDecode<'de> + Ord, -{ - fn borrow_decode>(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; + impl<'de, T> BorrowDecode<'de> for BTreeSet + where + T: BorrowDecode<'de> + Ord, + { + fn borrow_decode>(decoder: &mut D) -> Result { + let len = crate::de::decode_slice_len(decoder)?; + decoder.claim_container_read::(len)?; - let mut map = BTreeSet::new(); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); + let mut map = BTreeSet::new(); + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); - let key = T::borrow_decode(decoder)?; - map.insert(key); + let key = T::borrow_decode(decoder)?; + map.insert(key); + } + Ok(map) } - Ok(map) } -} -impl Encode for BTreeSet -where - T: Encode + Ord, -{ - fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { - crate::enc::encode_slice_len(encoder, self.len())?; - for item in self.iter() { - item.encode(encoder)?; + impl Encode for BTreeSet + where + T: Encode + Ord, + { + fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { + crate::enc::encode_slice_len(encoder, self.len())?; + for item in self.iter() { + item.encode(encoder)?; + } + Ok(()) } - Ok(()) } -} -impl Decode for VecDeque -where - T: Decode, -{ - fn decode(decoder: &mut D) -> Result { - Ok(Vec::::decode(decoder)?.into()) + impl Decode for VecDeque + where + T: Decode, + { + fn decode(decoder: &mut D) -> Result { + Ok(Vec::::decode(decoder)?.into()) + } } -} -impl<'de, T> BorrowDecode<'de> for VecDeque -where - T: BorrowDecode<'de>, -{ - fn borrow_decode>(decoder: &mut D) -> Result { - Ok(Vec::::borrow_decode(decoder)?.into()) + impl<'de, T> BorrowDecode<'de> for VecDeque + where + T: BorrowDecode<'de>, + { + fn borrow_decode>(decoder: &mut D) -> Result { + Ok(Vec::::borrow_decode(decoder)?.into()) + } } -} -impl Encode for VecDeque -where - T: Encode, -{ - fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { - crate::enc::encode_slice_len(encoder, self.len())?; - if unty::type_equal::() { - let slices: (&[T], &[T]) = self.as_slices(); - // Safety: T is u8 so turning this into `&[u8]` is okay - let slices: (&[u8], &[u8]) = unsafe { - ( - core::slice::from_raw_parts(slices.0.as_ptr().cast(), slices.0.len()), - core::slice::from_raw_parts(slices.1.as_ptr().cast(), slices.1.len()), - ) - }; - - encoder.writer().write(slices.0)?; - encoder.writer().write(slices.1)?; - } else { - for item in self.iter() { - item.encode(encoder)?; + impl Encode for VecDeque + where + T: Encode, + { + fn encode(&self, encoder: &mut E) -> Result<(), EncodeError> { + crate::enc::encode_slice_len(encoder, self.len())?; + if unty::type_equal::() { + let slices: (&[T], &[T]) = self.as_slices(); + // Safety: T is u8 so turning this into `&[u8]` is okay + let slices: (&[u8], &[u8]) = unsafe { + ( + core::slice::from_raw_parts(slices.0.as_ptr().cast(), slices.0.len()), + core::slice::from_raw_parts(slices.1.as_ptr().cast(), slices.1.len()), + ) + }; + + encoder.writer().write(slices.0)?; + encoder.writer().write(slices.1)?; + } else { + for item in self.iter() { + item.encode(encoder)?; + } } + Ok(()) } - Ok(()) } } @@ -336,11 +356,20 @@ impl Decode for String { } impl_borrow_decode!(String); +// TODO +// String does not implement Into for Box because it allocates again +// we could do this manually with `Box::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl Decode for Box { fn decode(decoder: &mut D) -> Result { String::decode(decoder).map(String::into_boxed_str) } } + +// TODO +// String does not implement Into for Box because it allocates again +// we could do this manually with `Box::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl_borrow_decode!(Box); impl Encode for String { @@ -355,7 +384,11 @@ where { fn decode(decoder: &mut D) -> Result { let t = T::decode(decoder)?; - Ok(Box::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let b = Box::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let b = Box::new(t); + Ok(b) } } impl<'de, T> BorrowDecode<'de> for Box @@ -364,7 +397,11 @@ where { fn borrow_decode>(decoder: &mut D) -> Result { let t = T::borrow_decode(decoder)?; - Ok(Box::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let b = Box::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let b = Box::new(t); + Ok(b) } } @@ -377,16 +414,52 @@ where } } +#[cfg(feature = "unstable-strict-oom-checks")] impl Decode for Box<[T]> where T: Decode + 'static, { fn decode(decoder: &mut D) -> Result { - let vec = Vec::decode(decoder)?; - Ok(vec.into_boxed_slice()) + let len = decode_slice_len(decoder)?; + decoder.claim_container_read::(len)?; + + unsafe { + let mut result = Box::try_new_uninit_slice(len)?; + + let mut guard = DropGuard { + slice: &mut result, + idx: 0, + }; + + while guard.idx < len { + decoder.unclaim_bytes_read(core::mem::size_of::()); + let t = T::decode(decoder)?; + + guard.slice.get_unchecked_mut(guard.idx).write(t); + guard.idx += 1; + } + + core::mem::forget(guard); + Ok(result.assume_init()) + } } } +#[cfg(not(feature = "unstable-strict-oom-checks"))] +impl Decode for Box<[T]> +where + T: Decode, +{ + fn decode(decoder: &mut D) -> Result { + let vec = Vec::::decode(decoder)?; + Ok(vec.into()) + } +} + +// TODO +// Vec does not implement Into for Box<[T]> because it allocates again +// we could do this manually with `Box::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl<'de, T> BorrowDecode<'de> for Box<[T]> where T: BorrowDecode<'de> + 'de, @@ -447,7 +520,11 @@ where { fn decode(decoder: &mut D) -> Result { let t = T::decode(decoder)?; - Ok(Rc::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let rc = Rc::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let rc = Rc::new(t); + Ok(rc) } } @@ -457,7 +534,11 @@ where { fn borrow_decode>(decoder: &mut D) -> Result { let t = T::borrow_decode(decoder)?; - Ok(Rc::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let rc = Rc::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let rc = Rc::new(t); + Ok(rc) } } @@ -470,6 +551,10 @@ where } } +// TODO +// Vec does not implement Into for Rc<[T]> because it allocates again +// we could do this manually with `Rc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl Decode for Rc<[T]> where T: Decode + 'static, @@ -480,6 +565,10 @@ where } } +// TODO +// Vec does not implement Into for Rc<[T]> because it allocates again +// we could do this manually with `Rc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl<'de, T> BorrowDecode<'de> for Rc<[T]> where T: BorrowDecode<'de> + 'de, @@ -497,10 +586,18 @@ where { fn decode(decoder: &mut D) -> Result { let t = T::decode(decoder)?; - Ok(Arc::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let arc = Arc::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let arc = Arc::new(t); + Ok(arc) } } +// TODO +// String does not implement Into for Arc because it allocates again +// we could do this manually with `Arc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] #[cfg(target_has_atomic = "ptr")] impl Decode for Arc { fn decode(decoder: &mut D) -> Result { @@ -516,10 +613,18 @@ where { fn borrow_decode>(decoder: &mut D) -> Result { let t = T::borrow_decode(decoder)?; - Ok(Arc::new(t)) + #[cfg(feature = "unstable-strict-oom-checks")] + let arc = Arc::try_new(t)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let arc = Arc::new(t); + Ok(arc) } } +// TODO +// String does not implement Into for Arc because it allocates again +// we could do this manually with `Arc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] #[cfg(target_has_atomic = "ptr")] impl<'de> BorrowDecode<'de> for Arc { fn borrow_decode>(decoder: &mut D) -> Result { @@ -538,6 +643,10 @@ where } } +// TODO +// Vec does not implement Into for Arc<[T]> +// we could do this manually with `Arc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] #[cfg(target_has_atomic = "ptr")] impl Decode for Arc<[T]> where @@ -549,6 +658,10 @@ where } } +// TODO +// Vec does not implement Into for Arc<[T]> +// we could do this manually with `Arc::try_new_uninit` +#[cfg(not(feature = "unstable-strict-oom-checks"))] #[cfg(target_has_atomic = "ptr")] impl<'de, T> BorrowDecode<'de> for Arc<[T]> where @@ -559,3 +672,20 @@ where Ok(vec.into()) } } + +/// A drop guard that will trigger when an item fails to decode. +/// If an item at index n fails to decode, we have to properly drop the 0..(n-1) values that have been read. +struct DropGuard<'a, T> { + slice: &'a mut [core::mem::MaybeUninit], + idx: usize, +} + +impl<'a, T> Drop for DropGuard<'a, T> { + fn drop(&mut self) { + unsafe { + for item in &mut self.slice[..self.idx] { + core::ptr::drop_in_place(item as *mut core::mem::MaybeUninit as *mut T); + } + } + } +} diff --git a/src/features/impl_std.rs b/src/features/impl_std.rs index 04fae2f1..5f136ba1 100644 --- a/src/features/impl_std.rs +++ b/src/features/impl_std.rs @@ -432,7 +432,9 @@ where decoder.claim_container_read::<(K, V)>(len)?; let hash_builder: S = Default::default(); - let mut map = HashMap::with_capacity_and_hasher(len, hash_builder); + let mut map = HashMap::with_hasher(hash_builder); + map.try_reserve(len)?; + for _ in 0..len { // See the documentation on `unclaim_bytes_read` as to why we're doing this here decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); @@ -454,8 +456,9 @@ where let len = crate::de::decode_slice_len(decoder)?; decoder.claim_container_read::<(K, V)>(len)?; - let hash_builder: S = Default::default(); - let mut map = HashMap::with_capacity_and_hasher(len, hash_builder); + let mut map = HashMap::with_hasher(S::default()); + map.try_reserve(len)?; + for _ in 0..len { // See the documentation on `unclaim_bytes_read` as to why we're doing this here decoder.unclaim_bytes_read(core::mem::size_of::<(K, V)>()); diff --git a/src/lib.rs b/src/lib.rs index 3c225eee..0ca822ac 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,10 @@ #![no_std] #![warn(missing_docs, unused_lifetimes)] #![cfg_attr(docsrs, feature(doc_cfg))] +#![cfg_attr( + feature = "unstable-strict-oom-checks", + feature(allocator_api, new_uninit) +)] //! Bincode is a crate for encoding and decoding using a tiny binary //! serialization strategy. Using it, you can easily go from having From 07b27020027b7bcbe6d03a584f3c42ba979ef8b2 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Wed, 17 Aug 2022 12:48:14 +0200 Subject: [PATCH 02/13] Fixed CI issues --- src/error.rs | 9 ++------- src/features/impl_alloc.rs | 2 +- src/features/serde/de_borrowed.rs | 2 +- src/features/serde/de_owned.rs | 2 +- src/features/serde/mod.rs | 7 +++---- src/features/serde/ser.rs | 4 ++-- 6 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/error.rs b/src/error.rs index d3e51ec6..94539b89 100644 --- a/src/error.rs +++ b/src/error.rs @@ -214,12 +214,14 @@ pub enum OutOfMemory { Alloc(AllocError), } +#[cfg(feature = "alloc")] impl From for DecodeError { fn from(e: TryReserveError) -> Self { Self::OutOfMemory(OutOfMemory::TryReserve(e)) } } +#[cfg(feature = "alloc")] impl From for EncodeError { fn from(e: TryReserveError) -> Self { Self::OutOfMemory(OutOfMemory::TryReserve(e)) @@ -240,13 +242,6 @@ impl From for EncodeError { } } -#[cfg(all(feature = "alloc", feature = "unstable-strict-oom-checks"))] -impl From for DecodeError { - fn from(e: AllocError) -> Self { - Self::OutOfMemory(OutOfMemory::Alloc(e)) - } -} - impl core::fmt::Display for DecodeError { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { // TODO: Improve this? diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs index b4195260..1a1e7bcf 100644 --- a/src/features/impl_alloc.rs +++ b/src/features/impl_alloc.rs @@ -420,7 +420,7 @@ where T: Decode + 'static, { fn decode(decoder: &mut D) -> Result { - let len = decode_slice_len(decoder)?; + let len = crate::de::decode_slice_len(decoder)?; decoder.claim_container_read::(len)?; unsafe { diff --git a/src/features/serde/de_borrowed.rs b/src/features/serde/de_borrowed.rs index 093a1984..c52d30e4 100644 --- a/src/features/serde/de_borrowed.rs +++ b/src/features/serde/de_borrowed.rs @@ -442,7 +442,7 @@ impl<'de, 'a, DE: BorrowDecoder<'de>> EnumAccess<'de> for SerdeDecoder<'a, 'de, V: DeserializeSeed<'de>, { let idx = u32::decode(&mut self.de)?; - let val = seed.deserialize(idx.into_deserializer())?; + let val = seed.deserialize(serde::de::value::U32Deserializer::::new(idx))?; Ok((val, self)) } } diff --git a/src/features/serde/de_owned.rs b/src/features/serde/de_owned.rs index 502a92a5..ee7b002f 100644 --- a/src/features/serde/de_owned.rs +++ b/src/features/serde/de_owned.rs @@ -447,7 +447,7 @@ impl<'de, 'a, DE: Decoder> EnumAccess<'de> for SerdeDecoder<'a, DE> { V: DeserializeSeed<'de>, { let idx = u32::decode(&mut self.de)?; - let val = seed.deserialize(idx.into_deserializer())?; + let val = seed.deserialize(serde::de::value::U32Deserializer::::new(idx))?; Ok((val, self)) } } diff --git a/src/features/serde/mod.rs b/src/features/serde/mod.rs index 7293f476..d88a2106 100644 --- a/src/features/serde/mod.rs +++ b/src/features/serde/mod.rs @@ -139,10 +139,9 @@ pub enum EncodeError { CustomError, } -#[allow(clippy::from_over_into)] -impl Into for EncodeError { - fn into(self) -> crate::error::EncodeError { - crate::error::EncodeError::Serde(self) +impl From for crate::error::EncodeError { + fn from(val: EncodeError) -> Self { + crate::error::EncodeError::Serde(val) } } diff --git a/src/features/serde/ser.rs b/src/features/serde/ser.rs index edb122d9..48b1265a 100644 --- a/src/features/serde/ser.rs +++ b/src/features/serde/ser.rs @@ -218,7 +218,7 @@ where } fn serialize_seq(mut self, len: Option) -> Result { - let len = len.ok_or_else(|| SerdeEncodeError::SequenceMustHaveLength.into())?; + let len = len.ok_or(EncodeError::Serde(SerdeEncodeError::SequenceMustHaveLength))?; len.encode(&mut self.enc)?; Ok(Compound { enc: self.enc }) } @@ -247,7 +247,7 @@ where } fn serialize_map(mut self, len: Option) -> Result { - let len = len.ok_or_else(|| SerdeEncodeError::SequenceMustHaveLength.into())?; + let len = len.ok_or(EncodeError::Serde(SerdeEncodeError::SequenceMustHaveLength))?; len.encode(&mut self.enc)?; Ok(Compound { enc: self.enc }) } From 1d2a0b05655caf414c30fb5621db359bdf209857 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Thu, 18 Aug 2022 20:47:32 +0200 Subject: [PATCH 03/13] Moved OOM to it's own CI file and added a MIRI step --- .github/workflows/rust.yml | 26 ------------ .github/workflows/strict_oom.yml | 70 ++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 26 deletions(-) create mode 100644 .github/workflows/strict_oom.yml diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 5d958b37..3f274537 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -128,32 +128,6 @@ fi", } ] }, - "no_oom": { - "name": "Strict OOM checks", - "runs-on": "ubuntu-latest", - "steps": [ - { - "uses": "actions/checkout@v2", - "name": "Checkout" - }, - { - "uses": "actions-rs/toolchain@v1", - "with": { - "profile": "minimal", - "toolchain": "nightly", - "components": "rust-src", - "override": true - }, - "name": "Install Rust nightly" - }, - { - "run": "cargo build --no-default-features --features unstable-strict-oom-checks -Z build-std=core,alloc --target x86_64-unknown-linux-gnu", - "env": { - "RUSTFLAGS": "--cfg no_global_oom_handling" - } - } - ] - }, "lints": { "name": "Lints", "runs-on": "ubuntu-latest", diff --git a/.github/workflows/strict_oom.yml b/.github/workflows/strict_oom.yml new file mode 100644 index 00000000..aaac6a2d --- /dev/null +++ b/.github/workflows/strict_oom.yml @@ -0,0 +1,70 @@ +{ + "name": "strict OOM", + "on": { + "push": { + "branches": [ + "trunk", + "v*.x", + "ci/*" + ] + }, + "pull_request": { + "branches": [ + "trunk", + "v*.x" + ] + } + }, + "jobs": { + "no_oom": { + "name": "Strict OOM checks", + "runs-on": "ubuntu-latest", + "steps": [ + { + "uses": "actions/checkout@v2", + "name": "Checkout" + }, + { + "uses": "actions-rs/toolchain@v1", + "with": { + "profile": "minimal", + "toolchain": "nightly", + "components": "rust-src", + "override": true + }, + "name": "Install Rust nightly" + }, + { + "run": "cargo build --no-default-features --features unstable-strict-oom-checks -Z build-std=core,alloc --target x86_64-unknown-linux-gnu", + "env": { + "RUSTFLAGS": "--cfg no_global_oom_handling" + } + } + ] + }, + "miri": { + "name": "MIRI", + "runs-on": "ubuntu-latest", + "steps": [ + { + "uses": "actions/checkout@v2", + "name": "Checkout" + }, + { + "run": "rustup toolchain install nightly --component miri +rustup override set nightly +cargo miri setup", + "name": "Install Rust nightly" + }, + { + "run": "cargo miri test", + "name": "Default features" + }, + { + "run": "cargo miri test --no-default-features --features unstable-strict-oom-checks", + "name": "Strict OOM check" + } + ] + } + } +} From 740c5eeb4547a98d40833f49fd06b912bf8e3130 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Thu, 18 Aug 2022 20:48:17 +0200 Subject: [PATCH 04/13] Fixed clippy lint --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index 94539b89..4a336d21 100644 --- a/src/error.rs +++ b/src/error.rs @@ -205,7 +205,7 @@ pub enum DecodeError { /// A wrapper to make all the out of memory errors consistent #[cfg(feature = "alloc")] -#[derive(Debug, PartialEq)] +#[derive(Debug)] pub enum OutOfMemory { /// Failed to reserve an entry TryReserve(TryReserveError), From cfcc4f8747d08400fbcd16e9a361769c4fcf36f2 Mon Sep 17 00:00:00 2001 From: victor koenders Date: Fri, 19 Aug 2022 11:43:06 +0200 Subject: [PATCH 05/13] Attempt at fixing MIRI CI --- .github/workflows/strict_oom.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/strict_oom.yml b/.github/workflows/strict_oom.yml index aaac6a2d..b9d0348a 100644 --- a/.github/workflows/strict_oom.yml +++ b/.github/workflows/strict_oom.yml @@ -51,8 +51,8 @@ "name": "Checkout" }, { - "run": "rustup toolchain install nightly --component miri -rustup override set nightly + "run": "rustup toolchain install nightly --component miri \n +rustup override set nightly \n cargo miri setup", "name": "Install Rust nightly" }, From 93c02c14a137a2d174d8c000a8f32b60fde3e5e0 Mon Sep 17 00:00:00 2001 From: victor koenders Date: Fri, 19 Aug 2022 12:57:02 +0200 Subject: [PATCH 06/13] Made miri ignore tempfile::tempfile() --- tests/std.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/std.rs b/tests/std.rs index 973c75d1..8ebf8f7b 100644 --- a/tests/std.rs +++ b/tests/std.rs @@ -53,7 +53,11 @@ fn test_std_cursor() { #[test] fn test_std_file() { - let mut file = tempfile::tempfile().expect("Could not create temp file"); + #[cfg_attr(miri, ignore)] + fn create_temp_file() -> std::fs::File { + tempfile::tempfile().expect("Could not create temp file") + } + let mut file = create_temp_file(); let bytes_written = bincode::encode_into_std_write( Foo { a: 30, b: 50 }, From 2b4677aefc78872e349c9ea24c5e3a3f68552e12 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Sun, 2 Oct 2022 11:06:43 +0200 Subject: [PATCH 07/13] Disabled test_std_file when running on miri as ignoring the tempfile did not fix the runtime error --- tests/std.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/std.rs b/tests/std.rs index 8ebf8f7b..9d395b5e 100644 --- a/tests/std.rs +++ b/tests/std.rs @@ -51,13 +51,10 @@ fn test_std_cursor() { assert_eq!(foo.b, 10); } +#[cfg(not(feature = "unstable-strict-oom-checks"))] #[test] fn test_std_file() { - #[cfg_attr(miri, ignore)] - fn create_temp_file() -> std::fs::File { - tempfile::tempfile().expect("Could not create temp file") - } - let mut file = create_temp_file(); + let mut file = tempfile::tempfile().expect("Could not create temp file"); let bytes_written = bincode::encode_into_std_write( Foo { a: 30, b: 50 }, From c96ac68ab9675979daaa05495143b6e1b52635cc Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Sun, 2 Oct 2022 11:29:26 +0200 Subject: [PATCH 08/13] Disabled test_std_file proper now (I hope) --- tests/std.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/std.rs b/tests/std.rs index 9d395b5e..2400b8ce 100644 --- a/tests/std.rs +++ b/tests/std.rs @@ -51,7 +51,8 @@ fn test_std_cursor() { assert_eq!(foo.b, 10); } -#[cfg(not(feature = "unstable-strict-oom-checks"))] +// miri seems to not be able to deal with `tempfile::tempfile()` +#[cfg_attr(miri, ignore)] #[test] fn test_std_file() { let mut file = tempfile::tempfile().expect("Could not create temp file"); From 680ca6d9147669ccd7fccfff61798be4eaaf154d Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Sun, 2 Oct 2022 11:54:03 +0200 Subject: [PATCH 09/13] Fixed warning and errors while running `unstable-strict-oom-checks` --- tests/alloc.rs | 26 +++++++++++++++++++++----- tests/basic_types.rs | 1 - 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/tests/alloc.rs b/tests/alloc.rs index 08c07bb0..afb6419b 100644 --- a/tests/alloc.rs +++ b/tests/alloc.rs @@ -6,12 +6,19 @@ extern crate alloc; mod utils; use alloc::borrow::Cow; +#[cfg(not(feature = "unstable-strict-oom-checks"))] use alloc::collections::*; -#[cfg(not(feature = "serde"))] +#[cfg(all(not(feature = "serde"), not(feature = "unstable-strict-oom-checks")))] use alloc::rc::Rc; -#[cfg(all(target_has_atomic = "ptr", not(feature = "serde")))] +#[cfg(all( + target_has_atomic = "ptr", + not(feature = "serde"), + not(feature = "unstable-strict-oom-checks") +))] use alloc::sync::Arc; -use utils::{the_same, the_same_with_comparer}; +use utils::the_same; +#[cfg(not(feature = "unstable-strict-oom-checks"))] +use utils::the_same_with_comparer; struct Foo { pub a: u32, @@ -85,7 +92,7 @@ fn test_alloc_commons() { the_same(Box::<[u32]>::from(vec![1, 2, 3, 4, 5])); the_same(Cow::::Owned(5)); the_same(Cow::::Borrowed(&5)); - #[cfg(not(feature = "serde"))] + #[cfg(all(not(feature = "serde"), not(feature = "unstable-strict-oom-checks")))] { // Serde doesn't support Rc or Arc the_same(Rc::::new(5)); @@ -98,6 +105,7 @@ fn test_alloc_commons() { } } + #[cfg(not(feature = "unstable-strict-oom-checks"))] the_same_with_comparer( { let mut map = BinaryHeap::::new(); @@ -110,16 +118,19 @@ fn test_alloc_commons() { }, |a, b| a.iter().collect::>() == b.iter().collect::>(), ); + #[cfg(not(feature = "unstable-strict-oom-checks"))] the_same({ let mut map = BTreeMap::::new(); map.insert(5, -5); map }); + #[cfg(not(feature = "unstable-strict-oom-checks"))] the_same({ let mut set = BTreeSet::::new(); set.insert(5); set }); + #[cfg(not(feature = "unstable-strict-oom-checks"))] the_same({ let mut set = VecDeque::::new(); set.push_back(15); @@ -161,9 +172,13 @@ fn test_container_limits() { } for slice in test_cases { + #[cfg(not(feature = "unstable-strict-oom-checks"))] validate_fail::>(slice); + #[cfg(not(feature = "unstable-strict-oom-checks"))] validate_fail::>(slice); + #[cfg(not(feature = "unstable-strict-oom-checks"))] validate_fail::>(slice); + #[cfg(not(feature = "unstable-strict-oom-checks"))] validate_fail::>(slice); validate_fail::>(slice); validate_fail::(slice); @@ -175,7 +190,7 @@ fn test_container_limits() { } } -#[cfg(target_has_atomic = "ptr")] +#[cfg(all(target_has_atomic = "ptr", not(feature = "unstable-strict-oom-checks")))] #[test] fn test_arc_str() { use alloc::sync::Arc; @@ -188,6 +203,7 @@ fn test_arc_str() { let start: Arc = Arc::clone(&start); bincode::encode_into_slice(start, &mut target, config).unwrap() }; + let slice = &target[..len]; let decoded: Arc = bincode::borrow_decode_from_slice(slice, config).unwrap().0; diff --git a/tests/basic_types.rs b/tests/basic_types.rs index 0aae093c..b59313ea 100644 --- a/tests/basic_types.rs +++ b/tests/basic_types.rs @@ -25,7 +25,6 @@ fn test_numbers() { the_same(5i128); the_same(5isize); - println!("Test {:?}", 5.0f32); the_same_with_comparer(5.0f32, |a, b| (a - b).abs() <= f32::EPSILON); the_same_with_comparer(5.0f64, |a, b| (a - b).abs() <= f64::EPSILON); From 36eea25675dbefc08e0eef8fd44ea37c29cd4d10 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Thu, 30 Mar 2023 13:24:59 +0200 Subject: [PATCH 10/13] Small improvements, added documentation about the new feature --- src/features/impl_alloc.rs | 46 +------------------------------------- src/lib.rs | 15 +++++++------ 2 files changed, 9 insertions(+), 52 deletions(-) diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs index 1a1e7bcf..56ea4147 100644 --- a/src/features/impl_alloc.rs +++ b/src/features/impl_alloc.rs @@ -42,20 +42,8 @@ impl enc::write::Writer for VecWriter { #[inline(always)] fn write(&mut self, bytes: &[u8]) -> Result<(), EncodeError> { self.inner.try_reserve(bytes.len())?; + self.inner.extend_from_slice(bytes); - let start = self.inner.len(); - - // Get a slice of `&mut [MaybeUninit]` of the remaining capacity - let remaining = &mut self.inner.spare_capacity_mut()[..bytes.len()]; - for (i, b) in bytes.iter().copied().enumerate() { - // TODO: is there a better way to copy from `&mut [MaybeUninit]` to `&[u8]`? - remaining[i].write(b); - } - - unsafe { - // Safety: We reserved enough bytes, and the bytes have values written to them - self.inner.set_len(start + bytes.len()); - } Ok(()) } } @@ -414,41 +402,9 @@ where } } -#[cfg(feature = "unstable-strict-oom-checks")] impl Decode for Box<[T]> where T: Decode + 'static, -{ - fn decode(decoder: &mut D) -> Result { - let len = crate::de::decode_slice_len(decoder)?; - decoder.claim_container_read::(len)?; - - unsafe { - let mut result = Box::try_new_uninit_slice(len)?; - - let mut guard = DropGuard { - slice: &mut result, - idx: 0, - }; - - while guard.idx < len { - decoder.unclaim_bytes_read(core::mem::size_of::()); - let t = T::decode(decoder)?; - - guard.slice.get_unchecked_mut(guard.idx).write(t); - guard.idx += 1; - } - - core::mem::forget(guard); - Ok(result.assume_init()) - } - } -} - -#[cfg(not(feature = "unstable-strict-oom-checks"))] -impl Decode for Box<[T]> -where - T: Decode, { fn decode(decoder: &mut D) -> Result { let vec = Vec::::decode(decoder)?; diff --git a/src/lib.rs b/src/lib.rs index 0ca822ac..2ac669cb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,13 +19,14 @@ //! //! # Features //! -//! |Name |Default?|Supported types for Encode/Decode|Enabled methods |Other| -//! |------|--------|-----------------------------------------|-----------------------------------------------------------------|-----| -//! |std | Yes |`HashMap` and `HashSet`|`decode_from_std_read` and `encode_into_std_write`| -//! |alloc | Yes |All common containers in alloc, like `Vec`, `String`, `Box`|`encode_to_vec`| -//! |atomic| Yes |All `Atomic*` integer types, e.g. `AtomicUsize`, and `AtomicBool`|| -//! |derive| Yes |||Enables the `BorrowDecode`, `Decode` and `Encode` derive macros| -//! |serde | No |`Compat` and `BorrowCompat`, which will work for all types that implement serde's traits|serde-specific encode/decode functions in the [serde] module|Note: There are several [known issues](serde/index.html#known-issues) when using serde and bincode| +//! |Name |Default?|Supported types for Encode/Decode|Enabled methods |Other| +//! |--------------------------|--------|-----------------------------------------|-----------------------------------------------------------------|-----| +//! |std | Yes |`HashMap` and `HashSet`|`decode_from_std_read` and `encode_into_std_write`| +//! |alloc | Yes |All common containers in alloc, like `Vec`, `String`, `Box`|`encode_to_vec`| +//! |atomic | Yes |All `Atomic*` integer types, e.g. `AtomicUsize`, and `AtomicBool`|| +//! |derive | Yes |||Enables the `BorrowDecode`, `Decode` and `Encode` derive macros| +//! |serde | No |`Compat` and `BorrowCompat`, which will work for all types that implement serde's traits|serde-specific encode/decode functions in the [serde] module|Note: There are several [known issues](serde/index.html#known-issues) when using serde and bincode| +//! |unstable-strict-oom-checks| No |||Enabled strict OOM checks which ensures that bincode will never cause OOM issues.
This requires nightly feature so you need to use a nightly compiler.
This may break or be changed at any point in the future| //! //! # Which functions to use //! From 735e17566bf3001adf89c15d8781e817003f5ac7 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Tue, 19 Sep 2023 18:35:59 +0200 Subject: [PATCH 11/13] Fixed a possible fallible allocation issue --- src/features/impl_alloc.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs index 56ea4147..2a37ce72 100644 --- a/src/features/impl_alloc.rs +++ b/src/features/impl_alloc.rs @@ -26,10 +26,10 @@ pub(crate) struct VecWriter { impl VecWriter { /// Create a new vec writer with the given capacity - pub fn with_capacity(cap: usize) -> Self { - Self { - inner: Vec::with_capacity(cap), - } + pub fn with_capacity(cap: usize) -> Result { + let mut inner = Vec::new(); + inner.try_reserve(cap)?; + Ok(Self { inner }) } // May not be used in all feature combinations #[allow(dead_code)] @@ -58,7 +58,7 @@ pub fn encode_to_vec(val: E, config: C) -> Result::new(writer, config); val.encode(&mut encoder)?; Ok(encoder.into_writer().inner) @@ -402,6 +402,8 @@ where } } +// TODO +// Vec does not implement Into for Box<[T]> because it allocates again impl Decode for Box<[T]> where T: Decode + 'static, @@ -414,7 +416,6 @@ where // TODO // Vec does not implement Into for Box<[T]> because it allocates again -// we could do this manually with `Box::try_new_uninit` #[cfg(not(feature = "unstable-strict-oom-checks"))] impl<'de, T> BorrowDecode<'de> for Box<[T]> where From e44d7c150a0fc00509592b8b6c77f12e19a53282 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Sun, 17 Mar 2024 19:28:50 +0100 Subject: [PATCH 12/13] Fixed some warnings --- src/features/impl_alloc.rs | 4 +++- src/lib.rs | 1 - src/varint/decode_unsigned.rs | 3 +-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs index 2a37ce72..3d65671d 100644 --- a/src/features/impl_alloc.rs +++ b/src/features/impl_alloc.rs @@ -266,7 +266,7 @@ where if unty::type_equal::() { decoder.claim_container_read::(len)?; // optimize for reading u8 vecs - let mut vec = alloc::vec![0u8; len]; + let mut vec = alloc::vec::from_elem(0u8, len); decoder.reader().read(&mut vec)?; // Safety: Vec is Vec Ok(unsafe { core::mem::transmute(vec) }) @@ -630,6 +630,7 @@ where } } +#[cfg(not(feature = "unstable-strict-oom-checks"))] /// A drop guard that will trigger when an item fails to decode. /// If an item at index n fails to decode, we have to properly drop the 0..(n-1) values that have been read. struct DropGuard<'a, T> { @@ -637,6 +638,7 @@ struct DropGuard<'a, T> { idx: usize, } +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl<'a, T> Drop for DropGuard<'a, T> { fn drop(&mut self) { unsafe { diff --git a/src/lib.rs b/src/lib.rs index 2ac669cb..db8b59a8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -100,7 +100,6 @@ pub mod de; pub mod enc; pub mod error; -pub use atomic::*; pub use de::{BorrowDecode, Decode}; pub use enc::Encode; diff --git a/src/varint/decode_unsigned.rs b/src/varint/decode_unsigned.rs index de0f0b95..ea2091d6 100644 --- a/src/varint/decode_unsigned.rs +++ b/src/varint/decode_unsigned.rs @@ -1,11 +1,10 @@ -use core::{convert::TryInto, u32}; - use super::{SINGLE_BYTE_MAX, U128_BYTE, U16_BYTE, U32_BYTE, U64_BYTE}; use crate::{ config::Endianness, de::read::Reader, error::{DecodeError, IntegerType}, }; +use core::u32; #[inline(never)] #[cold] From c4e11a1e7a4a20917363573256a389501cfefc61 Mon Sep 17 00:00:00 2001 From: Victor Koenders Date: Sun, 17 Mar 2024 19:51:43 +0100 Subject: [PATCH 13/13] Fixed compilation issues --- src/features/impl_alloc.rs | 81 +++++++++++++++++++++++++++----------- src/lib.rs | 8 +++- 2 files changed, 66 insertions(+), 23 deletions(-) diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs index 3d65671d..ea2d2ea8 100644 --- a/src/features/impl_alloc.rs +++ b/src/features/impl_alloc.rs @@ -1,5 +1,5 @@ use crate::{ - de::{read::Reader, BorrowDecoder, Decode, Decoder}, + de::{BorrowDecoder, Decode, Decoder}, enc::{ self, write::{SizeWriter, Writer}, @@ -16,6 +16,9 @@ use alloc::{ vec::Vec, }; +#[cfg(not(feature = "unstable-strict-oom-checks"))] +use crate::de::read::Reader; + #[cfg(target_has_atomic = "ptr")] use alloc::sync::Arc; @@ -38,6 +41,7 @@ impl VecWriter { } } +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl enc::write::Writer for VecWriter { #[inline(always)] fn write(&mut self, bytes: &[u8]) -> Result<(), EncodeError> { @@ -48,6 +52,21 @@ impl enc::write::Writer for VecWriter { } } +#[cfg(feature = "unstable-strict-oom-checks")] +impl enc::write::Writer for VecWriter { + #[inline(always)] + fn write(&mut self, bytes: &[u8]) -> Result<(), EncodeError> { + self.inner.try_reserve(bytes.len())?; + unsafe { + // Safety: We just reserved `bytes.len()` additional bytes + core::mem::MaybeUninit::copy_from_slice(self.inner.spare_capacity_mut(), bytes); + self.inner.set_len(self.inner.len() + bytes.len()); + } + + Ok(()) + } +} + /// Encode the given value into a `Vec` with the given `Config`. See the [config] module for more information. /// /// [config]: config/index.html @@ -263,25 +282,34 @@ where fn decode(decoder: &mut D) -> Result { let len = crate::de::decode_slice_len(decoder)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] if unty::type_equal::() { decoder.claim_container_read::(len)?; // optimize for reading u8 vecs let mut vec = alloc::vec::from_elem(0u8, len); decoder.reader().read(&mut vec)?; // Safety: Vec is Vec - Ok(unsafe { core::mem::transmute(vec) }) - } else { - decoder.claim_container_read::(len)?; + return Ok(unsafe { core::mem::transmute(vec) }); + } - let mut vec = Vec::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); + decoder.claim_container_read::(len)?; - vec.push(T::decode(decoder)?); - } - Ok(vec) + #[cfg(feature = "unstable-strict-oom-checks")] + let mut vec = Vec::try_with_capacity(len)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let mut vec = Vec::with_capacity(len); + + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); + + // Should never fail because we allocated enough capacity + #[cfg(feature = "unstable-strict-oom-checks")] + debug_assert!(vec.push_within_capacity(T::decode(decoder)?).is_ok()); + #[cfg(not(feature = "unstable-strict-oom-checks"))] + vec.push(T::decode(decoder)?); } + Ok(vec) } } @@ -292,25 +320,33 @@ where fn borrow_decode>(decoder: &mut D) -> Result { let len = crate::de::decode_slice_len(decoder)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] if unty::type_equal::() { decoder.claim_container_read::(len)?; // optimize for reading u8 vecs - let mut vec = alloc::vec![0u8; len]; + let mut vec = alloc::vec::from_elem(0u8, len); decoder.reader().read(&mut vec)?; // Safety: Vec is Vec - Ok(unsafe { core::mem::transmute(vec) }) - } else { - decoder.claim_container_read::(len)?; + return Ok(unsafe { core::mem::transmute(vec) }); + } + decoder.claim_container_read::(len)?; - let mut vec = Vec::with_capacity(len); - for _ in 0..len { - // See the documentation on `unclaim_bytes_read` as to why we're doing this here - decoder.unclaim_bytes_read(core::mem::size_of::()); + #[cfg(feature = "unstable-strict-oom-checks")] + let mut vec = Vec::try_with_capacity(len)?; + #[cfg(not(feature = "unstable-strict-oom-checks"))] + let mut vec = Vec::with_capacity(len); - vec.push(T::borrow_decode(decoder)?); - } - Ok(vec) + for _ in 0..len { + // See the documentation on `unclaim_bytes_read` as to why we're doing this here + decoder.unclaim_bytes_read(core::mem::size_of::()); + + // Should never fail because we allocated enough capacity + #[cfg(feature = "unstable-strict-oom-checks")] + debug_assert!(vec.push_within_capacity(T::borrow_decode(decoder)?).is_ok()); + #[cfg(not(feature = "unstable-strict-oom-checks"))] + vec.push(T::borrow_decode(decoder)?); } + Ok(vec) } } @@ -404,6 +440,7 @@ where // TODO // Vec does not implement Into for Box<[T]> because it allocates again +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl Decode for Box<[T]> where T: Decode + 'static, diff --git a/src/lib.rs b/src/lib.rs index db8b59a8..fab25dd0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,7 +3,13 @@ #![cfg_attr(docsrs, feature(doc_cfg))] #![cfg_attr( feature = "unstable-strict-oom-checks", - feature(allocator_api, new_uninit) + feature( + allocator_api, + new_uninit, + maybe_uninit_write_slice, + vec_push_within_capacity, + try_with_capacity + ) )] //! Bincode is a crate for encoding and decoding using a tiny binary