rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.34k stars 332 forks source link

Miri does not report UB when transmuting vtables between instances of the same trait with different associated types #3905

Open ChayimFriedman2 opened 2 hours ago

ChayimFriedman2 commented 2 hours ago

Take the following code for example (https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=f1fc9b16c4d0b7cc512fcd33ee2097b6):

trait Trait {
    type Assoc;

    fn foo(&self) {}
}

impl Trait for bool {
    type Assoc = bool;
}

fn main() {
    let v: Box<dyn Trait<Assoc = bool>> = Box::new(true);
    let _v: Box<dyn Trait<Assoc = ()>> = unsafe { std::mem::transmute(v) };
}

I included an empty function in the trait so its vtable won't be empty which may affect things somehow. AFAIK, changing principal trait (aka. not removing or adding auto traits only) is immediate UB, because the vtable has a validity invariant of being valid, and the compiler will exploit this (e.g. to hoist method loads).

However, Miri does not report this as UB.

RalfJung commented 2 hours ago

We check that the principal trait still matches. But I seem to recall there was some problem with dyn types also affecting the type checker, where two dyn trait references were considered equal even when they should not? I don't know if that has been fixed already and Miri needs to copy that fix (i.e., the fix doesn't automatically apply everywhere), or whether that work is still ongoing, or whether it is a different issue.

Cc @lcnr @compiler-errors The check that tries to ensure that the trait in the vtable matches the "expected" trait given in the type is here.

compiler-errors commented 1 hour ago

@RalfJung: The type checker specifically checks that all projections are equal, but miri only checks the principal. I could extend the vtable compatibility check to implement this check, too, if you'd like.

Side-note: We probably also need to check that the auto traits match... I feel like there may be some sketchy stuff going on when we transmute dyn Trait to dyn Trait + Send, though I cannot immediately turn that into UB.

compiler-errors commented 1 hour ago

But I seem to recall there was some problem with dyn types also affecting the type checker [...] I don't know if that has been fixed already and Miri needs to copy that fix (i.e., the fix doesn't automatically apply everywhere), or whether that work is still ongoing, or whether it is a different issue.

This is a totally unrelated error to the recent strengthening of checks for pointer conversions allowed for a dyn trait (i.e. disallowing conversion like *const dyn Foo<A> as *const dyn Foo<B>), since this is using transmute.