Skip to content

Forbid freely casting lifetime bounds of dyn-types #136776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
BoxyUwU wants to merge 6 commits into
base: main
Choose a base branch
from
+635 −21
Open

Forbid freely casting lifetime bounds of dyn-types #136776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0b2db02
Forbid object lifetime changing pointer casts
BoxyUwU Feb 9, 2025
2eace27
Add test for principal-less cast
Darksonn Nov 21, 2025
8a06cde
Add check for non-principal trait casts
Darksonn Nov 21, 2025
152a6f1
Test for casts removing principal trait
Darksonn Nov 24, 2025
5a4d526
tidy
BoxyUwU Dec 4, 2025
06e4c91
reviews
BoxyUwU Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ impl<'tcx> BorrowExplanation<'tcx> {
cx.add_sized_or_copy_bound_info(err, category, &path);

if let ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: _,
is_implicit_coercion: true,
unsize_to: Some(unsize_ty),
} = category
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/region_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,23 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
self.add_placeholder_from_predicate_note(&mut diag, &path);
self.add_sized_or_copy_bound_info(&mut diag, category, &path);

for constraint in &path {
if let ConstraintCategory::Cast { is_raw_ptr_dyn_type_cast: true, .. } =
constraint.category
{
diag.span_note(
constraint.span,
format!("raw pointer casts of trait objects cannot extend lifetimes"),
);
diag.note(format!(
"this was previously accepted by the compiler but was changed recently"
));
diag.help(format!(
"see <https://github.com/rust-lang/rust/issues/141402> for more information"
));
}
}

self.buffer_error(diag);
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1697,6 +1697,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// should be as limited as possible; the note is prone to false positives and this
// constraint usually isn't best to blame.
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: _,
unsize_to: Some(unsize_ty),
is_implicit_coercion: true,
} if to_region == self.universal_regions().fr_static
Expand Down
89 changes: 76 additions & 13 deletions compiler/rustc_borrowck/src/type_check/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1106,15 +1106,23 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
self.prove_predicate(
ty::ClauseKind::WellFormed(src_ty.into()),
location.to_locations(),
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: false,
is_implicit_coercion,
unsize_to: None,
},
);

let src_ty = self.normalize(src_ty, location);
if let Err(terr) = self.sub_types(
src_ty,
*ty,
location.to_locations(),
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: false,
is_implicit_coercion,
unsize_to: None,
},
) {
span_mirbug!(
self,
Expand All @@ -1135,7 +1143,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
self.prove_predicate(
ty::ClauseKind::WellFormed(src_ty.into()),
location.to_locations(),
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: false,
is_implicit_coercion,
unsize_to: None,
},
);

// The type that we see in the fcx is like
Expand All @@ -1148,7 +1160,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
src_ty,
*ty,
location.to_locations(),
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: false,
is_implicit_coercion,
unsize_to: None,
},
) {
span_mirbug!(
self,
Expand Down Expand Up @@ -1177,7 +1193,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
ty_fn_ptr_from,
*ty,
location.to_locations(),
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: false,
is_implicit_coercion,
unsize_to: None,
},
) {
span_mirbug!(
self,
Expand Down Expand Up @@ -1210,7 +1230,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
ty_fn_ptr_from,
*ty,
location.to_locations(),
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: false,
is_implicit_coercion,
unsize_to: None,
},
) {
span_mirbug!(
self,
Expand Down Expand Up @@ -1239,6 +1263,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
trait_ref,
location.to_locations(),
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: false,
is_implicit_coercion,
unsize_to: Some(unsize_to),
},
Expand All @@ -1264,7 +1289,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
*ty_from,
*ty_to,
location.to_locations(),
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: false,
is_implicit_coercion,
unsize_to: None,
},
) {
span_mirbug!(
self,
Expand Down Expand Up @@ -1327,7 +1356,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
*ty_elem,
*ty_to,
location.to_locations(),
ConstraintCategory::Cast { is_implicit_coercion, unsize_to: None },
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: false,
is_implicit_coercion,
unsize_to: None,
},
) {
span_mirbug!(
self,
Expand Down Expand Up @@ -1484,11 +1517,12 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
trait_ref,
location.to_locations(),
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: false,
is_implicit_coercion: true,
unsize_to: None,
},
);
} else if let ty::Dynamic(src_tty, _src_lt) =
} else if let ty::Dynamic(src_tty, src_lt) =
*self.struct_tail(src.ty, location).kind()
&& let ty::Dynamic(dst_tty, dst_lt) =
*self.struct_tail(dst.ty, location).kind()
Expand All @@ -1503,15 +1537,13 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
// Debug`) are in `rustc_hir_typeck`.

// Remove auto traits.
// Auto trait checks are handled in `rustc_hir_typeck` as FCW.
// Auto trait checks are handled in `rustc_hir_typeck`.
let src_obj = Ty::new_dynamic(
tcx,
tcx.mk_poly_existential_predicates(
&src_tty.without_auto_traits().collect::<Vec<_>>(),
),
// FIXME: Once we disallow casting `*const dyn Trait + 'short`
// to `*const dyn Trait + 'long`, then this can just be `src_lt`.
dst_lt,
src_lt,
);
let dst_obj = Ty::new_dynamic(
tcx,
Expand All @@ -1523,16 +1555,47 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {

debug!(?src_tty, ?dst_tty, ?src_obj, ?dst_obj);

// Trait parameters are invariant, the only part that actually has
// subtyping here is the lifetime bound of the dyn-type.
//
// For example in `dyn Trait<'a> + 'b <: dyn Trait<'c> + 'd` we would
// require that `'a == 'c` but only that `'b: 'd`.
//
// We must not allow freely casting lifetime bounds of dyn-types as it
// may allow for inaccessible VTable methods being callable: #136702
self.sub_types(
src_obj,
dst_obj,
location.to_locations(),
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: true,
is_implicit_coercion: false,
unsize_to: None,
},
)
.unwrap();
} else if let ty::Dynamic(src_tty, src_lt) =
*self.struct_tail(src.ty, location).kind()
&& let ty::Dynamic(dst_tty, dst_lt) =
*self.struct_tail(dst.ty, location).kind()
&& src_tty.principal().is_none()
&& dst_tty.principal().is_none()
Comment on lines +1581 to +1582
Copy link
Member Author

@BoxyUwU BoxyUwU Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Darksonn I think this currently means that casting something like: *mut dyn Trait + Send + 'a to *mut Send + 'b won't check 'a: 'b. We ought to drop the src_tty.principal().is_none() check here, unless I'm misreading this code :)

Copy link
Contributor

@Darksonn Darksonn Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you help come up with a test that catches this? I tried adding a few, but I got the results I expected.

Copy link
Member Author

@BoxyUwU BoxyUwU Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotta say I would have expected those tests to not be emitting errors right now and can't figure out just from skimming the code why they wouldn't be 🤔

Copy link
Member Author

@BoxyUwU BoxyUwU Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, it just happens to work due to MIR lowering jank. doing this kind of cast seems to result in both an Unsize cast to drop the principal, and then also a PtrToPtr cast where neither side has a principal (which then gets lifetime checked from ur changes)

Copy link
Member Author

@BoxyUwU BoxyUwU Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

fn unprincipled3_static<'a>(x: *mut (dyn Trait + Send + 'a)) -> *mut (dyn Send + 'static) {
    x as _
}
fn unprincipled3_static(_1: *mut dyn Trait + Send) -> *mut dyn Send {
    debug x => _1;                       // in scope 0 at ./qux.rs:2:29: 2:30
    let mut _0: *mut dyn std::marker::Send; // return place in scope 0 at ./qux.rs:2:65: 2:90
    let mut _2: *mut dyn std::marker::Send; // in scope 0 at ./qux.rs:3:5: 3:11
    let mut _3: *mut dyn std::marker::Send; // in scope 0 at ./qux.rs:3:5: 3:11
    let mut _4: *mut dyn std::marker::Send; // in scope 0 at ./qux.rs:3:5: 3:11
    let mut _5: *mut dyn Trait + std::marker::Send; // in scope 0 at ./qux.rs:3:5: 3:6

        _5 = copy _1;                    // scope 0 at ./qux.rs:3:5: 3:6
        _4 = move _5 as *mut dyn std::marker::Send (PointerCoercion(Unsize, AsCast)); // scope 0 at ./qux.rs:3:5: 3:11
        StorageDead(_5);                 // scope 0 at ./qux.rs:3:5: 3:6
        _3 = move _4 as *mut dyn std::marker::Send (PtrToPtr); // scope 0 at ./qux.rs:3:5: 3:11
        AscribeUserType(_3, o, UserTypeProjection { base: UserType(0), projs: [] }); // scope 0 at ./qux.rs:3:10: 3:11
        _2 = copy _3;                    // scope 0 at ./qux.rs:3:5: 3:11
        StorageDead(_4);                 // scope 0 at ./qux.rs:3:10: 3:11
        _0 = move _2 as *mut dyn std::marker::Send (PointerCoercion(Unsize, Implicit)); // scope 0 at ./qux.rs:3:5: 3:11

this is the mir from that test case. note the 3 separate pointer casts, one of which is a PtrToPtr between two pointers to dyn Send trait objects xd

Copy link
Member Author

@BoxyUwU BoxyUwU Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could ICE on encountering (Some, None), run crater and if anything blows up then you've got a test case, otherwise just leave it like that and then in the future if someone files an ICE report we have a test case :>

Alternatively we just write the code to handle (Some, None) properly and then hope it's correct without a test case

Copy link
Contributor

@Darksonn Darksonn Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you expect uncovered (Some, None) cases to ICE rather than be accepted without error?

Copy link
Contributor

@Darksonn Darksonn Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense that it happens in the coercion. After all, ptr-to-ptr casts are only possible in cases where the vtable is the same, but I would not expect dyn T1 + Send to have the same vtable as dyn Send. Especially considering that implies dyn T1 + Send has same vtable as dyn Send has same vtable as dyn T2 + Send. So such ptr-to-ptr casts probably simply do not exist.

Copy link
Member Author

@BoxyUwU BoxyUwU Nov 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic makes sense to me 👍 Thank you

Copy link
Contributor

@lcnr lcnr Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe that's true. dyn Trait + Send to dyn Send iirc doesn't change the vtable. The vtable of objects without principals is just size+align+drop_fn or sth which is the same for all traits so iirc @compiler-errors implemented trait upcasting to principal-less traits to just keep the old vtable

{
// The principalless (no non-auto traits) case:
// You can only cast `dyn Send + 'long` to `dyn Send + 'short`.
self.constraints.outlives_constraints.push(OutlivesConstraint {
sup: src_lt.as_var(),
sub: dst_lt.as_var(),
locations: location.to_locations(),
span: location.to_locations().span(self.body),
category: ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: true,
is_implicit_coercion: false,
unsize_to: None,
},
variance_info: ty::VarianceDiagInfo::default(),
from_closure: false,
});
}
}
CastKind::Transmute => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ pub enum ConstraintCategory<'tcx> {
UseAsStatic,
TypeAnnotation(AnnotationSource),
Cast {
is_raw_ptr_dyn_type_cast: bool,
/// Whether this cast is a coercion that was automatically inserted by the compiler.
is_implicit_coercion: bool,
/// Whether this is an unsizing coercion and if yes, this contains the target type.
Expand Down
7 changes: 6 additions & 1 deletion library/std/src/thread/lifecycle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,12 @@ where
// SAFETY: dynamic size and alignment of the Box remain the same. See below for why the
// lifetime change is justified.
let rust_start = unsafe {
Box::from_raw(Box::into_raw(Box::new(rust_start)) as *mut (dyn FnOnce() + Send + 'static))
let ptr = Box::into_raw(Box::new(rust_start));
let ptr = crate::mem::transmute::<
*mut (dyn FnOnce() + Send + '_),
*mut (dyn FnOnce() + Send + 'static),
>(ptr);
Box::from_raw(ptr)
};

let init = Box::new(ThreadInit { handle: thread.clone(), rust_start });
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/cast/ptr-ptr-to-ptr-ptr-different-regions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//@ check-pass

Copy link
Contributor

@lcnr lcnr Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment pls

trait Trait {
fn foo(&self) {}
}

fn bar<'a>(a: *mut *mut (dyn Trait + 'a)) -> *mut *mut (dyn Trait + 'static) {
a as _
}

fn main() {}
7 changes: 4 additions & 3 deletions tests/ui/cast/ptr-to-ptr-different-regions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//@ check-pass

// https://github.com/rust-lang/rust/issues/113257

#![deny(trivial_casts)] // The casts here are not trivial.

struct Foo<'a> { a: &'a () }
struct Foo<'a> {
a: &'a (),
}

fn extend_lifetime_very_very_safely<'a>(v: *const Foo<'a>) -> *const Foo<'static> {
// This should pass because raw pointer casts can do anything they want.
Expand All @@ -15,6 +15,7 @@ trait Trait {}

fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'static) {
ptr as _
//~^ ERROR: lifetime may not live long enough
}

fn main() {
Expand Down
31 changes: 31 additions & 0 deletions tests/ui/cast/ptr-to-ptr-different-regions.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
error: lifetime may not live long enough
--> $DIR/ptr-to-ptr-different-regions.rs:17:5
|
LL | fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'static) {
| -- lifetime `'a` defined here
LL | ptr as _
| ^^^^^^^^ returning this value requires that `'a` must outlive `'static`
|
= note: requirement occurs because of a mutable pointer to `dyn Trait`
= note: mutable pointers are invariant over their type parameter
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
note: raw pointer casts of trait objects cannot extend lifetimes
--> $DIR/ptr-to-ptr-different-regions.rs:17:5
|
LL | ptr as _
| ^^^^^^^^
= note: this was previously accepted by the compiler but was changed recently
= help: see <https://github.com/rust-lang/rust/issues/141402> for more information
help: consider changing the trait object's explicit `'static` bound to the lifetime of argument `ptr`
|
LL - fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'static) {
LL + fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'a) {
|
help: alternatively, add an explicit `'static` bound to this reference
|
LL - fn assert_static<'a>(ptr: *mut (dyn Trait + 'a)) -> *mut (dyn Trait + 'static) {
LL + fn assert_static<'a>(ptr: *mut (dyn Trait + 'static)) -> *mut (dyn Trait + 'static) {
|

error: aborting due to 1 previous error

12 changes: 12 additions & 0 deletions tests/ui/cast/ptr-to-ptr-indirect-different-regions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
trait Trait {
fn foo(&self) {}
}

Copy link
Contributor

@lcnr lcnr Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same, maybe also merge with the above test?

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

fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'a)>) -> *mut MyWrap<(dyn Trait + 'static)> {
a as _
//~^ ERROR: lifetime may not live long enough
}

fn main() {}
31 changes: 31 additions & 0 deletions tests/ui/cast/ptr-to-ptr-indirect-different-regions.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
error: lifetime may not live long enough
--> $DIR/ptr-to-ptr-indirect-different-regions.rs:8:5
|
LL | fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'a)>) -> *mut MyWrap<(dyn Trait + 'static)> {
| -- lifetime `'a` defined here
LL | a as _
| ^^^^^^ returning this value requires that `'a` must outlive `'static`
|
= note: requirement occurs because of a mutable pointer to `MyWrap<dyn Trait>`
= note: mutable pointers are invariant over their type parameter
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
note: raw pointer casts of trait objects cannot extend lifetimes
--> $DIR/ptr-to-ptr-indirect-different-regions.rs:8:5
|
LL | a as _
| ^^^^^^
= note: this was previously accepted by the compiler but was changed recently
= help: see <https://github.com/rust-lang/rust/issues/141402> for more information
help: consider changing the trait object's explicit `'static` bound to the lifetime of argument `a`
|
LL - fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'a)>) -> *mut MyWrap<(dyn Trait + 'static)> {
LL + fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'a)>) -> *mut MyWrap<(dyn Trait + 'a)> {
|
help: alternatively, add an explicit `'static` bound to this reference
|
LL - fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'a)>) -> *mut MyWrap<(dyn Trait + 'static)> {
LL + fn bar<'a>(a: *mut MyWrap<(dyn Trait + 'static)>) -> *mut MyWrap<(dyn Trait + 'static)> {
|

error: aborting due to 1 previous error

Loading
Loading