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

implied bounds from projections in impl header can be unsound #100051

Open
aliemjay opened this issue Aug 2, 2022 · 6 comments
Open

implied bounds from projections in impl header can be unsound #100051

aliemjay opened this issue Aug 2, 2022 · 6 comments
Labels
A-associated-items Area: Associated items such as associated types and consts. A-implied-bounds Area: Related to implied bounds C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. T-types Relevant to the types team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative

Comments

@aliemjay
Copy link
Member

aliemjay commented Aug 2, 2022

A variant of #98543 that has a different failure path and thus is not fixed by #99217. cc @lcnr

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=da2da1d93c1e2c99c640000a78e9a80f

trait Trait {
    type Type;
}

impl<T> Trait for T {
    type Type = ();
}

trait Extend<'a, 'b> {
    fn extend(self, s: &'a str) -> &'b str;
}

impl<'a, 'b> Extend<'a, 'b> for <&'b &'a () as Trait>::Type
where
    for<'what, 'ever> &'what &'ever (): Trait,
{
    fn extend(self, s: &'a str) -> &'b str {
        s
    }
}

fn main() {
    let y = <() as Extend<'_, '_>>::extend((), &String::from("Hello World"));
    println!("{}", y);
}

@rustbot label C-bug I-unsound T-types A-implied-bounds A-associated-items

@rustbot rustbot added A-associated-items Area: Associated items such as associated types and consts. A-implied-bounds Area: Related to implied bounds C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-types Relevant to the types team, which will review and decide on the PR/issue. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 2, 2022
@aliemjay aliemjay changed the title implied bounds fro projections in impl header can be unsound implied bounds for projections in impl header can be unsound Aug 2, 2022
@aliemjay aliemjay changed the title implied bounds for projections in impl header can be unsound implied bounds from projections in impl header can be unsound Aug 2, 2022
@lcnr
Copy link
Contributor

lcnr commented Aug 2, 2022

ah, that's fun 👍

@apiraino
Copy link
Contributor

apiraino commented Aug 3, 2022

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Aug 3, 2022
@lcnr
Copy link
Contributor

lcnr commented Oct 21, 2022

@rustbot claim

@lcnr lcnr removed their assignment Jan 13, 2023
@lcnr
Copy link
Contributor

lcnr commented Jan 13, 2023

To fix this we pretty much have to add explicit WellFormed predicates to the predicates of impls which results in solver cycles until we've made all trait predicates coinductive.

See this document for more info https://hackmd.io/_F7hq8ldSr-PuqudZ1A-nA#new-trait-solver-cycles

This is blocked on stabilizing the new trait solver.

@lcnr lcnr added the WG-trait-system-refactor The Rustc Trait System Refactor Initiative label Jan 13, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 23, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? `@jackh726`

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 24, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? ``@jackh726``

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 24, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? `@jackh726`

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 24, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? ``@jackh726``

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 24, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? `@jackh726`

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
JohnTitor added a commit to JohnTitor/rust that referenced this issue Apr 24, 2023
…unsound-issues, r=jackh726

Add `known-bug` tests for 11 unsound issues

r? ``@jackh726``

Should tests for other issues be in separate PRs?  Thanks.

Edit: Partially addresses rust-lang#105107.  This PR adds `known-bug` tests for 11 unsound issues:
- rust-lang#25860
- rust-lang#49206
- rust-lang#57893
- rust-lang#84366
- rust-lang#84533
- rust-lang#84591
- rust-lang#85099
- rust-lang#98117
- rust-lang#100041
- rust-lang#100051
- rust-lang#104005
@jackh726 jackh726 added the S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. label Apr 25, 2023
@Neutron3529
Copy link
Contributor

Seems #25860, an ancient issue.

@lcnr
Copy link
Contributor

lcnr commented Jun 20, 2024

While these issues have a similar root cause, the way it is exploited differs. The root cause is that we do not explicitly track implied bounds of impls and functions, instead assuming that they'll have to be proven when checking that the used types are well-formed at the call-site.

This assumption is invalidated in #25860 by subtyping the function pointers before calling it. In this issue we only check that the normalized impl header is well-formed and "normalize away" the type requiring the implied bound when using the impl, but cannot do so due to the where-bound while computing the implied bounds used by the impl.

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 such as associated types and consts. A-implied-bounds Area: Related to implied bounds C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority S-bug-has-test Status: This bug is tracked inside the repo by a `known-bug` test. T-types Relevant to the types team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative
Projects
Status: new solver everywhere
Development

No branches or pull requests

6 participants