Skip to content

[WIP] Forbid object lifetime changing pointer casts #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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
+249 −21
Draft

[WIP] Forbid object lifetime changing pointer casts #136776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19659db
Forbid object lifetime changing pointer casts
BoxyUwU Feb 9, 2025
cfa1e3a
Custom diagnostics
BoxyUwU May 22, 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 @@ -565,6 +565,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 do not cast away 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 @@ -2106,6 +2106,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 target_region == self.universal_regions().fr_static
Expand Down
68 changes: 54 additions & 14 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 @@ -1104,15 +1104,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 @@ -1133,7 +1141,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 @@ -1146,7 +1158,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 @@ -1175,7 +1191,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 @@ -1208,7 +1228,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 @@ -1237,6 +1261,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 @@ -1260,7 +1285,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
.iter()
.map(|predicate| predicate.with_self_ty(tcx, self_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,
},
);

let outlives_predicate = tcx.mk_predicate(Binder::dummy(
Expand All @@ -1271,7 +1300,11 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
self.prove_predicate(
outlives_predicate,
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,
},
);
}

Expand All @@ -1294,7 +1327,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 @@ -1357,7 +1394,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 @@ -1505,7 +1546,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
//
// Note that other checks (such as denying `dyn Send` -> `dyn
// Debug`) are in `rustc_hir_typeck`.
if let ty::Dynamic(src_tty, _src_lt, ty::Dyn) = *src_tail.kind()
if let ty::Dynamic(src_tty, src_lt, ty::Dyn) = *src_tail.kind()
&& let ty::Dynamic(dst_tty, dst_lt, ty::Dyn) = *dst_tail.kind()
&& src_tty.principal().is_some()
&& dst_tty.principal().is_some()
Copy link
Member

Choose a reason for hiding this comment

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

Is the "when there aren't principal traits" still true? It seems like I could get into trouble with a fn (self: dyn Send + 'static), right?

Copy link
Member Author

Choose a reason for hiding this comment

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

We forbid casting dyn Send to dyn Trait in general as we would have no way to construct a vtable. The opposite cast is also fine as the vtable for dyn Send (or other autotraits) necessarily doesn't contain potentially user-written functions with where clauses that could rely on the dyn type's lifetime bound. I should write this rationale down somewhere

Expand All @@ -1517,9 +1558,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, '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,
ty::Dyn,
);
let dst_obj = Ty::new_dynamic(
Expand All @@ -1538,6 +1577,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
dst_obj,
location.to_locations(),
ConstraintCategory::Cast {
is_raw_ptr_dyn_type_cast: true,
is_implicit_coercion: false,
unsize_to: None,
},
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 @@ -116,6 +116,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: 5 additions & 2 deletions library/std/src/thread/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,8 +578,11 @@ impl Builder {
let main = Box::new(main);
// SAFETY: dynamic size and alignment of the Box remain the same. See below for why the
// lifetime change is justified.
let main =
unsafe { Box::from_raw(Box::into_raw(main) as *mut (dyn FnOnce() + Send + 'static)) };
let main = unsafe {
let ptr = Box::into_raw(main) as *mut (dyn FnOnce() + Send + '_);
let ptr: *mut (dyn FnOnce() + Send + 'static) = crate::mem::transmute(ptr);
Box::from_raw(ptr)
};

Ok(JoinInner {
// SAFETY:
Expand Down
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 do not cast away 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

7 changes: 7 additions & 0 deletions tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.current.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ LL | let _send = unsend as *const S<dyn Cat<'static>>;
= note: requirement occurs because of the type `S<dyn Cat<'_>>`, which makes the generic argument `dyn Cat<'_>` invariant
= note: the struct `S<T>` is invariant over the parameter `T`
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
note: raw pointer casts of trait objects do not cast away lifetimes
--> $DIR/ptr-to-trait-obj-different-regions-id-trait.rs:24:17
|
LL | let _send = unsend as *const S<dyn Cat<'static>>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= 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

error: aborting due to 1 previous error

7 changes: 7 additions & 0 deletions tests/ui/cast/ptr-to-trait-obj-different-regions-id-trait.next.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ LL | let _send = unsend as *const S<dyn Cat<'static>>;
= note: requirement occurs because of the type `S<dyn Cat<'_>>`, which makes the generic argument `dyn Cat<'_>` invariant
= note: the struct `S<T>` is invariant over the parameter `T`
= help: see <https://doc.rust-lang.org/nomicon/subtyping.html> for more information about variance
note: raw pointer casts of trait objects do not cast away lifetimes
--> $DIR/ptr-to-trait-obj-different-regions-id-trait.rs:24:17
|
LL | let _send = unsend as *const S<dyn Cat<'static>>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= 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

error: aborting due to 1 previous error

8 changes: 8 additions & 0 deletions tests/ui/cast/ptr-to-trait-obj-different-regions-lt-ext.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ LL | fn bad_cast<'a>(x: *const dyn Static<'static>) -> *const dyn Static<'a> {
| -- lifetime `'a` defined here
LL | x as _
| ^^^^^^ returning this value requires that `'a` must outlive `'static`
|
note: raw pointer casts of trait objects do not cast away lifetimes
--> $DIR/ptr-to-trait-obj-different-regions-lt-ext.rs:12:5
|
LL | x 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

error: aborting due to 1 previous error

Loading
Loading