Skip to content

Commit

Permalink
Rollup merge of #135296 - lukas-code:dyn-leak-check, r=compiler-errors
Browse files Browse the repository at this point in the history
interpret: adjust vtable validity check for higher-ranked types

## What

Transmuting between trait objects where a generic argument or associated type only differs in bound regions (not bound at or above the trait object's binder) is now UB. For example

* transmuting between `&dyn Trait<for<'a> fn(&'a u8)>` and `&dyn Trait<fn(&'static u8)>` is UB.
* transmuting between `&dyn Trait<Assoc = for<'a> fn(&'a u8)>` and `&dyn Trait<Assoc = fn(&'static u8)>` is UB.
* transmuting between `&dyn Trait<for<'a> fn(&'a u8) -> (&'a u8, &'static u8)>` and `&dyn Trait<for<'a> fn(&'a u8) -> (&'static u8, &'a u8)>` is UB.

Transmuting between subtypes (in either direction) is still allowed, which means that bound regions that are bound at or above the trait object's binder can still be changed:

* transmuting between `&dyn for<'a> Trait<fn(&'a u8)>` and `&dyn for Trait<fn(&'static u8)>` is fine.
* transmuting between `&dyn for<'a> Trait<dyn Trait<fn(&'a u8)>>` and `&dyn for Trait<dyn Trait<fn(&'static u8)>>` is fine.

## Why

Very similar to rust-lang/rust#120217 and rust-lang/rust#120222, changing a trait object's generic argument to a type that only differs in bound regions can still affect the vtable layout and lead to segfaults at runtime (for an example see `src/tools/miri/tests/fail/validity/dyn-transmute-inner-binder.rs`).

Since we already already require that the trait object predicates must be equal modulo bound regions, it is only natural to extend this check to also require type equality considering bound regions.

However, it also makes sense to allow transmutes between a type and a subtype thereof. For example `&dyn for<'a> Trait<&'a u8>` is a subtype of `&dyn Trait<&'static ()>` and they are guaranteed to have the same vtable, so it makes sense to allow this transmute. So that's why bound lifetimes that are bound to the trait object itself are treated as free lifetime for the purpose of this check.

Note that codegen already relies on the property that subtyping cannot change the the vtable and this is asserted here (note the leak check): https://github.com/rust-lang/rust/blob/251206c27b619ccf3a08e2ac4c525dc343f08492/compiler/rustc_codegen_ssa/src/base.rs#L106-L153

Furthermore, we allow some pointer-to-pointer casts like `*const dyn for<'a> Trait<&'a u8>` to `*const Wrapper<dyn Trait<&'static u8>>` that instantiate the trait object binder and are currently lowered to a single pointer-to-pointer cast in MIR (`CastKind::PtrToPtr`) and *not* an unsizing coercion (`CastKind::PointerCoercion(Unsize)`), so the current MIR lowering of these would be UB if we didn't allow subtyping transmutes.

---

fixes rust-lang/rust#135230
cc `@rust-lang/opsem`
r? `@compiler-errors` for the implementation
  • Loading branch information
matthiaskrgr authored Feb 19, 2025
2 parents 5dc0064 + 0cd96b6 commit 4e9829b
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 0 deletions.
30 changes: 30 additions & 0 deletions tests/fail/validity/dyn-transmute-inner-binder.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Test that transmuting from `&dyn Trait<fn(&'static ())>` to `&dyn Trait<for<'a> fn(&'a ())>` is UB.
//
// The vtable of `() as Trait<fn(&'static ())>` and `() as Trait<for<'a> fn(&'a ())>` can have
// different entries and, because in the former the entry for `foo` is vacant, this test will
// segfault at runtime.

trait Trait<U> {
fn foo(&self)
where
U: HigherRanked,
{
}
}
impl<T, U> Trait<U> for T {}

trait HigherRanked {}
impl HigherRanked for for<'a> fn(&'a ()) {}

// 2nd candidate is required so that selecting `(): Trait<fn(&'static ())>` will
// evaluate the candidates and fail the leak check instead of returning the
// only applicable candidate.
trait Unsatisfied {}
impl<T: Unsatisfied> HigherRanked for T {}

fn main() {
let x: &dyn Trait<fn(&'static ())> = &();
let y: &dyn Trait<for<'a> fn(&'a ())> = unsafe { std::mem::transmute(x) };
//~^ ERROR: wrong trait in wide pointer vtable
y.foo();
}
15 changes: 15 additions & 0 deletions tests/fail/validity/dyn-transmute-inner-binder.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: Undefined Behavior: constructing invalid value: wrong trait in wide pointer vtable: expected `Trait<for<'a> fn(&'a ())>`, but encountered `Trait<fn(&())>`
--> tests/fail/validity/dyn-transmute-inner-binder.rs:LL:CC
|
LL | let y: &dyn Trait<for<'a> fn(&'a ())> = unsafe { std::mem::transmute(x) };
| ^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: wrong trait in wide pointer vtable: expected `Trait<for<'a> fn(&'a ())>`, but encountered `Trait<fn(&())>`
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `main` at tests/fail/validity/dyn-transmute-inner-binder.rs:LL:CC

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

30 changes: 30 additions & 0 deletions tests/pass/dyn-upcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ fn main() {
drop_principal();
modulo_binder();
modulo_assoc();
bidirectional_subtyping();
}

fn vtable_nop_cast() {
Expand Down Expand Up @@ -531,3 +532,32 @@ fn modulo_assoc() {

(&() as &dyn Trait as &dyn Middle<()>).say_hello(&0);
}

fn bidirectional_subtyping() {
// Test that transmuting between subtypes of dyn traits is fine, even in the
// "wrong direction", i.e. going from a lower-ranked to a higher-ranked dyn trait.
// Note that compared to the `dyn-transmute-inner-binder` test, the `for` is on the
// *outside* here!

trait Trait<U: ?Sized> {}
impl<T, U: ?Sized> Trait<U> for T {}

struct Wrapper<T: ?Sized>(T);

let x: &dyn Trait<fn(&'static ())> = &();
let _y: &dyn for<'a> Trait<fn(&'a ())> = unsafe { std::mem::transmute(x) };

let x: &dyn for<'a> Trait<fn(&'a ())> = &();
let _y: &dyn Trait<fn(&'static ())> = unsafe { std::mem::transmute(x) };

let x: &dyn Trait<dyn Trait<fn(&'static ())>> = &();
let _y: &dyn for<'a> Trait<dyn Trait<fn(&'a ())>> = unsafe { std::mem::transmute(x) };

let x: &dyn for<'a> Trait<dyn Trait<fn(&'a ())>> = &();
let _y: &dyn Trait<dyn Trait<fn(&'static ())>> = unsafe { std::mem::transmute(x) };

// This lowers to a ptr-to-ptr cast (which behaves like a transmute)
// and not an unsizing coercion:
let x: *const dyn for<'a> Trait<&'a ()> = &();
let _y: *const Wrapper<dyn Trait<&'static ()>> = x as _;
}

0 comments on commit 4e9829b

Please sign in to comment.