Trait objects can call nonexistent concrete methods

[matthewjasper]

This code segfaults at runtime

use std::{
    fmt::{self, Debug},
};

trait X {
    fn foo(&self) where Self: Debug;
}

impl X for fn(&()) {
    fn foo(&self) where fn(&()): Debug {}
}

impl Debug for dyn X {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "hi!")
    }
}

pub fn main() {
    let x: fn(&()) = |_| ();
    let y = &x as &dyn X;
    y.foo(); // Segfault at opt-level 0, SIGILL otherwise.
}

cc #48557 - Makes this easier to come across.

[leoyvens]

Edit 3: This really isn't the way to fix this issue, you may skip this comment.
Edit 2: I pursued this tentative fix in #50815 but it didn't actually work. Maybe this is not the way to fix it or deeper changes would be needed.

I did some investigation. The ideal fix is to catch this at declaration site, in the well-formedness (WF) check. Failing that we should be catching it at the call site, but that code dodged all checks.

Edit: Actually with #48214 in some cases that WF check is downgraded to a lint, so we must also get the call site check right to fix this for all cases.

The presence of a for<'a> fn(&'a ()), the desugaring of fn(&()), brings suspicion to uses of skip_binder() which would skip the binder for<'a> and turn 'a into an escaping region.

The WF check for the for<'a> fn(&'a ()): Debug trait predicate is reached by a call to check_where_clauses, then to ty::wf::predicate_obligations where we confidently skip the binder

rust/src/librustc/ty/wf.rs

Lines 75 to 78 in 2a3f536

	// (*) ok to skip binders, because wf code is prepared for it
	match *predicate {
	ty::Predicate::Trait(ref t) => {
	wf.compute_trait_ref(&t.skip_binder().trait_ref, Elaborate::None); // (*)

only to completely ignore any trait predicates containing an escaping region in compute_trait_ref, so the predicate is not checked at all for WF.

rust/src/librustc/ty/wf.rs

Lines 191 to 196 in 2a3f536

	self.out.extend(
	trait_ref.substs.types()
	.filter(|ty| !ty.has_escaping_regions())
	.map(|ty| traits::Obligation::new(cause.clone(),
	param_env,
	ty::Predicate::WellFormed(ty))));

I'm not sure how to fix but this certainly looks like guilty code.

[nikomatsakis]

OK, so, this first of all is not dependent on #48557. For example, this example will reproduce the problem on stable:

https://play.rust-lang.org/?gist=8c8b887b74da66782339e3a79c21c378&version=stable&mode=debug

The problem is rather a flaw in the object safety rules. Specifically, the object safety rules are supposed to prevent Self from appearing in the where clauses:

rust/src/librustc/traits/object_safety.rs

Lines 132 to 134 in dbd10f8

	if self.predicates_reference_self(trait_def_id, false) {
	violations.push(ObjectSafetyViolation::SupertraitSelf);
	}

But for some reason we seem to permit Self in the special case of Self: Trait:

rust/src/librustc/traits/object_safety.rs

Lines 167 to 170 in dbd10f8

	ty::Predicate::Trait(ref data) => {
	// In the case of a trait predicate, we can skip the "self" type.
	data.skip_binder().input_types().skip(1).any(|t| t.has_self_ty())
	}

I'm actually..not sure why we do that. It seems ok on the trait level, as that is validated at the impl site, but not at the method level.

[leoyvens]

Yes my previous comment was really misled, has nothing to do with HRTB or closures, see this minimization:

trait Trait {}

trait X {
    fn foo(&self) where Self: Trait;
}

impl X for () {
    fn foo(&self) {}
}

impl Trait for dyn X {}

pub fn main() {
    <X as X>::foo(&()); // Segfault at opt-level 0, SIGILL otherwise.
}