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

projecting to assoc type of supertrait that is implemented differently for trait object goes wrong #131891

Open
lukas-code opened this issue Oct 18, 2024 · 4 comments
Assignees
Labels
A-associated-items Area: Associated items (types, constants & functions) A-trait-objects Area: trait objects, vtable layout C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@lukas-code
Copy link
Member

lukas-code commented Oct 18, 2024

I tried this code: playground

#![feature(ptr_metadata)]

use std::ptr::Pointee;

trait Trait: Pointee {
    fn meta(&self) -> Self::Metadata;
}

impl Trait for () {
    fn meta(&self) {}
}

fn main() {
    let d: &dyn Trait<Metadata = ()> = &();
    let m = d.meta();
    dbg!(m);
}

I expected to see this happen: The type of m is () (or Trait is dyn incompatible).

Instead, this happened: The type of m is DynMetadata<dyn Trait<Metadata = ()>>.

Presumably this affects all traits with #[rustc_deny_explicit_impl(implement_via_object = false)] and associated types.

Miri does report UB for this code:

error: Undefined Behavior: calling a function with return type () passing return place of type std::ptr::DynMetadata<dyn Trait<Metadata = ()>>
  --> src/main.rs:15:13
   |
15 |     let m = d.meta();
   |             ^^^^^^^^ calling a function with return type () passing return place of type std::ptr::DynMetadata<dyn Trait<Metadata = ()>>
   |
   = 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
   = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets
   = help: if you think this code should be accepted anyway, please report an issue with Miri
   = note: BACKTRACE:
   = note: inside `main` at src/main.rs:15:13: 15:21

Meta

playground nightly

Build using the Nightly version: 1.84.0-nightly

(2024-10-17 3ed6e3cc69857129c1d3)

@rustbot label A-trait-objects A-associated-items T-types I-unsound requires-nightly -needs-triage

@lukas-code lukas-code added the C-bug Category: This is a bug. label Oct 18, 2024
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-associated-items Area: Associated items (types, constants & functions) A-trait-objects Area: trait objects, vtable layout I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-types Relevant to the types team, which will review and decide on the PR/issue. labels Oct 18, 2024
@lukas-code lukas-code removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 18, 2024
@compiler-errors
Copy link
Member

Reminds me of the unsoundness with Any trait overlap except for projections, and in this case an overlap between built-in impls and objects... I'll look into it later today after breakfast.

@rustbot claim

@compiler-errors
Copy link
Member

We have two choices:

  1. Treat all #[rustc_deny_explicit_impl(implement_via_object = false)] as object-unsafe.
  2. Reject supertrait associated types from signatures if they come from supertraits that are #[rustc_deny_explicit_impl(implement_via_object = false)].

@lukas-code
Copy link
Member Author

  1. Treat assoc items of #[rustc_deny_explicit_impl(implement_via_object = false)] traits as not coming from a supertrait for the dyn compatibility check only.
    (i.e. ignore supertraits with implement_via_object = false here:
    // Compute supertraits of current trait lazily.
    if self.supertraits.is_none() {
    self.supertraits = Some(
    util::supertraits(
    self.tcx,
    ty::Binder::dummy(ty::TraitRef::identity(
    self.tcx,
    self.trait_def_id,
    )),
    )
    .map(|trait_ref| {
    self.tcx.erase_regions(
    self.tcx.instantiate_bound_regions_with_erased(trait_ref),
    )
    })
    .collect(),
    );
    }
    // Determine whether the trait reference `Foo as
    // SomeTrait` is in fact a supertrait of the
    // current trait. In that case, this type is
    // legal, because the type `X` will be specified
    // in the object type. Note that we can just use
    // direct equality here because all of these types
    // are part of the formal parameter listing, and
    // hence there should be no inference variables.
    let is_supertrait_of_current_trait =
    self.supertraits.as_ref().unwrap().contains(
    &data.trait_ref(self.tcx).fold_with(
    &mut EraseEscapingBoundRegions {
    tcx: self.tcx,
    binder: ty::INNERMOST,
    },
    ),
    );
    )

@compiler-errors
Copy link
Member

Sorry, yes, that's what I meant for (2.). Specifically, they're as-if they came from a non supertrait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) A-trait-objects Area: trait objects, vtable layout C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants