dropck unsoundness: unions are ignored

[RalfJung]

So it turns out that this is a problem: The following code should be rejected by dropck, but it compiles.

#![feature(untagged_unions)]
#![allow(unions_with_drop_fields)]

use std::cell::Cell;
use std::ops::Deref;

union Wrap<T> { x: T }

impl<T> Drop for Wrap<T>  {
    fn drop(&mut self) {
        unsafe { std::ptr::drop_in_place(&mut self.x as *mut _); }
    }
}

impl<T> Wrap<T> {
    fn new(x: T) -> Self {
        Wrap { x }
    }
}

impl<T> Deref for Wrap<T> {
    type Target = T;
    #[inline]
    fn deref(&self) -> &Self::Target {
        unsafe {
            &self.x
        }
    }
}

struct C<'a>(Cell<Option<&'a C<'a>>>);

impl<'a> Drop for C<'a> {
    fn drop(&mut self) {}
}

fn main() {
    let v : Wrap<C> = Wrap::new(C(Cell::new(None)));
    v.0.set(Some(&v));
}

This is a nightly-only (because implementing Drop for unions is unstable) soundness issue. I need unsafe code to exploit it, but Wrap above should be a perfectly safe to use type.

[petrochenkov]

Pretty recent regression.

[petrochenkov]

Wrap above should be a perfectly safe to use type

Not without recently discussed guarantees on union validity :)
Remember how third parties can legally write garbage into Wrap that's not a valid T? This means neither deref nor drop are safe operation in that interpretation.

[RalfJung]

Not without recently discussed guarantees on union validity :)

Remember how third parties can legally write garbage into Wrap that's not a valid T?

No, that's not at all what I was saying. We were discussing basic layout invariants. These are invariants the compiler knows about and exploits for enum layout optimizations; violating them is insta-UB. For example, a reference must be non-NULL and aligned. Even a reference being valid is already not covered here as it cannot expressed in terms of just its bit pattern.

However, for private fields, types may have arbitrary additional invariants. I hope we agree that the layout invariant for Vec is simple: Just looking at the type (NonNull<T>, usize, usize), the only layout invariants we have is that none of the fields can be undef and that the first field must not be NULL. Still, a third party cannot legally write any garbage conforming with these invariants into those fields, like setting capactiy to usize::MAX! Doing this is not insta-UB, but can easily lead to UB on the next call to push.

This is an example for an extra invariant that the compiler does not know about, that the data structure upholds through privacy and that unsafe code in this module can hence rely on. Wrap works the same way, its only field is private for this very reason.

[petrochenkov]

@RalfJung
Answered in #32836 (comment) to avoid offtopic here.