From a8fe89c87a0cf03ee6f3cea9202c42809c6840ff Mon Sep 17 00:00:00 2001 From: bendn Date: Wed, 6 Sep 2023 11:21:32 +0700 Subject: [PATCH] fix unsoundness --- Cargo.toml | 2 +- benches/affine_transformations.rs | 7 +-- benches/overlays.rs | 30 +++------- src/builder.rs | 94 +++++++++++++++++++++++++++++++ src/lib.rs | 58 +++++++++++++------ src/overlay.rs | 2 + 6 files changed, 148 insertions(+), 45 deletions(-) create mode 100644 src/builder.rs diff --git a/Cargo.toml b/Cargo.toml index e7e869b..60b0735 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "fimg" -version = "0.2.2" +version = "0.3.0" authors = ["bend-n "] license = "MIT" edition = "2021" diff --git a/benches/affine_transformations.rs b/benches/affine_transformations.rs index 1b38428..de2162a 100644 --- a/benches/affine_transformations.rs +++ b/benches/affine_transformations.rs @@ -7,11 +7,8 @@ macro_rules! bench { (fn $name: ident() { run $fn: ident() }) => { #[bench] fn $name(b: &mut Bencher) { - let mut img: Image<_, 4> = Image::new( - 64.try_into().unwrap(), - 64.try_into().unwrap(), - include_bytes!("4_180x180.imgbuf").to_vec(), - ); + let mut img: Image<_, 4> = + Image::build(64, 64).buf(include_bytes!("4_180x180.imgbuf").to_vec()); #[allow(unused_unsafe)] b.iter(|| unsafe { for _ in 0..256 { diff --git a/benches/overlays.rs b/benches/overlays.rs index 63af444..b1ee3f3 100644 --- a/benches/overlays.rs +++ b/benches/overlays.rs @@ -6,16 +6,8 @@ use test::Bencher; #[bench] fn overlay_3on3at(bench: &mut Bencher) { let mut v = vec![0u8; 3 * 64 * 64]; - let mut a: Image<_, 3> = Image::new( - 64.try_into().unwrap(), - 64.try_into().unwrap(), - v.as_mut_slice(), - ); - let b = Image::<&[u8], 3>::new( - 4.try_into().unwrap(), - 4.try_into().unwrap(), - *&include_bytes!("3_4x4.imgbuf"), - ); + let mut a: Image<_, 3> = Image::build(64, 64).buf(v.as_mut_slice()); + let b: Image<&[u8], 3> = Image::build(4, 4).buf(include_bytes!("3_4x4.imgbuf")); bench.iter(|| unsafe { for x in 0..16 { for y in 0..16 { @@ -23,17 +15,13 @@ fn overlay_3on3at(bench: &mut Bencher) { } } }); - assert_eq!(a.as_ref().buffer, include_bytes!("3x3_at_out.imgbuf")); + assert_eq!(a.as_ref().buffer(), include_bytes!("3x3_at_out.imgbuf")); } #[bench] fn overlay_4on3at(bench: &mut Bencher) { let mut a: Image<_, 3> = Image::alloc(64, 64); - let b = Image::<&[u8], 4>::new( - 4.try_into().unwrap(), - 4.try_into().unwrap(), - *&include_bytes!("4_4x4.imgbuf"), - ); + let b: Image<&[u8], 4> = Image::build(4, 4).buf(include_bytes!("4_4x4.imgbuf")); bench.iter(|| unsafe { for x in 0..16 { for y in 0..16 { @@ -41,17 +29,13 @@ fn overlay_4on3at(bench: &mut Bencher) { } } }); - assert_eq!(a.as_ref().buffer, include_bytes!("4x3_at_out.imgbuf")); + assert_eq!(a.as_ref().buffer(), include_bytes!("4x3_at_out.imgbuf")); } #[bench] fn overlay_4on4at(bench: &mut Bencher) { let mut a: Image<_, 4> = Image::alloc(64, 64); - let b = Image::<&[u8], 4>::new( - 4.try_into().unwrap(), - 4.try_into().unwrap(), - *&include_bytes!("4_4x4.imgbuf"), - ); + let b: Image<&[u8], 4> = Image::build(4, 4).buf(include_bytes!("4_4x4.imgbuf")); bench.iter(|| unsafe { for x in 0..16 { for y in 0..16 { @@ -59,5 +43,5 @@ fn overlay_4on4at(bench: &mut Bencher) { } } }); - assert_eq!(a.as_ref().buffer, include_bytes!("4x4_at_out.imgbuf")); + assert_eq!(a.as_ref().buffer(), include_bytes!("4x4_at_out.imgbuf")); } diff --git a/src/builder.rs b/src/builder.rs new file mode 100644 index 0000000..1f9b880 --- /dev/null +++ b/src/builder.rs @@ -0,0 +1,94 @@ +//! safe builder for the image +//! +//! does not let you do funny things +use std::marker::PhantomData; + +use crate::Image; + +impl Image { + /// creates a builder + pub const fn build(w: u32, h: u32) -> Builder { + Builder::new(w, h) + } +} + +/// Safe [Image] builder. +pub struct Builder { + /// the width in a zeroable type. zeroable so as to make the check in [`buf`] easier. + width: u32, + /// the height in a zeroable type. + height: u32, + #[allow(clippy::missing_docs_in_private_items)] + _buffer: PhantomData, +} +impl Builder { + /// create new builder + pub const fn new(w: u32, h: u32) -> Self { + Self { + width: w, + height: h, + _buffer: PhantomData, + } + } + + /// apply a buffer, and build + pub fn buf(self, buffer: B) -> Image { + if buffer.len() as u32 != C as u32 * self.width * self.height { + panic!("invalid buffer size"); + } + Image { + buffer, + width: self.width.try_into().expect("passed zero width to builder"), + height: self + .height + .try_into() + .expect("passed zero height to builder"), + } + } +} + +impl Builder, C> { + /// allocate this image + pub fn alloc(self) -> Image, C> { + Image::alloc(self.width, self.height) + } +} + +/// seals the [`Buffer`] trait +mod buf { + /// A valid buffer for use in the builder + pub trait Buffer { + #[doc(hidden)] + fn len(&self) -> usize; + } + impl Buffer for Vec { + fn len(&self) -> usize { + self.len() + } + } + impl Buffer for &[T] { + fn len(&self) -> usize { + <[T]>::len(self) + } + } + impl Buffer for &mut [T] { + fn len(&self) -> usize { + <[T]>::len(self) + } + } + impl Buffer for [T; N] { + fn len(&self) -> usize { + N + } + } + impl Buffer for &[T; N] { + fn len(&self) -> usize { + N + } + } + impl Buffer for &mut [T; N] { + fn len(&self) -> usize { + N + } + } +} diff --git a/src/lib.rs b/src/lib.rs index a760a88..725945d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,7 @@ use std::{num::NonZeroU32, slice::SliceIndex}; mod affine; +pub mod builder; mod overlay; pub use overlay::{Overlay, OverlayAt}; @@ -77,11 +78,11 @@ unsafe fn really_unsafe_index(x: u32, y: u32, w: u32) -> usize { #[derive(Clone, Debug, PartialEq, Eq)] pub struct Image { /// column order 2d slice/vec - pub buffer: T, + buffer: T, /// image horizontal size - pub width: NonZeroU32, + width: NonZeroU32, /// image vertical size - pub height: NonZeroU32, + height: NonZeroU32, } impl Default for Image<&'static [u8], CHANNELS> { @@ -109,13 +110,33 @@ impl Image { #[inline] /// create a new image - pub const fn new(width: NonZeroU32, height: NonZeroU32, buffer: T) -> Self { + /// + /// # Safety + /// + /// does not check that buffer.len() == w * h * C + /// + /// using this with invalid values may result in future UB + pub const unsafe fn new(width: NonZeroU32, height: NonZeroU32, buffer: T) -> Self { Self { buffer, width, height, } } + + /// returns a immutable reference to the backing buffer + pub const fn buffer(&self) -> &T { + &self.buffer + } + + /// returns a mutable(!) reference to the backing buffer + /// + /// # Safety + /// + /// please do not change buffer size. + pub unsafe fn buffer_mut(&mut self) -> &mut T { + &mut self.buffer + } } impl Image<&[u8], CHANNELS> { @@ -216,28 +237,38 @@ impl, const CHANNELS: usize> Image Image<&mut [u8], CHANNELS> { /// Downcast the mutable reference pub fn as_ref(&self) -> Image<&[u8], CHANNELS> { - Image::new(self.width, self.height, self.buffer) + // SAFETY: we got constructed okay, parameters must be valid + unsafe { Image::new(self.width, self.height, self.buffer) } } } impl Image<&mut [u8], CHANNELS> { /// Copy this ref image pub fn copy(&mut self) -> Image<&mut [u8], CHANNELS> { - Image::new(self.width, self.height, self.buffer) + #[allow(clippy::undocumented_unsafe_blocks)] + unsafe { + Image::new(self.width, self.height, self.buffer) + } } } impl Image, CHANNELS> { /// Create a reference to this owned image pub fn as_ref(&self) -> Image<&[u8], CHANNELS> { - Image::new(self.width, self.height, &self.buffer) + #[allow(clippy::undocumented_unsafe_blocks)] + unsafe { + Image::new(self.width, self.height, &self.buffer) + } } } impl Image, CHANNELS> { /// Create a mutable reference to this owned image pub fn as_mut(&mut self) -> Image<&mut [u8], CHANNELS> { - Image::new(self.width, self.height, &mut self.buffer) + #[allow(clippy::undocumented_unsafe_blocks)] + unsafe { + Image::new(self.width, self.height, &mut self.buffer) + } } } @@ -302,14 +333,9 @@ save!(1 == Grayscale("Y")); #[cfg(test)] macro_rules! img { - [[$($v:literal),+] [$($v2:literal),+]] => {{ - let from: Image, 1> = Image::new( - 2.try_into().unwrap(), - 2.try_into().unwrap(), - vec![$($v,)+ $($v2,)+] - ); - from - }} + [[$($v:literal),+] [$($v2:literal),+]] => { + Image::, 1>::build(2,2).buf(vec![$($v,)+ $($v2,)+]) + } } #[cfg(test)] use img; diff --git a/src/overlay.rs b/src/overlay.rs index 9624cb9..fbc9b17 100644 --- a/src/overlay.rs +++ b/src/overlay.rs @@ -131,6 +131,8 @@ impl OverlayAt> for Image<&mut [u8], 3> { let o_x = ((j + y as usize) * self.width() as usize + x as usize) * 3 ..((j + y as usize) * self.width() as usize + x as usize + ($n as usize)) * 3; + debug_assert!(o_x.end < self.buffer().len()); + debug_assert!(i_x.end < with.buffer().len()); // SAFETY: bounds are ✅ let a = unsafe { self.buffer.get_unchecked_mut(o_x) }; // SAFETY: we are in ⬜!