diff --git a/include/dav1d/picture.rs b/include/dav1d/picture.rs index 8003d6ec5..be360cb2d 100644 --- a/include/dav1d/picture.rs +++ b/include/dav1d/picture.rs @@ -29,6 +29,7 @@ use crate::src::error::Rav1dResult; use crate::src::pixels::Pixels; use crate::src::send_sync_non_null::SendSyncNonNull; use crate::src::strided::Strided; +use crate::src::unique::Unique; use crate::src::with_offset::WithOffset; use libc::ptrdiff_t; use libc::uintptr_t; @@ -94,7 +95,7 @@ impl From for Dav1dPictureParameters { pub struct Dav1dPicture { pub seq_hdr: Option>, pub frame_hdr: Option>, - pub data: [Option>; 3], + pub data: [Option>; 3], pub stride: [ptrdiff_t; 2], pub p: Dav1dPictureParameters, pub m: Dav1dDataProps, @@ -134,7 +135,7 @@ pub struct Rav1dPictureDataComponentInner { /// even if [`Self::stride`] is negative. /// /// It is aligned to [`RAV1D_PICTURE_ALIGNMENT`]. - ptr: NonNull, + ptr: Unique, /// The length of [`Self::ptr`] in [`u8`] bytes. /// @@ -151,12 +152,12 @@ impl Rav1dPictureDataComponentInner { /// # Safety /// /// `ptr`, `len`, and `stride` must follow the requirements of [`Dav1dPicAllocator::alloc_picture_callback`]. - unsafe fn new(ptr: Option>, len: usize, stride: isize) -> Self { + unsafe fn new(ptr: Option>, len: usize, stride: isize) -> Self { let ptr = match ptr { None => { return Self { // Ensure it is aligned enough. - ptr: NonNull::::dangling().cast(), + ptr: Unique::::dangling().cast(), len: 0, stride, }; @@ -168,13 +169,15 @@ impl Rav1dPictureDataComponentInner { assert!(ptr.cast::().is_aligned()); let ptr = if stride < 0 { - let ptr = ptr.as_ptr(); - // SAFETY: According to `Dav1dPicAllocator::alloc_picture_callback`, - // if the `stride` is negative, this is how we get the start of the data. - // `.offset(-stride)` puts us at one element past the end of the slice, - // and `.sub(len)` puts us back at the start of the slice. - let ptr = unsafe { ptr.offset(-stride).sub(len) }; - NonNull::new(ptr).unwrap() + ptr.map(|ptr| { + let ptr = ptr.as_ptr(); + // SAFETY: According to `Dav1dPicAllocator::alloc_picture_callback`, + // if the `stride` is negative, this is how we get the start of the data. + // `.offset(-stride)` puts us at one element past the end of the slice, + // and `.sub(len)` puts us back at the start of the slice. + let ptr = unsafe { ptr.offset(-stride).sub(len) }; + NonNull::new(ptr).unwrap() + }) } else { ptr }; @@ -189,7 +192,7 @@ impl Rav1dPictureDataComponentInner { /// so it is sound to further subdivide it into disjoint `&mut`s. pub fn wrap_buf(buf: &mut [BD::Pixel], stride: usize) -> Self { let buf = AsBytes::as_bytes_mut(buf); - let ptr = NonNull::new(buf.as_mut_ptr()).unwrap(); + let ptr = Unique::from_ref_mut(buf).cast(); assert!(ptr.cast::().is_aligned()); let len = buf.len(); assert!(len % RAV1D_PICTURE_GUARANTEED_MULTIPLE == 0); @@ -292,11 +295,14 @@ impl Rav1dPictureDataComponent { self.as_strided_mut_ptr::().cast_const() } - fn as_dav1d(&self) -> Option> { + fn as_dav1d(&self) -> Option> { if self.byte_len() == 0 { None } else { - NonNull::new(self.as_strided_byte_mut_ptr().cast()) + let ptr = NonNull::new(self.as_strided_byte_mut_ptr()).unwrap(); + // SAFETY: The `ptr` originally comes from a `Unique` in `Rav1dPictureDataComponentInner::ptr`. + let ptr = unsafe { Unique::new(ptr) }; + Some(ptr.cast()) } } diff --git a/lib.rs b/lib.rs index 3bc3ec59c..5be35beb3 100644 --- a/lib.rs +++ b/lib.rs @@ -56,6 +56,7 @@ pub mod src { pub(crate) mod relaxed_atomic; pub mod send_sync_non_null; pub(crate) mod strided; + pub mod unique; pub(crate) mod with_offset; pub(crate) mod wrap_fn_ptr; // TODO(kkysen) Temporarily `pub(crate)` due to a `pub use` until TAIT. diff --git a/src/c_box.rs b/src/c_box.rs index fc6b3169f..005179b42 100644 --- a/src/c_box.rs +++ b/src/c_box.rs @@ -1,12 +1,11 @@ #![deny(unsafe_op_in_unsafe_fn)] use crate::src::send_sync_non_null::SendSyncNonNull; +use crate::src::unique::Unique; use std::ffi::c_void; -use std::marker::PhantomData; use std::ops::Deref; use std::pin::Pin; use std::ptr::drop_in_place; -use std::ptr::NonNull; pub type FnFree = unsafe extern "C" fn(ptr: *const u8, cookie: Option>); @@ -38,37 +37,6 @@ impl Free { } } -/// Same as [`core::ptr::Unique`]. -/// -/// A wrapper around a [`NonNull`]`` that indicates that the possessor -/// of this wrapper owns the referent. -/// -/// [`Unique`]`` behaves "as if" it were an instance of `T`. -/// It implements [`Send`]/[`Sync`] if `T: `[`Send`]/[`Sync`]. -/// It also implies the kind of strong aliasing guarantees an instance of `T` can expect: -/// the referent of the pointer should not be modified -/// without a unique path to its owning [`Unique`]. -/// -/// Unlike [`NonNull`]``, `Unique` is covariant over `T`. -/// This should always be correct for any type which upholds [`Unique`]'s aliasing requirements. -#[derive(Debug)] -pub struct Unique { - pointer: NonNull, - // NOTE: this marker has no consequences for variance, but is necessary - // for dropck to understand that we logically own a `T`. - // - // For details, see: - // https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data - _marker: PhantomData, -} - -/// SAFETY: [`Unique`] is [`Send`] if `T: `[`Send`] -/// because the data it references is unaliased. -unsafe impl Send for Unique {} - -/// SAFETY: [`Unique`] is [`Sync`] if `T: `[`Sync`] -unsafe impl Sync for Unique {} - /// A C/custom [`Box`]. /// /// That is, it is analogous to a [`Box`], @@ -97,7 +65,7 @@ impl AsRef for CBox { // so we can take `&` references of it. // Furthermore, `data` is never moved and is valid to dereference, // so this reference can live as long as `CBox` and still be valid the whole time. - Self::C { data, .. } => unsafe { data.pointer.as_ref() }, + Self::C { data, .. } => unsafe { data.as_ref() }, } } } @@ -115,7 +83,7 @@ impl Drop for CBox { match self { Self::Rust(_) => {} // Drop normally. Self::C { data, free, .. } => { - let ptr = data.pointer.as_ptr(); + let ptr = data.as_ptr(); // SAFETY: See below. // The [`FnFree`] won't run Rust's `fn drop`, // so we have to do this ourselves first. @@ -135,14 +103,8 @@ impl CBox { /// until `free.free` is called on it, which must deallocate it. /// `free.free` is always called with `free.cookie`, /// which must be accessed thread-safely. - pub unsafe fn from_c(data: NonNull, free: Free) -> Self { - Self::C { - data: Unique { - pointer: data, - _marker: PhantomData, - }, - free, - } + pub unsafe fn from_c(data: Unique, free: Free) -> Self { + Self::C { data, free } } pub fn from_box(data: Box) -> Self { diff --git a/src/data.rs b/src/data.rs index 65a056841..f6f00b097 100644 --- a/src/data.rs +++ b/src/data.rs @@ -10,8 +10,8 @@ use crate::src::c_box::Free; use crate::src::error::Rav1dError::EINVAL; use crate::src::error::Rav1dResult; use crate::src::send_sync_non_null::SendSyncNonNull; +use crate::src::unique::Unique; use std::ffi::c_void; -use std::ptr::NonNull; impl From> for Rav1dData { fn from(data: CArc<[u8]>) -> Self { @@ -36,7 +36,7 @@ impl Rav1dData { /// /// See [`CBox::from_c`]'s safety for `data`, `free_callback`, `cookie`. pub unsafe fn wrap( - data: NonNull<[u8]>, + data: Unique<[u8]>, free_callback: Option, cookie: Option>, ) -> Rav1dResult { @@ -53,7 +53,7 @@ impl Rav1dData { /// See [`CBox::from_c`]'s safety for `user_data`, `free_callback`, `cookie`. pub unsafe fn wrap_user_data( &mut self, - user_data: NonNull, + user_data: Unique, free_callback: Option, cookie: Option>, ) -> Rav1dResult { diff --git a/src/lib.rs b/src/lib.rs index 0b05febfa..854077ce1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -51,6 +51,7 @@ use crate::src::send_sync_non_null::SendSyncNonNull; use crate::src::thread_task::rav1d_task_delayed_fg; use crate::src::thread_task::rav1d_worker_task; use crate::src::thread_task::FRAME_ERROR; +use crate::src::unique::Unique; use parking_lot::Mutex; use std::cmp; use std::ffi::c_char; @@ -862,7 +863,7 @@ pub unsafe extern "C" fn dav1d_data_create(buf: Option>, sz: #[no_mangle] pub unsafe extern "C" fn dav1d_data_wrap( buf: Option>, - ptr: Option>, + ptr: Option>, sz: usize, free_callback: Option, user_data: Option>, @@ -873,8 +874,10 @@ pub unsafe extern "C" fn dav1d_data_wrap( validate_input!((sz <= usize::MAX / 2, EINVAL))?; // SAFETY: `ptr` is the start of a `&[u8]` slice of length `sz`. let data = unsafe { slice::from_raw_parts(ptr.as_ptr(), sz) }; + // SAFETY: `data` is unique since `ptr` was unique. + let data = unsafe { Unique::from_ref(data) }; // SAFETY: `ptr`, and thus `data`, is valid to dereference until `free_callback` is called on it, which deallocates it. - let data = unsafe { Rav1dData::wrap(data.into(), free_callback, user_data) }?; + let data = unsafe { Rav1dData::wrap(data, free_callback, user_data) }?; let data_c = data.into(); // SAFETY: `buf` is safe to write to. unsafe { buf.as_ptr().write(data_c) }; @@ -890,7 +893,7 @@ pub unsafe extern "C" fn dav1d_data_wrap( #[no_mangle] pub unsafe extern "C" fn dav1d_data_wrap_user_data( buf: Option>, - user_data: Option>, + user_data: Option>, free_callback: Option, cookie: Option>, ) -> Dav1dResult { diff --git a/src/picture.rs b/src/picture.rs index 5463c51dd..2e89d249e 100644 --- a/src/picture.rs +++ b/src/picture.rs @@ -26,14 +26,13 @@ use crate::src::log::Rav1dLog as _; use crate::src::log::Rav1dLogger; use crate::src::mem::MemPool; use crate::src::send_sync_non_null::SendSyncNonNull; +use crate::src::unique::Unique; use bitflags::bitflags; use libc::ptrdiff_t; use parking_lot::Mutex; use std::ffi::c_int; use std::ffi::c_void; use std::mem; -use std::ptr; -use std::ptr::NonNull; use std::sync::atomic::AtomicU32; use std::sync::Arc; use to_method::To as _; @@ -142,16 +141,16 @@ unsafe extern "C" fn dav1d_default_picture_alloc( // but this way is simpler and more uniform, especially when we move to slices. let data = [data0, data1, data2].map(|data| { if data.is_empty() { - ptr::null_mut() + None } else { - data.as_mut_ptr().cast() + Some(Unique::from_ref_mut(data).cast()) } }); // SAFETY: Guaranteed by safety preconditions. let p_c = unsafe { &mut *p_c }; p_c.stride = stride; - p_c.data = data.map(NonNull::new); + p_c.data = data; p_c.allocator_data = Some(SendSyncNonNull::from_box(buf).cast::()); // The caller will create the real `Rav1dPicture` from the `Dav1dPicture` fields set above, // so we don't want to drop the `Rav1dPicture` we created for convenience here. diff --git a/src/unique.rs b/src/unique.rs new file mode 100644 index 000000000..9be5a7d97 --- /dev/null +++ b/src/unique.rs @@ -0,0 +1,190 @@ +//! Largely copied from [`rust-lang/rust/library/core/src/ptr/unique.rs`](https://github.com/rust-lang/rust/blob/e35364a521372ce682e4bd4a5850d97ea33b0eab/library/core/src/ptr/unique.rs#L58). + +use std::fmt; +use std::marker::PhantomData; +use std::ptr::NonNull; + +/// Same as [`core::ptr::Unique`]. +/// +/// A wrapper around a [`NonNull`] that indicates that the possessor +/// of this wrapper owns the referent. +/// +/// [`Unique`] behaves "as if" it were an instance of `T`. +/// It implements [`Send`]/[`Sync`] if `T: `[`Send`]/[`Sync`]. +/// It also implies the kind of strong aliasing guarantees an instance of `T` can expect: +/// the referent of the pointer should not be modified +/// without a unique path to its owning [`Unique`]. +/// +/// Unlike [`NonNull`], [`Unique`] is covariant over `T`. +/// This should always be correct for any type which upholds [`Unique`]'s aliasing requirements. +#[repr(transparent)] +pub struct Unique { + pointer: NonNull, + // NOTE: this marker has no consequences for variance, but is necessary + // for dropck to understand that we logically own a `T`. + // + // For details, see: + // https://github.com/rust-lang/rfcs/blob/master/text/0769-sound-generic-drop.md#phantom-data + _marker: PhantomData, +} + +/// SAFETY: [`Unique`] is [`Send`] if `T: `[`Send`] +/// because the data it references is unaliased. +unsafe impl Send for Unique {} + +/// SAFETY: [`Unique`] is [`Sync`] if `T: `[`Sync`] +unsafe impl Sync for Unique {} + +impl Unique { + /// Creates a new [`Unique`] that is dangling, but well-aligned. + /// + /// This is useful for initializing types which lazily allocate, like + /// [`Vec::new`] does. + /// + /// Note that the pointer value may potentially represent a valid pointer to + /// a `T`, which means this must not be used as a "not yet initialized" + /// sentinel value. Types that lazily allocate must track initialization by + /// some other means. + #[must_use] + #[inline] + pub const fn dangling() -> Self { + // FIXME(const-hack) replace with `From` + Unique { + pointer: NonNull::dangling(), + _marker: PhantomData, + } + } + + #[inline] + #[must_use] + pub fn is_aligned(&self) -> bool { + self.pointer.is_aligned() + } +} + +impl Unique { + /// Creates a new [`Unique`]. + /// + /// # Safety + /// + /// `ptr` must be unique. + #[inline] + pub const unsafe fn new(ptr: NonNull) -> Self { + Unique { + pointer: ptr, + _marker: PhantomData, + } + } + + #[inline] + pub fn from_ref_mut(reference: &mut T) -> Self { + let ptr = reference.into(); + // SAFETY: `&mut` guarantees uniqueness. + unsafe { Self::new(ptr) } + } + + /// # Safety + /// + /// `reference` must be unique. + #[inline] + pub unsafe fn from_ref(reference: &T) -> Self { + let ptr = reference.into(); + // SAFETY: Guaranteed by safety preconditions. + unsafe { Self::new(ptr) } + } + + /// Acquires the underlying `*mut` pointer. + #[must_use = "`self` will be dropped if the result is not used"] + #[inline] + pub const fn as_ptr(self) -> *mut T { + self.pointer.as_ptr() + } + + /// Acquires the underlying `*mut` pointer. + #[must_use = "`self` will be dropped if the result is not used"] + #[inline] + pub const fn as_non_null_ptr(self) -> NonNull { + self.pointer + } + + /// Dereferences the content. + /// + /// The resulting lifetime is bound to self so this behaves "as if" + /// it were actually an instance of T that is getting borrowed. If a longer + /// (unbound) lifetime is needed, use `&*my_ptr.as_ptr()`. + /// + /// # Safety + /// + /// The pointer must be valid to dereference. + #[must_use] + #[inline] + pub const unsafe fn as_ref(&self) -> &T { + // SAFETY: the caller must guarantee that `self` meets all the + // requirements for a reference. + unsafe { self.pointer.as_ref() } + } + + /// Mutably dereferences the content. + /// + /// The resulting lifetime is bound to self so this behaves "as if" + /// it were actually an instance of T that is getting borrowed. If a longer + /// (unbound) lifetime is needed, use `&mut *my_ptr.as_ptr()`. + /// + /// # Safety + /// + /// The pointer must be valid to dereference.#[must_use] + #[inline] + pub unsafe fn as_mut(&mut self) -> &mut T { + // SAFETY: the caller must guarantee that `self` meets all the + // requirements for a mutable reference. + unsafe { self.pointer.as_mut() } + } + + /// Casts to a pointer of another type. + #[must_use = "`self` will be dropped if the result is not used"] + #[inline] + pub const fn cast(self) -> Unique { + // FIXME(const-hack): replace with `From` + // SAFETY: is `NonNull` + Unique { + pointer: self.pointer.cast(), + _marker: PhantomData, + } + } + + pub fn map(self, f: impl FnOnce(NonNull) -> NonNull) -> Self { + Self { + pointer: f(self.pointer), + _marker: PhantomData, + } + } +} + +impl Clone for Unique { + #[inline] + fn clone(&self) -> Self { + *self + } +} + +impl Copy for Unique {} + +impl fmt::Debug for Unique { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Pointer::fmt(&self.as_ptr(), f) + } +} + +impl fmt::Pointer for Unique { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Pointer::fmt(&self.as_ptr(), f) + } +} + +impl From<&mut T> for Unique { + /// Converts a `&mut T` to a [`Unique`]. + #[inline] + fn from(reference: &mut T) -> Self { + Self::from_ref_mut(reference) + } +} diff --git a/tools/dav1d.rs b/tools/dav1d.rs index b6da10f3e..368bae70f 100644 --- a/tools/dav1d.rs +++ b/tools/dav1d.rs @@ -81,6 +81,7 @@ use rav1d::src::lib::DAV1D_API_VERSION_MAJOR; use rav1d::src::lib::DAV1D_API_VERSION_MINOR; use rav1d::src::lib::DAV1D_API_VERSION_PATCH; use rav1d::src::send_sync_non_null::SendSyncNonNull; +use rav1d::src::unique::Unique; use rav1d::Dav1dResult; use std::ffi::c_char; use std::ffi::c_double; @@ -253,7 +254,8 @@ unsafe extern "C" fn picture_alloc( let align_m1: ptrdiff_t = (DAV1D_PICTURE_ALIGNMENT - 1) as ptrdiff_t; let data: *mut u8 = (buf as ptrdiff_t + align_m1 & !align_m1) as *mut u8; (*p).data[0] = - NonNull::new(data.offset(y_sz as isize).offset(-(y_stride as isize)) as *mut c_void); + NonNull::new(data.offset(y_sz as isize).offset(-(y_stride as isize)) as *mut c_void) + .map(|ptr| Unique::new(ptr)); (*p).data[1] = NonNull::new( (if has_chroma != 0 { data.offset(y_sz as isize) @@ -262,7 +264,8 @@ unsafe extern "C" fn picture_alloc( } else { 0 as *mut u8 }) as *mut c_void, - ); + ) + .map(|ptr| Unique::new(ptr)); (*p).data[2] = NonNull::new( (if has_chroma != 0 { data.offset(y_sz as isize) @@ -271,7 +274,8 @@ unsafe extern "C" fn picture_alloc( } else { 0 as *mut u8 }) as *mut c_void, - ); + ) + .map(|ptr| Unique::new(ptr)); Dav1dResult(0) } diff --git a/tools/output/md5.rs b/tools/output/md5.rs index 606bc2eaa..2ca2af8cc 100644 --- a/tools/output/md5.rs +++ b/tools/output/md5.rs @@ -542,7 +542,8 @@ unsafe extern "C" fn md5_write(md5: *mut MD5Context, p: *mut Dav1dPicture) -> c_ let hbd = ((*p).p.bpc > 8) as c_int; let w = (*p).p.w; let h = (*p).p.h; - let mut yptr: *mut u8 = (*p).data[0].map_or_else(ptr::null_mut, NonNull::as_ptr) as *mut u8; + let mut yptr: *mut u8 = + (*p).data[0].map_or_else(ptr::null_mut, |data| data.as_ptr()) as *mut u8; let mut y = 0; while y < h { md5_update(md5, yptr, (w << hbd) as c_uint); @@ -559,7 +560,7 @@ unsafe extern "C" fn md5_write(md5: *mut MD5Context, p: *mut Dav1dPicture) -> c_ let mut pl = 1; while pl <= 2 { let mut uvptr: *mut u8 = - (*p).data[pl as usize].map_or_else(ptr::null_mut, NonNull::as_ptr) as *mut u8; + (*p).data[pl as usize].map_or_else(ptr::null_mut, |data| data.as_ptr()) as *mut u8; let mut y_0 = 0; while y_0 < ch { md5_update(md5, uvptr, (cw << hbd) as c_uint); diff --git a/tools/output/y4m2.rs b/tools/output/y4m2.rs index 66570e90d..bf265f466 100644 --- a/tools/output/y4m2.rs +++ b/tools/output/y4m2.rs @@ -159,7 +159,7 @@ unsafe extern "C" fn y4m2_write(c: *mut Y4m2OutputContext, p: *mut Dav1dPicture) fprintf((*c).f, b"FRAME\n\0" as *const u8 as *const c_char); let mut ptr: *mut u8; let hbd = ((*p).p.bpc > 8) as c_int; - ptr = (*p).data[0].map_or_else(ptr::null_mut, NonNull::as_ptr) as *mut u8; + ptr = (*p).data[0].map_or_else(ptr::null_mut, |data| data.as_ptr()) as *mut u8; let mut y = 0; loop { if !(y < (*p).p.h) { @@ -188,7 +188,7 @@ unsafe extern "C" fn y4m2_write(c: *mut Y4m2OutputContext, p: *mut Dav1dPicture) current_block = 13797916685926291137; break; } - ptr = (*p).data[pl as usize].map_or_else(ptr::null_mut, NonNull::as_ptr) + ptr = (*p).data[pl as usize].map_or_else(ptr::null_mut, |data| data.as_ptr()) as *mut u8; let mut y_0 = 0; while y_0 < ch { diff --git a/tools/output/yuv.rs b/tools/output/yuv.rs index 6c81c5a9c..4a5195d63 100644 --- a/tools/output/yuv.rs +++ b/tools/output/yuv.rs @@ -73,7 +73,7 @@ unsafe extern "C" fn yuv_write(c: *mut YuvOutputContext, p: *mut Dav1dPicture) - let mut current_block: u64; let mut ptr: *mut u8; let hbd = ((*p).p.bpc > 8) as c_int; - ptr = (*p).data[0].map_or_else(ptr::null_mut, NonNull::as_ptr) as *mut u8; + ptr = (*p).data[0].map_or_else(ptr::null_mut, |data| data.as_ptr()) as *mut u8; let mut y = 0; loop { if !(y < (*p).p.h) { @@ -102,7 +102,7 @@ unsafe extern "C" fn yuv_write(c: *mut YuvOutputContext, p: *mut Dav1dPicture) - current_block = 7976072742316086414; break; } - ptr = (*p).data[pl as usize].map_or_else(ptr::null_mut, NonNull::as_ptr) + ptr = (*p).data[pl as usize].map_or_else(ptr::null_mut, |data| data.as_ptr()) as *mut u8; let mut y_0 = 0; while y_0 < ch {