Skip to content
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.

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

Improve DispatchFromDyn and CoerceUnsized impl validation #135228

Merged
merged 3 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ hir_analysis_dispatch_from_dyn_multi = implementing the `DispatchFromDyn` trait

hir_analysis_dispatch_from_dyn_repr = structs implementing `DispatchFromDyn` may not have `#[repr(packed)]` or `#[repr(C)]`

hir_analysis_dispatch_from_dyn_zst = the trait `DispatchFromDyn` may only be implemented for structs containing the field being coerced, ZST fields with 1 byte alignment, and nothing else
hir_analysis_dispatch_from_dyn_zst = the trait `DispatchFromDyn` may only be implemented for structs containing the field being coerced, ZST fields with 1 byte alignment that don't mention type/const generics, and nothing else
.note = extra field `{$name}` of type `{$ty}` is not allowed

hir_analysis_drop_impl_negative = negative `Drop` impls are not supported
Expand Down
42 changes: 34 additions & 8 deletions compiler/rustc_hir_analysis/src/coherence/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,19 +259,37 @@ fn visit_implementation_of_dispatch_from_dyn(checker: &Checker<'_>) -> Result<()
let coerced_fields = fields
.iter()
.filter(|field| {
// Ignore PhantomData fields
let unnormalized_ty = tcx.type_of(field.did).instantiate_identity();
if tcx
.try_normalize_erasing_regions(
ty::TypingEnv::non_body_analysis(tcx, def_a.did()),
unnormalized_ty,
)
.unwrap_or(unnormalized_ty)
.is_phantom_data()
{
return false;
}

let ty_a = field.ty(tcx, args_a);
let ty_b = field.ty(tcx, args_b);

if let Ok(layout) =
tcx.layout_of(infcx.typing_env(param_env).as_query_input(ty_a))
{
if layout.is_1zst() {
// FIXME: We could do normalization here, but is it really worth it?
if ty_a == ty_b {
// Allow 1-ZSTs that don't mention type params.
//
// Allowing type params here would allow us to possibly transmute
// between ZSTs, which may be used to create library unsoundness.
if let Ok(layout) =
tcx.layout_of(infcx.typing_env(param_env).as_query_input(ty_a))
&& layout.is_1zst()
&& !ty_a.has_non_region_param()
{
// ignore 1-ZST fields
return false;
}
}

if ty_a == ty_b {
res = Err(tcx.dcx().emit_err(errors::DispatchFromDynZST {
span,
name: field.name,
Expand Down Expand Up @@ -460,8 +478,16 @@ pub(crate) fn coerce_unsized_info<'tcx>(
.filter_map(|(i, f)| {
let (a, b) = (f.ty(tcx, args_a), f.ty(tcx, args_b));

if tcx.type_of(f.did).instantiate_identity().is_phantom_data() {
// Ignore PhantomData fields
// Ignore PhantomData fields
let unnormalized_ty = tcx.type_of(f.did).instantiate_identity();
if tcx
.try_normalize_erasing_regions(
ty::TypingEnv::non_body_analysis(tcx, def_a.did()),
unnormalized_ty,
)
.unwrap_or(unnormalized_ty)
.is_phantom_data()
{
return None;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/invalid_dispatch_from_dyn_impls.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error[E0378]: the trait `DispatchFromDyn` may only be implemented for structs containing the field being coerced, ZST fields with 1 byte alignment, and nothing else
error[E0378]: the trait `DispatchFromDyn` may only be implemented for structs containing the field being coerced, ZST fields with 1 byte alignment that don't mention type/const generics, and nothing else
--> $DIR/invalid_dispatch_from_dyn_impls.rs:10:1
|
LL | / impl<T, U> DispatchFromDyn<WrapperWithExtraField<U>> for WrapperWithExtraField<T>
Expand Down Expand Up @@ -35,7 +35,7 @@ LL | | where
LL | | T: Unsize<U>,
| |_________________^

error[E0378]: the trait `DispatchFromDyn` may only be implemented for structs containing the field being coerced, ZST fields with 1 byte alignment, and nothing else
error[E0378]: the trait `DispatchFromDyn` may only be implemented for structs containing the field being coerced, ZST fields with 1 byte alignment that don't mention type/const generics, and nothing else
--> $DIR/invalid_dispatch_from_dyn_impls.rs:46:1
|
LL | / impl<T: ?Sized, U: ?Sized> DispatchFromDyn<OverAligned<U>> for OverAligned<T>
Expand Down
25 changes: 25 additions & 0 deletions tests/ui/self/dispatch-from-dyn-zst-transmute-zst-nonzst.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// We used to allow erroneous `DispatchFromDyn` impls whose RHS type contained
// fields that weren't ZSTs. I don't believe this was possible to abuse, but
// it's at least nice to give users better errors.

#![feature(arbitrary_self_types)]
#![feature(unsize)]
#![feature(dispatch_from_dyn)]

use std::marker::Unsize;
use std::ops::DispatchFromDyn;

struct Dispatchable<T: ?Sized, Z> {
_ptr: Box<T>,
z: Z,
}

impl<T, U> DispatchFromDyn<Dispatchable<U, i32>> for Dispatchable<T, ()>
//~^ ERROR implementing the `DispatchFromDyn` trait requires multiple coercions
where
T: Unsize<U> + ?Sized,
U: ?Sized,
{
}

fn main() {}
16 changes: 16 additions & 0 deletions tests/ui/self/dispatch-from-dyn-zst-transmute-zst-nonzst.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0378]: implementing the `DispatchFromDyn` trait requires multiple coercions
--> $DIR/dispatch-from-dyn-zst-transmute-zst-nonzst.rs:17:1
|
LL | / impl<T, U> DispatchFromDyn<Dispatchable<U, i32>> for Dispatchable<T, ()>
LL | |
LL | | where
LL | | T: Unsize<U> + ?Sized,
LL | | U: ?Sized,
| |______________^
|
= note: the trait `DispatchFromDyn` may only be implemented for a coercion between structures with a single field being coerced
= note: currently, 2 fields need coercions: `_ptr` (`Box<T>` to `Box<U>`), `z` (`()` to `i32`)

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0378`.
34 changes: 34 additions & 0 deletions tests/ui/self/dispatch-from-dyn-zst-transmute.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#![feature(arbitrary_self_types)]
#![feature(unsize)]
#![feature(dispatch_from_dyn)]

use std::marker::PhantomData;
use std::marker::Unsize;
use std::ops::DispatchFromDyn;
use std::ops::Deref;

struct IsSendToken<T: ?Sized>(PhantomData<fn(T) -> T>);

struct Foo<'a, U: ?Sized> {
token: IsSendToken<U>,
ptr: &'a U,
}

impl<'a, T, U> DispatchFromDyn<Foo<'a, U>> for Foo<'a, T>
//~^ ERROR implementing the `DispatchFromDyn` trait requires multiple coercions
where
T: Unsize<U> + ?Sized,
U: ?Sized {}

trait Bar {
fn f(self: Foo<'_, Self>);
}

impl<U: ?Sized> Deref for Foo<'_, U> {
type Target = U;
fn deref(&self) -> &U {
self.ptr
}
}

fn main() {}
16 changes: 16 additions & 0 deletions tests/ui/self/dispatch-from-dyn-zst-transmute.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0378]: implementing the `DispatchFromDyn` trait requires multiple coercions
--> $DIR/dispatch-from-dyn-zst-transmute.rs:17:1
|
LL | / impl<'a, T, U> DispatchFromDyn<Foo<'a, U>> for Foo<'a, T>
LL | |
LL | | where
LL | | T: Unsize<U> + ?Sized,
LL | | U: ?Sized {}
| |_____________^
|
= note: the trait `DispatchFromDyn` may only be implemented for a coercion between structures with a single field being coerced
= note: currently, 2 fields need coercions: `token` (`IsSendToken<T>` to `IsSendToken<U>`), `ptr` (`&'a T` to `&'a U`)

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0378`.
25 changes: 25 additions & 0 deletions tests/ui/self/phantomdata-in-coerce-and-dispatch-impls.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//@ check-pass

#![feature(coerce_unsized, dispatch_from_dyn, unsize)]

use std::marker::Unsize;
use std::ops::{CoerceUnsized, DispatchFromDyn};
use std::marker::PhantomData;

trait Mirror {
type Assoc;
}
impl<T> Mirror for T {
type Assoc = T;
}

struct W<T: 'static> {
t: &'static T,
f: <PhantomData<T> as Mirror>::Assoc,
}

impl<T, U> CoerceUnsized<W<U>> for W<T> where T: Unsize<U> {}

impl<T, U> DispatchFromDyn<W<U>> for W<T> where T: Unsize<U> {}

fn main() {}
Loading