Skip to content

Commit

Permalink
#![deny(clippy::undocumented_unsafe_blocks)]: Document the remainin…
Browse files Browse the repository at this point in the history
…g ones (not the `unsafe impl Send + Sync`) (#1321)
  • Loading branch information
kkysen authored Jul 17, 2024
2 parents 74f485b + 8fbd68b commit 068be6b
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 18 deletions.
5 changes: 4 additions & 1 deletion include/dav1d/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ pub(crate) type Rav1dUserData = Option<CArc<u8>>;
impl From<Dav1dUserData> for Rav1dUserData {
fn from(value: Dav1dUserData) -> Self {
let Dav1dUserData { data: _, r#ref } = value;
r#ref.map(|r#ref| unsafe { CArc::from_raw(r#ref) })
r#ref.map(|r#ref| {
// SAFETY: `r#ref` came from `CArc::into_raw`.
unsafe { CArc::from_raw(r#ref) }
})
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/align.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ macro_rules! def_align {

impl AlignedByteChunk for $name<[u8; $align]> {}

/// SAFETY: We never materialize a `&mut [V]` since we do a direct cast.
unsafe impl<V, const N: usize> AsMutPtr for $name<[V; N]> {
type Target = V;

Expand Down Expand Up @@ -243,6 +244,9 @@ impl<T: Copy, C: AlignedByteChunk> Default for AlignedVec<T, C> {
pub type AlignedVec32<T> = AlignedVec<T, Align32<[u8; 32]>>;
pub type AlignedVec64<T> = AlignedVec<T, Align64<[u8; 64]>>;

/// SAFETY: We never materialize a `&mut [T]` since we
/// only materialize a `&mut AlignedVec<T, _>` and call [`AlignedVec::as_mut_ptr`] on it,
/// which calls [`Vec::as_mut_ptr`] and never materializes a `&mut [V]`.
unsafe impl<T: Copy, C: AlignedByteChunk> AsMutPtr for AlignedVec<T, C> {
type Target = T;

Expand Down
34 changes: 17 additions & 17 deletions src/c_arc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,31 +78,31 @@ impl<T: ?Sized> AsRef<T> for CArc<T> {
#[cfg(debug_assertions)]
{
use std::mem;
use std::ptr;
use to_method::To;

// Some extra checks to check if our ptrs are definitely invalid.

let real_ref = (*self.owner).as_ref().get_ref();
assert_eq!(real_ref.to::<NonNull<T>>(), self.base_stable_ref.0);

// Cast through `*const ()` and use [`pointer::byte_offset_from`]
// to remove any fat ptr metadata.
let offset = unsafe {
self.stable_ref
.0
.as_ptr()
.cast::<()>()
.byte_offset_from((real_ref as *const T).cast::<()>())
};
let offset = offset.try_to::<usize>().unwrap();
let real_ptr = ptr::from_ref(real_ref);
let stable_ptr = self.stable_ref.0.as_ptr().cast_const();
// Cast through `*const ()` to remove any fat ptr metadata.
// Use arithmetic on the addresses (similar to `.wrapping_*` methods),
// as they don't have safety conditions (which we're checking here).
let [real_address, stable_address] =
[real_ptr, stable_ptr].map(|ptr| ptr.cast::<()>() as isize);
let offset = stable_address - real_address;
let len = mem::size_of_val(real_ref);
let out_of_bounds = offset > len;
if out_of_bounds {
dbg!(real_ref as *const T);
dbg!(self.stable_ref.0.as_ptr());
dbg!(offset);
dbg!(len);
panic!("CArc::stable_ref is out of bounds");
if offset < 0 || offset > len as isize {
panic!(
"CArc::stable_ref is out of bounds:
real_ref: {real_ptr:?}
stable_ref: {stable_ptr:?}
offset: {offset}
len: {len}"
);
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/c_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ impl<T: ?Sized> AsRef<T> for CBox<T> {
fn as_ref(&self) -> &T {
match self {
Self::Rust(r#box) => r#box.as_ref(),
// SAFETY: `data` is a `Unique<T>`, which behaves as if it were a `T`,
// 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() },
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/disjoint_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,9 @@ impl<V> DisjointMut<Vec<V>> {
}
}

/// SAFETY: We never materialize a `&mut [V]` since we
/// only materialize a `&mut Vec<V>` and call [`Vec::as_mut_ptr`] on it,
/// which never materializes a `&mut [V]`.
unsafe impl<V> AsMutPtr for Vec<V> {
type Target = V;

Expand All @@ -950,6 +953,7 @@ unsafe impl<V> AsMutPtr for Vec<V> {
}
}

/// SAFETY: We never materialize a `&mut [V]` since we do a direct cast.
unsafe impl<V, const N: usize> AsMutPtr for [V; N] {
type Target = V;

Expand All @@ -962,6 +966,7 @@ unsafe impl<V, const N: usize> AsMutPtr for [V; N] {
}
}

/// SAFETY: We never materialize a `&mut [V]` since we do a direct unsizing cast.
unsafe impl<V> AsMutPtr for [V] {
type Target = V;

Expand All @@ -974,6 +979,8 @@ unsafe impl<V> AsMutPtr for [V] {
}
}

/// SAFETY: We never materialize a `&mut [V]` since we go use [`addr_of_mut!`]
/// to create a `*mut [V]` directly, which we then unsize cast.
unsafe impl<V> AsMutPtr for Box<[V]> {
type Target = V;

Expand Down
2 changes: 2 additions & 0 deletions src/mc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1829,6 +1829,7 @@ unsafe extern "C" fn blend_v_c_erased<BD: BitDepth>(
) {
// SAFETY: Was passed as `FFISafe::new(_)` in `blend_dir::Fn::call`.
let dst = *unsafe { FFISafe::get(dst) };
// SAFETY: Reverse of cast in `blend_dir::Fn::call`.
let tmp = unsafe { &*tmp.cast() };
let w = w as usize;
let h = h as usize;
Expand All @@ -1849,6 +1850,7 @@ unsafe extern "C" fn blend_h_c_erased<BD: BitDepth>(
) {
// SAFETY: Was passed as `FFISafe::new(_)` in `blend_dir::Fn::call`.
let dst = *unsafe { FFISafe::get(dst) };
// SAFETY: Reverse of cast in `blend_dir::Fn::call`.
let tmp = unsafe { &*tmp.cast() };
let w = w as usize;
let h = h as usize;
Expand Down

0 comments on commit 068be6b

Please sign in to comment.