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

user annotations on associated consts can be ill-formed #104763

Closed
aliemjay opened this issue Nov 23, 2022 · 0 comments · Fixed by #120019
Closed

user annotations on associated consts can be ill-formed #104763

aliemjay opened this issue Nov 23, 2022 · 0 comments · Fixed by #120019
Assignees
Labels
A-borrow-checker Area: The borrow checker A-NLL Area: Non Lexical Lifetimes (NLL) C-bug Category: This is a bug. NLL-sound Working towards the "invalid code does not compile" goal T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@aliemjay
Copy link
Member

aliemjay commented Nov 23, 2022

The following should fail: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=55b6d108fe6712536a9c7a7abea56bc0

trait Trait {
    const TRAIT: bool;
}

impl<T> Trait for &'static T {
    const TRAIT: bool = true;
}

fn test<T>() {
    <&'static T>::TRAIT;
}

It should require a T: 'static bound on test.

This is unsound but can't currently be exploited because of #98852. A potential exploit would be like:

trait Trait<'a> {
    const TRAIT: fn(&'a str) -> &'static str;
}

impl<'a> Trait<'a> for &'static &'a () { // implies 'a: 'static
    const TRAIT: fn(&'a str) -> &'static str = {
        |x| x // this should pass if we use implied bounds from impl header
    };
}

fn test<T>() {
    let val = <&'static &'_ ()>::TRAIT(&String::new()); // we should error here!
    println!("{}", val);
}

@rustbot label T-types C-bug A-NLL NLL-sound A-borrow-checker
@rustbot claim

@rustbot rustbot added A-borrow-checker Area: The borrow checker A-NLL Area: Non Lexical Lifetimes (NLL) C-bug Category: This is a bug. NLL-sound Working towards the "invalid code does not compile" goal T-types Relevant to the types team, which will review and decide on the PR/issue. labels Nov 23, 2022
bors added a commit to rust-lang-ci/rust that referenced this issue Nov 20, 2023
fix fn/const items implied bounds and wf check

These are two distinct changes (edit: actually three, see below):
1. Wf-check all fn item args. This is a soundness fix.
Fixes rust-lang#104005

2. Use implied bounds from impl header in borrowck of associated functions/consts. This strictly accepts more code and helps to mitigate the impact of other breaking changes.
Fixes rust-lang#98852
Fixes rust-lang#102611

The first is a breaking change and will likely have a big impact without the the second one. See the first commit for how it breaks libstd.

Landing the second one without the first will allow more incorrect code to pass. For example an exploit of rust-lang#104005 would be as simple as:
```rust
use core::fmt::Display;

trait ExtendLt<Witness> {
    fn extend(self) -> Box<dyn Display>;
}

impl<T: Display> ExtendLt<&'static T> for T {
    fn extend(self) -> Box<dyn Display> {
        Box::new(self)
    }
}

fn main() {
    let val = (&String::new()).extend();
    println!("{val}");
}
```

The third change is to to check WF of user type annotations before normalizing them (fixes rust-lang#104764, fixes rust-lang#104763). It is mutually dependent on the second change above: an attempt to land it separately in rust-lang#104746 caused several crater regressions that can all be mitigated by using the implied from the impl header. It is also necessary for the soundness of associated consts that use the implied bounds of impl header. See rust-lang#104763 and how the third commit fixes the soundness issue in `tests/ui/wf/wf-associated-const.rs` that was introduces by the previous commit.

cc `@lcnr`
r? types
@bors bors closed this as completed in 6bf600b Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker A-NLL Area: Non Lexical Lifetimes (NLL) C-bug Category: This is a bug. NLL-sound Working towards the "invalid code does not compile" goal T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
2 participants