Skip to content

Commit

Permalink
Merge pull request #90 from RustyYato/ptr-union-clone-fix
Browse files Browse the repository at this point in the history
Fix UnionN::clone to check alignment of the cloned pointer
  • Loading branch information
CAD97 authored Jan 16, 2025
2 parents 26c260e + 6cb91b5 commit 2f698df
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 2 deletions.
9 changes: 7 additions & 2 deletions crates/ptr-union/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,9 +558,14 @@ macro_rules! impl_union {
{
paste::paste! {
fn clone(&self) -> Self {
let builder = unsafe { <$Builder<$($A,)*>>::new_unchecked() };
#[cold]
#[inline(never)]
fn clone_error<A>() -> ! {
panic!("Tried to clone {} in a {}, but the cloned pointer wasn't sufficiently aligned", core::any::type_name::<A>(), stringify!($Union))
}

None
$(.or_else(|| self.[<clone_ $a>]().map(|this| builder.$a(this))))*
$(.or_else(|| self.[<clone_ $a>]().map(|this| Self::[<new_ $a>](this).unwrap_or_else(|_| clone_error::<$A>()))))*
.unwrap_or_else(|| unsafe { unreachable_unchecked() })
}
}
Expand Down
67 changes: 67 additions & 0 deletions crates/ptr-union/tests/clone_unaligned.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
//! This is a regression test for https://github.com/CAD97/pointer-utils/issues/89
//!
//! The idea here is to have a Box like pointer which we can control the alignment for to
//! ensure that the tests are stable (doesn't depend on allocator shenanigans).
use std::{ptr::NonNull, sync::atomic::AtomicUsize};

use ptr_union::Union8;

#[derive(Debug)]
struct MyBox {
ptr: NonNull<u8>,
}

// SAFETY:
// * MyBox doesn't have any shared mutability
// * the address of the returned pointer doesn't depend on the address of MyBox
// * MyBox doesn't implement Deref
unsafe impl erasable::ErasablePtr for MyBox {
fn erase(this: Self) -> erasable::ErasedPtr {
this.ptr.cast()
}

unsafe fn unerase(this: erasable::ErasedPtr) -> Self {
Self { ptr: this.cast() }
}
}

static OFFSET: AtomicUsize = AtomicUsize::new(8);

impl MyBox {
fn new() -> Self {
let offset = OFFSET.fetch_add(1, std::sync::atomic::Ordering::SeqCst);
MyBox {
ptr: NonNull::new(offset as _).unwrap(),
}
}
}

impl Clone for MyBox {
fn clone(&self) -> Self {
Self::new()
}
}

type Union = Union8<
MyBox,
NonNull<u8>,
NonNull<u8>,
NonNull<u8>,
NonNull<u8>,
NonNull<u8>,
NonNull<u8>,
NonNull<u8>,
>;

#[test]
#[allow(clippy::redundant_clone)]
#[should_panic = "but the cloned pointer wasn't sufficiently aligned"]
fn test_clone_unaligned() {
let bx = MyBox::new();
// this can't fail since the first `MyBox` is created at address 8, which is aligned to 8 bytes
let x = Union::new_a(bx).unwrap();

// this clone should panic, since the next `MyBox` is created at address 9, which is not aligned to 8 bytes
let _y = x.clone();
}

0 comments on commit 2f698df

Please sign in to comment.