diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 53af7bb0..3f274537 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`" }, @@ -158,7 +158,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 +213,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/.github/workflows/strict_oom.yml b/.github/workflows/strict_oom.yml new file mode 100644 index 00000000..b9d0348a --- /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 \n +rustup override set nightly \n +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" + } + ] + } + } +} 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..4a336d21 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,49 @@ 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)] +pub enum OutOfMemory { + /// Failed to reserve an entry + TryReserve(TryReserveError), + #[cfg(feature = "unstable-strict-oom-checks")] + /// Failed to allocate memory + 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)) + } +} + +#[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)) + } } impl core::fmt::Display for DecodeError { diff --git a/src/features/impl_alloc.rs b/src/features/impl_alloc.rs index f5186a76..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}, @@ -11,12 +11,14 @@ use crate::{ use alloc::{ borrow::{Cow, ToOwned}, boxed::Box, - collections::*, rc::Rc, string::String, vec::Vec, }; +#[cfg(not(feature = "unstable-strict-oom-checks"))] +use crate::de::read::Reader; + #[cfg(target_has_atomic = "ptr")] use alloc::sync::Arc; @@ -27,10 +29,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)] @@ -39,10 +41,28 @@ 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> { + self.inner.try_reserve(bytes.len())?; self.inner.extend_from_slice(bytes); + + Ok(()) + } +} + +#[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(()) } } @@ -57,194 +77,201 @@ pub fn encode_to_vec(val: E, config: C) -> Result::new(writer, config); val.encode(&mut encoder)?; Ok(encoder.into_writer().inner) } -impl 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(()) } } @@ -255,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![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) }); + } - 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) } } @@ -284,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) } } @@ -336,11 +380,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 +408,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 +421,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 +438,22 @@ 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, { fn decode(decoder: &mut D) -> Result { - let vec = Vec::decode(decoder)?; - Ok(vec.into_boxed_slice()) + let vec = Vec::::decode(decoder)?; + Ok(vec.into()) } } +// TODO +// Vec does not implement Into for Box<[T]> because it allocates again +#[cfg(not(feature = "unstable-strict-oom-checks"))] impl<'de, T> BorrowDecode<'de> for Box<[T]> where T: BorrowDecode<'de> + 'de, @@ -447,7 +514,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 +528,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 +545,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 +559,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 +580,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 +607,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 +637,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 +652,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 +666,22 @@ where Ok(vec.into()) } } + +#[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> { + slice: &'a mut [core::mem::MaybeUninit], + idx: usize, +} + +#[cfg(not(feature = "unstable-strict-oom-checks"))] +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/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 }) } diff --git a/src/lib.rs b/src/lib.rs index 3c225eee..fab25dd0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,16 @@ #![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, + maybe_uninit_write_slice, + vec_push_within_capacity, + try_with_capacity + ) +)] //! Bincode is a crate for encoding and decoding using a tiny binary //! serialization strategy. Using it, you can easily go from having @@ -15,13 +25,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 //! @@ -95,7 +106,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] 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); diff --git a/tests/std.rs b/tests/std.rs index 973c75d1..2400b8ce 100644 --- a/tests/std.rs +++ b/tests/std.rs @@ -51,6 +51,8 @@ fn test_std_cursor() { assert_eq!(foo.b, 10); } +// 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");