[Solved] Variance of `dyn Trait + 'a`

Continuing the discussion from The Confessional Thread: Parts of Rust that I still don't get after all this time:

cc @Yandros, @ExpHp

I think this is a bug, I tried out some more variations

// none of these compile
// struct Foo<'x>(dyn Trait + 'x);
// struct Foo<'x>(*const (dyn Trait + 'x));
// struct Foo<'x>(*mut (dyn Trait + 'x));
// struct Foo<'x>(Box<dyn Trait + 'x>);
// type Foo<'x> = Box<dyn Trait + 'x>;

// only this compiles
type Foo<'x> = dyn Trait + 'x;

trait Trait {}
const _ : () = {
    fn check<'short, 'long : 'short> (
        it: &'short mut Foo<'long>,
    )
    {
        let _: &'short mut Foo<'short> = it;
    }
    let _ = check;
};

In particular, I'm surprised that struct Foo<'x>(dyn Trait + 'x); doesn't work is different from a trait object. There should be no difference between it and dyn Trait + 'x, but the trait object compiles where the struct does not! It looks like just adding the struct breaks things, which is really surprising.

3 Likes

Interesting, especially the unsized wrapper. I somehow considered that Sized was playing a role (as the reason one cannot easily exploit this variance with mem::swap and the like is because of !Sized-ness).

So, either this is not exploitable, and thus the struct Foo<'x>(dyn Trait + 'x); case should also work, (minor bug), or it is exploitable, and thus &mut ...<dyn Trait + 'x> should never be covariant (soundness bug!).

2 Likes

I do think Sized has something to do it. That's the problem I immediately ran into when trying to exploit it, because typically to exploit covariance bugs you need to replace the pointed-to value.

Perhaps it can be exploited in some way using the unsized locals feature?

1 Like
#![feature(raw)]

fn swap_unsized<T: ?Sized>(a: &mut T, b: &mut T) {
    use std::raw::TraitObject;
    use std::alloc::Layout;
    
    let a_layout = Layout::for_value(a);
    let b_layout = Layout::for_value(b);
    
    assert_eq!(a_layout, b_layout);
    
    let ptr_layout = Layout::new::<&mut T>();
    
    if ptr_layout.size() == 2 * std::mem::size_of::<usize>() {
        unsafe {
            let a_trait: TraitObject = std::mem::transmute_copy(&a);
            let b_trait: TraitObject = std::mem::transmute_copy(&b);
            
            assert_eq!(a_trait.vtable, b_trait.vtable);
        }
    } else {
        assert_eq!(ptr_layout.size(), std::mem::size_of::<usize>())
    }
    
    unsafe {
        let a = std::slice::from_raw_parts_mut(
            a as *mut T as *mut MaybeUninit<u8>,
            a_layout.size(),
        );
        let b = std::slice::from_raw_parts_mut(
            b as *mut T as *mut MaybeUninit<u8>,
            b_layout.size(),
        ):

        a.swap_with_slice(b);
    }
}

This function should be safe for swapping trait objects. It uses the nightly raw feature, but that's all. It will incidentally work for slices and sized types as well, but I don't think that's relevant right now.

edit: fixed behavior around padding byte

edit 2: swap_unsized is unsound for trait objects precicely to allow the unsizing coercions shown in op, see @eddyb's comments below

1 Like

Indeed. And if we were able to have downcasting without the : 'static bound (although I think that will never happen), then we could swap two identical trait objects by downcast_muting them to their concrete type.


Aside

Using mem::swap::<u8>() there is unsound, you should use ptr::swap, ptr::swap_nonoverlapping or mem::swap::<MaybeUninit<u8>>()

You could, on the other hand, swap in "one go" by using <[MaybeUninit<u8>]>::swap_with_slice()

Other than that, it's a very interesting function that could, for instance, enable .take() on trait objects that originated from an Option type :slightly_smiling_face:

1 Like

It's possible &'short mut Foo<'long> coerces to &'short mut Foo<'short> (via unsizing coercion), can you try adding a *mut (just so you don't need another lifetime) around the whole thing?

It's possible we missed a bug here (I remember we fixed some variance issue in this area a while ago), or it's technically fine (but in a way unrelated to variance, just dyn Trait being special).

AFAICT shrinking the dyn Trait lifetime is unexploitable because nothing inside the trait object can observe it.

The underlying type is always immutable, even if you have a &mut dyn Trait, because it's fundamentally impossible (in Rust) to compare types at runtime (even if they have the same lifetime bound, the actual lifetimes inside might differ, and those can't be compared).

4 Likes

EDIT: misunderstood the suggestion

Sorry, I was a bit too unclear. *mut dyn Trait coerces exactly in the same way.

This is what I meant, and looks like variance is fine and you were seeing an unsizing coercion.

The definite proof is in the MIR though, for the original example you have this line:

_2 = move _3 as &mut dyn Trait (Pointer(Unsize));
3 Likes

That explains why adding a struct in between would break things.

2 Likes

Just to drive the point home here, since I missed some upthread discussion: this means the swap_unsized function is unsound and should never be used.

Also, if you're interested whether this coercion is intentional or where it happens, it's in traits::select (link to specific line).

2 Likes

This is intentional behavior that was RFC'ed in 0599

5 Likes

Even if we didn't have unsizing coercions that could shrink the lifetime bound on the trait, swap_unsized would still be unsound.

The problem is that when you create &mut dyn Trait + 'x from &mut T, only T: 'x is required, T could have any number of lifetimes that aren't separately tracked, they just need to be longer than, or equal to, 'x.

So there's no way at runtime to check that two &mut dyn Trait + 'x point to the same exact concrete type even if 'x is the same.

(I should mention that &mut dyn Any + 'static lets you downcast to some &mut T, and you could mem::swap a pair of those, but the 'static is very important there, disallowing any lifetimes)

3 Likes

This was the first key point, the second one being that "forcing a manual implementation of an assignment" such as @KrishnaSannasi's swap_unsize is unsound:

  • Indeed, equality of layout and of vtable pointer (and of 'lt in dyn Trait + 'lt) does not suffice to assume that the underlying / erased types are the same.

    Counter-example

    Given,

    #[derive(Debug)]
    struct Struct<'short, 'a : 'short, 'b : 'short> {
       short: &'short u8,
       a: &'a u8,
       b: &'b u8,
    }
    

    there is no way to distinguish a &mut Struct::<'short, 'a1, 'b1> { ... } as &mut (dyn Debug + 'short) from a &mut Struct::<'short, 'a2, 'b2> { ... } as &mut (dyn Debug + 'short), so they would pass the swap_unsized guards and cause unsoundness, without using the "suprising" unsized coercion of the OP.

2 Likes