Skip to content

Commit

Permalink
Auto merge of #120248 - WaffleLapkin:bonk-ptr-object-casts, r=<try>
Browse files Browse the repository at this point in the history
Make casts of pointers to trait objects stricter

This is an attempt to `fix` #120222 and #120217.

cc `@oli-obk` `@compiler-errors` `@lcnr`
  • Loading branch information
bors committed Jan 22, 2024
2 parents 021861a + 3c3cf17 commit 2c99da5
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 31 deletions.
47 changes: 36 additions & 11 deletions compiler/rustc_hir_typeck/src/cast.rs
Expand Up @@ -42,7 +42,7 @@ use rustc_middle::ty::cast::{CastKind, CastTy};
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::{self, Ty, TypeAndMut, TypeVisitableExt, VariantDef};
use rustc_session::lint;
use rustc_span::def_id::{DefId, LOCAL_CRATE};
use rustc_span::def_id::LOCAL_CRATE;
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_trait_selection::infer::InferCtxtExt;
Expand Down Expand Up @@ -72,7 +72,7 @@ enum PointerKind<'tcx> {
/// No metadata attached, ie pointer to sized type or foreign type
Thin,
/// A trait object
VTable(Option<DefId>),
VTable(Option<ty::Binder<'tcx, ty::ExistentialTraitRef<'tcx>>>),
/// Slice
Length,
/// The unsize info of this projection or opaque type
Expand Down Expand Up @@ -100,7 +100,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

Ok(match *t.kind() {
ty::Slice(_) | ty::Str => Some(PointerKind::Length),
ty::Dynamic(tty, _, ty::Dyn) => Some(PointerKind::VTable(tty.principal_def_id())),
ty::Dynamic(tty, _, ty::Dyn) => Some(PointerKind::VTable(tty.principal())),
ty::Adt(def, args) if def.is_struct() => match def.non_enum_variant().tail_opt() {
None => Some(PointerKind::Thin),
Some(f) => {
Expand Down Expand Up @@ -611,14 +611,39 @@ impl<'a, 'tcx> CastCheck<'tcx> {
} else {
match self.try_coercion_cast(fcx) {
Ok(()) => {
if self.expr_ty.is_unsafe_ptr() && self.cast_ty.is_unsafe_ptr() {
// When casting a raw pointer to another raw pointer, we cannot convert the cast into
// a coercion because the pointee types might only differ in regions, which HIR typeck
// cannot distinguish. This would cause us to erroneously discard a cast which will
// lead to a borrowck error like #113257.
// We still did a coercion above to unify inference variables for `ptr as _` casts.
// This does cause us to miss some trivial casts in the trival cast lint.
debug!(" -> PointerCast");
if let ty::RawPtr(src_pointee) = self.expr_ty.kind()
&& let ty::RawPtr(tgt_pointee) = self.cast_ty.kind()
{
if let Ok(Some(src_kind)) = fcx.pointer_kind(src_pointee.ty, self.expr_span)
&& let Ok(Some(tgt_kind)) =
fcx.pointer_kind(tgt_pointee.ty, self.cast_span)
{
match (src_kind, tgt_kind) {
// When casting a raw pointer to another raw pointer, we cannot convert the cast into
// a coercion because the pointee types might only differ in regions, which HIR typeck
// cannot distinguish. This would cause us to erroneously discard a cast which will
// lead to a borrowck error like #113257.
// We still did a coercion above to unify inference variables for `ptr as _` casts.
// This does cause us to miss some trivial casts in the trivial cast lint.
(PointerKind::Thin, PointerKind::Thin)
| (PointerKind::Length, PointerKind::Length) => {
debug!(" -> PointerCast");
}

// If we are not casting pointers to sized types or slice-ish DSTs
// (handled above), we need to make a coercion cast. This prevents
// casts like `*const dyn Trait<'a> -> *const dyn Trait<'b>` which
// are unsound.
//
// See <https://github.com/rust-lang/rust/issues/120217>
(_, _) => {
debug!(" -> CoercionCast");
fcx.typeck_results
.borrow_mut()
.set_coercion_cast(self.expr.hir_id.local_id);
}
}
}
} else {
self.trivial_cast_lint(fcx);
debug!(" -> CoercionCast");
Expand Down
6 changes: 4 additions & 2 deletions library/std/src/thread/mod.rs
Expand Up @@ -164,6 +164,7 @@ use crate::ffi::{CStr, CString};
use crate::fmt;
use crate::io;
use crate::marker::PhantomData;
use crate::mem::transmute;
use crate::mem::{self, forget};
use crate::num::NonZeroU64;
use crate::num::NonZeroUsize;
Expand Down Expand Up @@ -545,10 +546,11 @@ impl Builder {
scope_data.increment_num_running_threads();
}

let main = Box::new(main);
let main: Box<dyn FnOnce() + '_> = 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() + 'static)) };
let main =
unsafe { transmute::<Box<dyn FnOnce() + '_>, Box<dyn FnOnce() + 'static>>(main) };

Ok(JoinInner {
// SAFETY:
Expand Down
12 changes: 0 additions & 12 deletions tests/ui/cast/cast-rfc0401-vtable-kinds.rs
Expand Up @@ -16,13 +16,6 @@ impl<T> Foo<T> for () {}
impl Foo<u32> for u32 { fn foo(&self, _: u32) -> u32 { self+43 } }
impl Bar for () {}

unsafe fn round_trip_and_call<'a>(t: *const (dyn Foo<u32>+'a)) -> u32 {
let foo_e : *const dyn Foo<u16> = t as *const _;
let r_1 = foo_e as *mut dyn Foo<u32>;

(&*r_1).foo(0)
}

#[repr(C)]
struct FooS<T:?Sized>(T);
#[repr(C)]
Expand All @@ -38,11 +31,6 @@ fn tuple_i32_to_u32<T:?Sized>(u: *const (i32, T)) -> *const (u32, T) {


fn main() {
let x = 4u32;
let y : &dyn Foo<u32> = &x;
let fl = unsafe { round_trip_and_call(y as *const dyn Foo<u32>) };
assert_eq!(fl, (43+4));

let s = FooS([0,1,2]);
let u: &FooS<[u32]> = &s;
let u: *const FooS<[u32]> = u;
Expand Down
6 changes: 0 additions & 6 deletions tests/ui/cast/ptr-to-ptr-different-regions.rs
Expand Up @@ -11,12 +11,6 @@ fn extend_lifetime_very_very_safely<'a>(v: *const Foo<'a>) -> *const Foo<'static
v as *const Foo<'static>
}

trait Trait {}

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

fn main() {
let unit = ();
let foo = Foo { a: &unit };
Expand Down
30 changes: 30 additions & 0 deletions tests/ui/cast/ptr-to-trait-obj-different-args.rs
@@ -0,0 +1,30 @@
// check-fail
//
// issue: <https://github.com/rust-lang/rust/issues/120222>


trait A {}
impl<T> A for T {}
trait B {}
impl<T> B for T {}

trait Trait<G> {}
struct X;
impl<T> Trait<X> for T {}
struct Y;
impl<T> Trait<Y> for T {}

fn main() {
let a: *const dyn A = &();
let b: *const dyn B = a as _; //~ error: casting `*const dyn A` as `*const dyn B` is invalid

let x: *const dyn Trait<X> = &();
let y: *const dyn Trait<Y> = x as _; //~ error: casting `*const dyn Trait<X>` as `*const dyn Trait<Y>` is invalid

_ = (b, y);
}

fn generic<T>(x: *const dyn Trait<X>, t: *const dyn Trait<T>) {
let _: *const dyn Trait<T> = x as _; //~ error: casting `*const (dyn Trait<X> + 'static)` as `*const dyn Trait<T>` is invalid
let _: *const dyn Trait<X> = t as _; //~ error: casting `*const (dyn Trait<T> + 'static)` as `*const dyn Trait<X>` is invalid
}
35 changes: 35 additions & 0 deletions tests/ui/cast/ptr-to-trait-obj-different-args.stderr
@@ -0,0 +1,35 @@
error[E0606]: casting `*const dyn A` as `*const dyn B` is invalid
--> $DIR/ptr-to-trait-obj-different-args.rs:19:27
|
LL | let b: *const dyn B = a as _;
| ^^^^^^
|
= note: vtable kinds may not match

error[E0606]: casting `*const dyn Trait<X>` as `*const dyn Trait<Y>` is invalid
--> $DIR/ptr-to-trait-obj-different-args.rs:22:34
|
LL | let y: *const dyn Trait<Y> = x as _;
| ^^^^^^
|
= note: vtable kinds may not match

error[E0606]: casting `*const (dyn Trait<X> + 'static)` as `*const dyn Trait<T>` is invalid
--> $DIR/ptr-to-trait-obj-different-args.rs:28:34
|
LL | let _: *const dyn Trait<T> = x as _;
| ^^^^^^
|
= note: vtable kinds may not match

error[E0606]: casting `*const (dyn Trait<T> + 'static)` as `*const dyn Trait<X>` is invalid
--> $DIR/ptr-to-trait-obj-different-args.rs:29:34
|
LL | let _: *const dyn Trait<X> = t as _;
| ^^^^^^
|
= note: vtable kinds may not match

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0606`.
31 changes: 31 additions & 0 deletions tests/ui/cast/ptr-to-trait-obj-different-regions.rs
@@ -0,0 +1,31 @@
// check-fail
//
// issue: <https://github.com/rust-lang/rust/issues/120217>

#![feature(arbitrary_self_types)]

trait Static<'a> {
fn proof(self: *const Self, s: &'a str) -> &'static str;
}

fn bad_cast<'a>(x: *const dyn Static<'static>) -> *const dyn Static<'a> {
x as _ //~ error: lifetime may not live long enough
}

impl Static<'static> for () {
fn proof(self: *const Self, s: &'static str) -> &'static str {
s
}
}

fn extend_lifetime(s: &str) -> &'static str {
bad_cast(&()).proof(s)
}

fn main() {
let s = String::from("Hello World");
let slice = extend_lifetime(&s);
println!("Now it exists: {slice}");
drop(s);
println!("Now it’s gone: {slice}");
}
10 changes: 10 additions & 0 deletions tests/ui/cast/ptr-to-trait-obj-different-regions.stderr
@@ -0,0 +1,10 @@
error: lifetime may not live long enough
--> $DIR/ptr-to-trait-obj-different-regions.rs:12:5
|
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`

error: aborting due to 1 previous error

0 comments on commit 2c99da5

Please sign in to comment.