Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UnionN::clone to check alignment of the cloned pointer #90

Merged
merged 3 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}
Loading