Array and Vec's Clone specialization is maybe unsound with conditionally Copy types.

[theemathas]

Currently, the Clone impl for [T; N] uses specialization. If T implements Copy, then cloning a [T; N] will do a memcpy (ignoring T's Clone impl). If T doesn't implement Copy, then it will iterate and call T's Clone impl.

However, specialization doesn't look at lifetimes. Therefore, even if T implements Copy for only some lifetimes, cloning a [T; N] will do a memcopy. This is incorrect in the case where T actually doesn't implement Copy. For example:

struct Weird<'a>(&'a i32);

impl Clone for Weird<'_> {
 fn clone(&self) -> Self {
 println!("clone() called");
 Weird(self.0)
 }
}

impl Copy for Weird<'static> {}

fn main() {
 let local = 1;
 let _ = [Weird(&local)].clone();
}

In the above code, Weird only implements Copy when its lifetime is 'static. Therefore, I think that cloning a [Weird<'a>; 1] should call the clone() method. However, running the above code in either stable (1.82.0) or nightly (1.84.0-nightly (2024-10-30 759e07f063fb8e6306ff)) rust doesn't print anything. I believe that this is incorrect.

I am unsure whether or not this can cause UB in purely safe code, (hence the "maybe" in the issue title). But maybe someone else could figure out how to do that?

However, I have written this code, which uses unsafe code to implement a WeirdCow type whose public API I believe is sound on its own, but can be used in combination with array's Clone specialization to cause use-after-free. Either this WeirdCow type is unsound, or the array's Clone implementation is unsound, and I can't figure out which.

@rustbot label +I-unsound

[ChayimFriedman2]

I believe it was discussed before, and the conclusion was that the specialization is okay and it's invalid to implement Copy depending on lifetimes, and the compiler should probably lint/err at it.

[scottmcm]

@theemathas Is this at all specific to the array impl? Can you do the same thing with Vec, which has the same specialization?

rust/library/alloc/src/slice.rs

Line 156 in a8e1186

impl<T: Copy> ConvertVec for T {

[theemathas]

Can you do the same thing with Vec

Yes. This code also doesn't print anything.

struct Weird<'a>(&'a i32);

impl Clone for Weird<'_> {
 fn clone(&self) -> Self {
 println!("clone() called");
 Weird(self.0)
 }
}

impl Copy for Weird<'static> {}

fn main() {
 let local = 1;
 let _ = vec![Weird(&local)].clone();
}

[Noratrieb]

Implementing copy conditionally on lifetimes is nonsensical and unsupported. MIR building will lower all moves of such a value to copy's, which borrowck will then error about when it's not static.
This impl should be an error (which is probably super hard, the same way sound specialization is).

[hanna-kruppe]

More discussion of impl Copy for Foo<'static>: #126179

[lcnr]

Related to #89948. I think that until we actually forbid such Copy impls, I strongly feel like this is a std bug and your WeirdCow should be a sound abstraction. We should not specialize on Copy and doing so is unsound as it ignores lifetime constraints.

Whether Copy with lifetime constraints makes your type pretty much unusable (it prevents ever moving it as all moves get upgraded to use Copy) does not feel relevant to whether std should be allowed to do this. It does provide a good argument for linting/forbidding the Copy impls, at which point std can specialization on Copy without using the unsound (from a type system POV) #[rustc_unsafe_specialization_marker] trait:

rust/compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs

Lines 58 to 66 in 7fbd4ce

	//! ### rustc_unsafe_specialization_marker
	//!
	//! There are also some specialization on traits with no methods, including the
	//! stable `FusedIterator` trait. We allow marking marker traits with an
	//! unstable attribute that means we ignore them in point 3 of the checks
	//! above. This is unsound, in the sense that the specialized impl may be used
	//! when it doesn't apply, but we allow it in the short term since it can't
	//! cause use after frees with purely safe code in the same way as specializing
	//! on traits with methods can.

However, given that std does specialize on Copy and I am quite confident this that won't change in the medium-term, anything relying on lifetime constraints of Copy for soundness is not a sound abstraction until then. I do think we should keep this open as a bug until either specialization is sound wrt to lifetimes or we've changed these Copy impls to be an error.

[theemathas]

I found a somewhat popular crate (2K stars) that specifically depends on this Clone specialization existing, and depends on a Copy impl that is conditional on type equality.

https://github.com/AFLplusplus/LibAFL/blob/89cff637025c1652c24e8d97a30a2e3d01f187a4/libafl_bolts/src/tuples.rs#L27-L49

/// Returns if the type `T` is equal to `U`, ignoring lifetimes.

[purplesyringa]

Note that it's possible to avoid this specialization by using the typeid crate in that case (without losing optimizations). But code like this might easily be duplicated all over the ecosystem.