From 9290b179ae4e1ae579887ab52ad6b831c58b222c Mon Sep 17 00:00:00 2001 From: RustyYato Date: Wed, 15 Jan 2025 12:09:22 -0700 Subject: [PATCH 1/3] Fix UnionN::clone to check alignemnt ... ... of the cloned pointer. See #89 for details fixes #89 --- crates/ptr-union/src/lib.rs | 9 +++- crates/ptr-union/tests/clone_unaligned.rs | 66 +++++++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 crates/ptr-union/tests/clone_unaligned.rs diff --git a/crates/ptr-union/src/lib.rs b/crates/ptr-union/src/lib.rs index b3f23aa..494bb70 100644 --- a/crates/ptr-union/src/lib.rs +++ b/crates/ptr-union/src/lib.rs @@ -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() -> ! { + panic!("Tried to clone {} in a {}, but the cloned pointer wasn't sufficiently aligned", core::any::type_name::(), stringify!($Union)) + } + None - $(.or_else(|| self.[]().map(|this| builder.$a(this))))* + $(.or_else(|| self.[]().map(|this| Self::[](this).unwrap_or_else(|_| clone_error::<$A>()))))* .unwrap_or_else(|| unsafe { unreachable_unchecked() }) } } diff --git a/crates/ptr-union/tests/clone_unaligned.rs b/crates/ptr-union/tests/clone_unaligned.rs new file mode 100644 index 0000000..2c762c9 --- /dev/null +++ b/crates/ptr-union/tests/clone_unaligned.rs @@ -0,0 +1,66 @@ +//! 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, +} + +// 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 { + pub 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, + NonNull, + NonNull, + NonNull, + NonNull, + NonNull, + NonNull, +>; + +#[test] +#[should_panic = "but the cloned pointer wasn't sufficiently aligned"] +fn test() { + 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(); +} From 93ab53f500dfe8c9ca7329237b43382a873408bc Mon Sep 17 00:00:00 2001 From: RustyYato Date: Wed, 15 Jan 2025 12:12:48 -0700 Subject: [PATCH 2/3] fix test name --- crates/ptr-union/tests/clone_unaligned.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ptr-union/tests/clone_unaligned.rs b/crates/ptr-union/tests/clone_unaligned.rs index 2c762c9..bbbe0c2 100644 --- a/crates/ptr-union/tests/clone_unaligned.rs +++ b/crates/ptr-union/tests/clone_unaligned.rs @@ -56,7 +56,7 @@ type Union = Union8< #[test] #[should_panic = "but the cloned pointer wasn't sufficiently aligned"] -fn test() { +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(); From 6cb91b56c829c6dfb3b5145a14383a8f41f6beb5 Mon Sep 17 00:00:00 2001 From: RustyYato Date: Wed, 15 Jan 2025 15:06:13 -0700 Subject: [PATCH 3/3] fix clippy warnings --- crates/ptr-union/tests/clone_unaligned.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/ptr-union/tests/clone_unaligned.rs b/crates/ptr-union/tests/clone_unaligned.rs index bbbe0c2..aee6667 100644 --- a/crates/ptr-union/tests/clone_unaligned.rs +++ b/crates/ptr-union/tests/clone_unaligned.rs @@ -29,7 +29,7 @@ unsafe impl erasable::ErasablePtr for MyBox { static OFFSET: AtomicUsize = AtomicUsize::new(8); impl MyBox { - pub fn new() -> Self { + fn new() -> Self { let offset = OFFSET.fetch_add(1, std::sync::atomic::Ordering::SeqCst); MyBox { ptr: NonNull::new(offset as _).unwrap(), @@ -55,6 +55,7 @@ type Union = Union8< >; #[test] +#[allow(clippy::redundant_clone)] #[should_panic = "but the cloned pointer wasn't sufficiently aligned"] fn test_clone_unaligned() { let bx = MyBox::new();