Arguments of an impl method may have stronger implied bounds than trait method

[adamritter]

I tried this code:

use std::{cell::RefCell};

pub struct MessageListeners<'a> {
    listeners: RefCell<Vec<Box<dyn FnMut(()) + 'a>>>
}

impl<'a> MessageListeners<'a> {
    pub fn new()->Self {
        MessageListeners { listeners: RefCell::new(Vec::new()) }
    }
    pub fn listen(&self, f: impl FnMut(())+'a) {
        self.listeners.borrow_mut().push(Box::new(f));
    }
    pub fn send(&self, message: ()) {
        for listener in self.listeners.borrow_mut().iter_mut() {
            (*listener)(message.clone());
        }
    }
    pub fn map(&self, f: impl Fn(())->())->MessageListeners<'a> {
        let r = MessageListeners::new();
        self.listeners().listen(|m|  r.send(f(m)));
        r
    }
}

pub trait MessageListenersInterface {
    fn listeners(&self)->&MessageListeners;
}

impl<'a> MessageListenersInterface for MessageListeners<'a> {
    fn listeners<'b>(&'b self)->&'a MessageListeners<'b> {
        self
    }
}


#[test]
fn test_message_listeners_map0() {
    let ml = MessageListeners::new();
    let f = |m: ()| {};
    let g = |m: ()| m;
    ml.map(g).listen(f);
    ml.send(());
}

#[test]
fn test_message_listeners_map2() {
    let ml = MessageListeners::new();
    let f = |m: ()| {};
    let g = |m: ()| m;
    let temp_variable=ml.map(g);
    temp_variable.listen(f);
    ml.send(());
}

I expected to see this happen: the two tests do the same thing

Instead, this happened: the first unit test SEG faults, the second succeeds

Meta

rustc --version --verbose:

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: aarch64-apple-darwin
release: 1.65.0
LLVM version: 15.0.0```

<!--
Include a backtrace in the code block by setting `RUST_BACKTRACE=1` in your
environment. E.g. `RUST_BACKTRACE=1 cargo build`.
-->
<details><summary>Backtrace</summary>
<p>

Caused by:
process didn't exit successfully: /Users/adamritter/Documents/GitHub/synced_collection/target/debug/deps/synccollection-9d89a83a383169be (signal: 11, SIGSEGV: invalid memory reference)```

[frxstrem]

MIRI claims undefined behavior here:

error: Undefined Behavior: constructing invalid value: encountered a dangling reference (use-after-free)
  --> src/lib.rs:21:38
   |
21 |         self.listeners().listen(|m|  r.send(f(m)));
   |                                      ^^^^^^^^^^^^ constructing invalid value: encountered a dangling reference (use-after-free)
   |
   = 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
   = note: BACKTRACE:
   = note: inside closure at src/lib.rs:21:38
   = note: inside `<std::boxed::Box<dyn std::ops::FnMut(())> as std::ops::FnMut<((),)>>::call_mut` at /Users/cognite/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/boxed.rs:2016:9
note: inside `MessageListeners::<'_>::send` at src/lib.rs:16:13
  --> src/lib.rs:16:13
   |
16 |             (*listener)(message.clone());
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `test_message_listeners_map0` at src/lib.rs:43:5
  --> src/lib.rs:43:5
   |
43 |     ml.send(());
   |     ^^^^^^^^^^^
note: inside closure at src/lib.rs:38:34
  --> src/lib.rs:38:34
   |
37 | #[test]
   | ------- in this procedural macro expansion
38 | fn test_message_listeners_map0() {
   |                                  ^
   = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

Given that this code only consists of safe code, that smells like a compiler bug.

[jruderman]

First crashes in nightly-2020-10-07.

Bisection details

I used:

cargo-bisect-rustc  --preserve --script ./unsound.sh --start 2020-01-01 --end 2020-12-05

With unsound.sh containing:

! cargo test 2>&1 | grep "SEGV"

Earlier versions report a lifetime error:

Lifetime error reported with nightly-2020-10-06

error[E0308]: method not compatible with trait
  --> src/main.rs:35:5
   |
35 |     fn listeners<'b>(&'b self)->&'a MessageListeners<'b> {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch
   |
   = note: expected fn pointer `fn(&MessageListeners<'a>) -> &MessageListeners<'_>`
              found fn pointer `fn(&MessageListeners<'a>) -> &'a MessageListeners<'_>`
note: the lifetime `'a` as defined on the impl at 34:6...
  --> src/main.rs:34:6
   |
34 | impl<'a> MessageListenersInterface for MessageListeners<'a> {
   |      ^^
note: ...does not necessarily outlive the anonymous lifetime #1 defined on the method body at 35:5
  --> src/main.rs:35:5
   |
35 |     fn listeners<'b>(&'b self)->&'a MessageListeners<'b> {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Was that lifetime error correct?

[apiraino]

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-critical

[RustyYato]

Was that lifetime error correct?

Yes, the lifetime error is correct fn(&self) -> &'a _ is incompatible with the definition in the trait: fn(&self) -> &_.

[compiler-errors]

Looks like it may be a bug in compare_predicate_entailment (trait<=>impl method compatibility checking). I'll take a look at this today, and if I can't find a root cause + fix in a few hours, I'll give it back.

@rustbot claim

[compiler-errors]

found 6 bors merge commits in the specified range
  commit[0] 2020-10-05UTC: Auto merge of #77080 - richkadel:llvm-coverage-counters-2, r=tmandry
  commit[1] 2020-10-06UTC: Auto merge of #77606 - JohnTitor:rollup-7rgahdt, r=JohnTitor
  commit[2] 2020-10-06UTC: Auto merge of #77594 - timvermeulen:chain_advance_by, r=scottmcm
  commit[3] 2020-10-06UTC: Auto merge of #73905 - matthewjasper:projection-bounds-2, r=nikomatsakis
  commit[4] 2020-10-06UTC: Auto merge of #76356 - caass:hooks, r=jyn514
  commit[5] 2020-10-06UTC: Auto merge of #77386 - joshtriplett:static-glibc, r=petrochenkov

Almost certainly due to #73905.

[compiler-errors]

The issue is minimized to this basically:

pub struct Listeners<'a> {
    listeners: RefCell<Vec<Box<dyn FnMut(()) + 'a>>>
}

pub trait ListenersInterface {
    fn listeners<'b>(&'b self) -> &'b Listeners<'b>;
}

impl<'a> ListenersInterface for Listeners<'a> {
    fn listeners<'b>(&'b self) -> &'a Listeners<'b> {
        self
    }
}

The fact that the impl compiles is incorrect, I think.

Basically, for this to be sound, given the assumptions of the impl + the assumptions from the trait's method's where clauses (and assuming the trait's method's types are well-formed), we should be able to prove that the impl's method's types are well-formed.

This boils down to: given WellFormed(&'b Listeners<'a>) + WellFormed(&'b Listeners<'b>) (i.e. given 'a: 'b), proving that WellFormed(&'a Listeners<'b>) (i.e. 'b: 'a), which is not possible.

[aliemjay]

edit: Oh it's already a runtime error. I thought it was an ICE 👀.

An exploit of this bug for an actual ub:

trait Trait {
    fn get<'s>(s: &'s str, _: &'static &'static ()) -> &'static str;
}
impl Trait for () {
    fn get<'s>(s: &'s str, _: &'static &'s ()) -> &'static str {
        s
    }
}
fn main() {
    let val = <() as Trait>::get(&String::from("blah blah blah"), &&());
    println!("{}", val);
}

[albertlarsan68]

An exploit of this bug for an actual ub:

trait Trait {
    fn get<'s>(s: &'s str, _: &'static &'static str) -> &'static str;
}
impl Trait for () {
    fn get<'s>(s: &'s str, _: &'static &'s str) -> &'static str {
        s
    }
}
fn main() {
    let val = <() as Trait>::get(&String::from("blah blah blah"), &"");
    println!("{}", val);
}

This reminds me of #25860

[apiraino]

Visiting during T-compiler meeting, reprioritizing.

@rustbot label -P-critical +P-high

[lcnr]

closing as a duplicate of #80176, currently have a deny by default future-compat lint here. We'll convert that lint to a hard error soon-ish