Is `&mut T -> &mut ManuallyDrop<T>` well-defined and sound?

help
1

Like this:

const fn as_manually_drop<T>(x: &mut T) -> &mut ManuallyDrop<T> {
    let ptr: *mut ManuallyDrop<T> = std::ptr::from_mut(x).cast();
    unsafe { ptr.as_mut_unchecked() }
}

I can’t see a reason it wouldn’t be – reading through it doesn’t change anything, and writing seems to only be able to do the same things as ManuallyDrop::into_inner, which is safe – but it also seems like the kind of thing that would already exist if it were correct and useful, and I’m pretty new to writing sound interfaces that use unsafe Rust. (I’m only using it to ManuallyDrop::take a field of a struct that’s already in a ManuallyDrop, so nothing really changes if I have to mark it unsafe, but it would be nice to know what’s correct. Miri doesn’t report any issues with this use.)

2

I don't see how this is useful? can you provide an example use case?

3

I know this isn't quite your question, since you want to call ManuallyDrop::take, not ManuallyDrop::drop, but the below discussion will come back around. Also, most of the below is an exploration of your literal question, I'll offer something else at the end.

Suppose you call ManuallyDrop::drop on the result of your method. That would end up leaving a &mut T reference pointing to a destructed value. I'm not quite sure what's left of a value after you run its destructor, in general... at the very least, I know that running Box's destructor results in the remaining value being data which is not valid for type Box. (By which I mean, the compiler knows about some of Box's invariants, and the remnant of a destructed Box does not satisfy Box's invariants.)

So, you'd have a &mut T which might not point to a valid T.

Now, is this sound? Well, right now, on the current compiler version, yes. And the opsem team is currently leaning towards deciding that this is sound, but they have not yet finalized that decision: so, as of now, the language does not guarantee that this code is sound; a future compiler version could make it UB.

Consider what would happen if you called ManuallyDrop::take on the value, and then a panic unexpectedly occurred in a scope where the &mut T or &mut ManuallyDrop<T> is still live, such that the taken value is dropped as the program unwinds. Then, when T = Box<_>, even though you didn't drop the original value, calling the destructor of the copy of the box still thoroughly invalidates the original Box value, which is still behind a live &mut Box<_> reference.

So, unless you're extremely careful about edge cases like panics... it'd be easy to accidentally rely on unspecified behavior.

What should you do? I'd recommend directly using ptr::read on a raw pointer to the field(s) of the struct.

Here's the first example in std I could find: rust/library/alloc/src/ffi/c_str.rs at 4ddd4538a881317c622ed674b08300b8fc8dabdd · rust-lang/rust · GitHub

Here's an example in a crate of my own: anchored-leveldb/crates/anchored-leveldb/src/pub_leveldb/structs.rs at 7cc38877ba4670a390474fcd9faad81f3d208e4b · robofinch/anchored-leveldb · GitHub

4

One of the ways I’m using/experimenting with it (@nerditation) is this:

struct Foo(X, Y);

impl Drop for Foo {
    fn drop(&mut self) {
        panic!("can't just drop this!");
    }
}

impl Foo {
    fn consume_properly(self) {
        let x;
        let y;

        {
            let mut md = ManuallyDrop::new(self);
            x = {
                let r = as_manually_drop(&mut md.0);
                unsafe { ManuallyDrop::take(r) }
            };
            y = {
                let r = as_manually_drop(&mut md.1);
                unsafe { ManuallyDrop::take(r) }
            };
        }

        // do things with `x` and `y`
    }
}

which seems to normally be done with ptr::read, yeah, but I was curious if the helper function could be introduced soundly. If the only way to create a problem with it is with the already-unsafe ManuallyDrop::drop/ManuallyDrop::take, then that should be okay, right? (And in this take case, the &mut T doesn’t continue to exist.)

Thank you for the detailed information and real-world examples, @robofinch!

5

yes, it is typically done with ptr::read().

what's the point of ManuallyDrop if you don't use it to wrap owned value? it is useless, I mean, what benefit in doing so?

as for the question in the title, I believe the answer is yes, it is sound.

although I don't know if it's explicitly said so, I always rely on the assumption that transmutation from a reference (&T or &mut T) to a #[repr(transparent)] wrapper reference (&W<T> or &mut W<T>) is sound, as long as I pay attention to the usual caveats, like variances etc.

6

That's what I was going to comment looking at this code: what’s the point of doing this over ptr::read? But you already brought ptr::read up yourself. Still, I fail to see any advantage; rather the opposite: IMHO reading something like

impl Foo {
    fn consume_properly(self) {
        let x;
        let y;

        {
            let mut md = ManuallyDrop::new(self);
            x = unsafe { ptr::read(&mut md.0) };
            y = unsafe { ptr::read(&mut md.1) };
        }

        // do things with `x` and `y`
    }
}

makes me a lot less confused as to what’s going on; the code is shorter and more similar to other code like this that I might have seen before.

And it’s not like you were able to spare any need for using unsafe by means of introducing another use of ManuallyDrop here, right?[1] ManuallyDrop is more valuable for wrapping owned things into it, like local variables, or fields in structs; creating it behind a level of indirection seems somewhat pointless, unless we’re still missing a different / better use-case in this discussion.

  1. Rather, it’d be useful to explore the possibility of new language features that allows you to do the action of “moving [one/multiple] (public) field(s) out of a struct without calling the destructor on the whole thing” without any need for unsafe at all. ↩︎