add CloneFromCell and Cell::get_cloned

[Qelxiros]

Tracking issue: #145329

[rustbot]

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Qelxiros]

@rustbot ready (unless the CI fails)

[tgross35]

The docs and safety comment don't quite cover it; moves are bitwise copies, and of course that can be cloned as well as the original. Copy really just means that a bitwise copy doesn't invalidate the original. I'm not sure what the exact preconditions are here though, in most cases isn't this just something like Freeze + Unpin?

I'm sure I am missing something, cc @programmerjake

[programmerjake]

the idea is that if you have &Cell<T> it's unsound to call T::clone on that reference for generic T since it could contain references to the same &Cell<T> which allows it to modify T while a &T was passed to clone. So, instead, we do a bitwise copy of T to a another memory location that isn't aliased and call T::clone on that, making sure to forget the bitwise copy using something like ManuallyDrop. not all types are safe to just outright bitwise copy while the original is still valid (e.g. doing a bitwise copy on T = Cell<Option<String>> allows you to drop the String twice by taking it from the copy and from the original Cell<Cell<Option<String>>>).

[programmerjake]

T: CloneFromCopy means the following code is sound:

pub fn clone_from_copy<T: CloneFromCopy>(v: &Cell<T>) -> T {
    let copy = unsafe { ManuallyDrop::new(ptr::read(v).into_inner()) };
    T::clone(&copy)
}

[tgross35]

To put the above into preconditions,

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[Qelxiros]

@rustbot ready

[programmerjake]

sorry I wasn't very clear before: it's important to not drop the bitwise copy, otherwise it's not safe to impl for Rc and other reference-counted types:

fn f(v: &Cell<Rc<u8>>) -> Rc<u8>
where
    Rc<u8>: CloneFromCopy,
{
    // Safety: this is unsound, but CloneFromCopy's current wording implies this is fine:
    let copy = unsafe { ptr::read(v.as_ptr()) };
    let retval = copy.clone();
    drop(copy);
    retval
}

fn cause_use_after_free() {
    let a = Cell::new(Rc::new(0u8));
    let b = f(&a); // f clones once and drops once so reference count is 1
    drop(a); // reference count is now 0, backing memory is freed
    dbg!(b); // use after free, this is UB
}

[tgross35]

(I'll effectively delegate this review to Jake)

@rustbot author

[Qelxiros]

@rustbot ready

[programmerjake]

generally looks good, though I think we should wait and see if we can't do: #145329 (comment) (edit: we can't)

View changes since this review

[tgross35]

Thanks Jacob for reviewing.

@bors r=programmerjake,tgross35 rollup

[bors]

📌 Commit 08b4641 has been approved by programmerjake,tgross35

It is now in the queue for this repository.