rust-lang / miri

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

"constructing invalid value: wrong trait in wide pointer vtable" for seemingly identical traits #3541

Closed Jules-Bertholet closed 3 months ago

Jules-Bertholet commented 5 months ago

I don't have a minimal reduction atm, but the issue can be reproduced by cloning https://github.com/Jules-Bertholet/unsized-vec and running cargo miri test -p emplacable. This results in the following report of UB:

constructing invalid value: wrong trait in wide pointer vtable: expected `for<'b> std::ops::FnMut(std::alloc::Layout, <[std::boxed::Box<i32>] as std::ptr::Pointee>::Metadata, &'b mut (dyn std::ops::FnMut(*mut std::marker::PhantomData<[std::boxed::Box<i32>]>) + 'b))`, but encountered `for<'a> std::ops::FnMut<(std::alloc::Layout, usize, &'a mut (dyn std::ops::FnMut(*mut std::marker::PhantomData<[std::boxed::Box<i32>]>) + 'a))>`

But the two traits are the same! <[std::boxed::Box<i32>] as std::ptr::Pointee>::Metadata is usize, and for<'a> ... vs for<'b> ... should not make a difference either.

@rustbot label I-false-UB A-validation

RalfJung commented 5 months ago

Probably the use of != around here is wrong. @lcnr what is the proper way to compare two Option<ty::Binder<'tcx, ExistentialTraitRef<'tcx>>>?

lcnr commented 5 months ago
let ocx = ObligationCtxt::new(infcx);
let param_env = ty::ParamEnv::reveal_all(); // in an empty environment post borrowck
ocx.eq(param_env, lhs, rhs)?; // equate the two trait refs
ocx.select_all_or_error().is_empty() // and check any potential nested constraints
RalfJung commented 5 months ago

Thanks! We actually have a param_env in the interpreter so I guess it won't be reveal_all. But we don't have a infcx.

lcnr commented 4 months ago

you can create a local one, given that the input should not refer to either placeholders or inference variables

RalfJung commented 3 months ago

@lcnr any idea how to get a reproducible example for this? I tried this code but it doesn't trigger the error -- probably things get normalized earlier already so by the time we check the trait type, everything is already equal.

#![feature(unboxed_closures)]
#![feature(ptr_metadata)]

type T1<'a> = &'a dyn for<'b> Fn(usize);
type T2<'a> = &'a dyn for<'b> Fn<(<[std::boxed::Box<i32>] as std::ptr::Pointee>::Metadata,), Output=()>;

fn main() {
    let x: T1 = &|_| {};
    let y: T2 = unsafe { std::mem::transmute(x) };
    y(42);
}
RalfJung commented 3 months ago

I don't have a minimal reduction atm, but the issue can be reproduced by cloning https://github.com/Jules-Bertholet/unsized-vec and running cargo miri test -p emplacable. This results in the following report of UB:

I get a build error instead:

error[E0283]: type annotations needed
   --> emplacable/src/lib.rs:196:75
    |
196 |         let wide_ptr: *mut str = ptr::from_raw_parts_mut(buf.as_mut_ptr().cast(), LEN);
    |                                  -----------------------                  ^^^^ cannot infer type of the type parameter `U` declared on the method `cast`
    |                                  |
    |                                  required by a bound introduced by this call
    |
    = note: cannot satisfy `_: core::ptr::Thin`
note: required by a bound in `core::ptr::from_raw_parts_mut`
   --> /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/ptr/metadata.rs:137:29
    |
136 | pub const fn from_raw_parts_mut<T: ?Sized>(
    |              ------------------ required by a bound in this function
137 |     data_pointer: *mut impl Thin,
    |                             ^^^^ required by this bound in `from_raw_parts_mut`
help: consider specifying the generic argument
    |
196 |         let wide_ptr: *mut str = ptr::from_raw_parts_mut(buf.as_mut_ptr().cast::<U>(), LEN);
    |                                                                               +++++
help: consider removing this method call, as the receiver has type `*mut core::mem::MaybeUninit<u8>` and `*mut core::mem::MaybeUninit<u8>: core::ptr::Thin` trivially holds
    |
196 -         let wide_ptr: *mut str = ptr::from_raw_parts_mut(buf.as_mut_ptr().cast(), LEN);
196 +         let wide_ptr: *mut str = ptr::from_raw_parts_mut(buf.as_mut_ptr(), LEN);
    |
lcnr commented 3 months ago

@lcnr any idea how to get a reproducible example for this? I tried this code but it doesn't trigger the error -- probably things get normalized earlier already so by the time we check the trait type, everything is already equal.

#![feature(unboxed_closures)]
#![feature(ptr_metadata)]

type T1<'a> = &'a dyn for<'b> Fn(usize);
type T2<'a> = &'a dyn for<'b> Fn<(<[std::boxed::Box<i32>] as std::ptr::Pointee>::Metadata,), Output=()>;

fn main() {
    let x: T1 = &|_| {};
    let y: T2 = unsafe { std::mem::transmute(x) };
    y(42);
}

unfortunately not, my idea was something like the following, as types from the HIR get normalized, but that didn't seem to do the trick :thinking:

#![feature(ptr_metadata)]
type T1<'a> = &'a dyn for<'b> Fn(usize);
type T2<'a> = &'a dyn for<'b> Fn(<[std::boxed::Box<i32>] as std::ptr::Pointee>::Metadata);

#[repr(transparent)]
struct Foo<'a> {
    x: &'a T2<'a>,
}

fn main() {
    let foo = Foo { x: &((&|_| ()) as &dyn Fn(usize)) };
    let x: T1<'_> = foo.x;
    x(42);
}
RalfJung commented 3 months ago

I got something by minimizing emplacable.

#![feature(ptr_metadata)]
// This test is the result of minimizing the `emplacable` crate to reproduce
// <https://github.com/rust-lang/miri/issues/3541>.

use std::{ops::FnMut, ptr::Pointee};

pub type EmplacerFn<'a, T> = dyn for<'b> FnMut(<T as Pointee>::Metadata) + 'a;

#[repr(transparent)]
pub struct Emplacer<'a, T>(EmplacerFn<'a, T>)
where
    T: ?Sized;

impl<'a, T> Emplacer<'a, T>
where
    T: ?Sized,
{
    pub unsafe fn from_fn<'b>(emplacer_fn: &'b mut EmplacerFn<'a, T>) -> &'b mut Self {
        unsafe { &mut *((emplacer_fn as *mut EmplacerFn<'a, T>) as *mut Self) }
    }
}

pub fn box_new_with<T>()
where
    T: ?Sized,
{
    let emplacer_closure = &mut |_meta| {
        unreachable!();
    };

    unsafe { Emplacer::<T>::from_fn(emplacer_closure) };
}

fn main() {
    box_new_with::<[Box<i32>]>();
}

@Jules-Bertholet thanks for making this a small self-contained crate, minimization would probably not have been feasible otherwise. :)

Jules-Bertholet commented 3 months ago

I get a build error instead

Such is the way of nightly-only crates… It's fixed now