Check WF of source type's signature on fn pointer cast

[compiler-errors]

trafficstars

This PR patches the implied bounds holes slightly for #129005, #25860.

Like most implied bounds related unsoundness fixes, this isn't complete w.r.t. higher-ranked function signatures, but I believe it implements a pretty good heuristic for now.

What does this do?

This PR makes a partial patch for a soundness hole in a FnDef -> FnPtr "reifying" pointer cast where we were never checking that the signature we are casting from is actually well-formed. Because of this, and because FnDef doesn't require its signature to be well-formed (just its predicates must hold), we are essentially allowed to "cast away" implied bounds that are assumed within the body of the FnDef:

fn foo<'a, 'b, T>(_: &'a &'b (), v: &'b T) -> &'a T { v }

fn bad<'short, T>(x: &'short T) -> &'static T {
    let f: fn(_, &'short T) -> &'static T = foo;
    f(&&(), x)
}

In this example, subtyping ends up casting the _ type (which should be &'static &'short ()) to some other type that no longer serves as a "witness" to the lifetime relationship 'short: 'static which would otherwise be required for this call to be WF. This happens regardless of if foo's lifetimes are early- or late-bound.

This PR implements two checks:

1. We check that the signature of the FnDef is well-formed before casting it. This ensures that there is at least one point in the MIR where we ensure that the FnDef's implied bounds are actually satisfied by the caller.
2. Implements a special case where if we're casting from a higher-ranked FnDef to a non-higher-ranked, we instantiate the binder of the FnDef with infer vars and ensure that it is a supertype of the target of the cast.

The (2.) is necessary to validate that these pointer casts are valid for higher-ranked FnDef. Otherwise, the example above would still pass even if help's 'a lifetime were late-bound.

Further work

The WF checks for function calls are scattered all over the MIR. We check the WF of args in call terminators, we check the WF of FnDef when we create a const operand referencing it, and we check the WF of the return type in #115538, to name a few.

One way to make this a bit cleaner is to simply extend #115538 to always check that the signature is WF for FnDef types. I may do this as a follow-up, but I wanted to keep this simple since this leads to some pretty bad NLL diagnostics regressions, and AFAICT this solution is complete enough.

Crater triage

Done here: https://github.com/rust-lang/rust/pull/129021#issuecomment-2297702647

r? lcnr

[compiler-errors]

@bors try

[bors]

:hourglass: Trying commit aa3574cd04b64eee30e1a79b556b055471e76f38 with merge f781a78d815e6a304bf6061b8a8fa69d7ce8dfd5...

[bors]

:sunny: Try build successful - checks-actions Build commit: f781a78d815e6a304bf6061b8a8fa69d7ce8dfd5 (f781a78d815e6a304bf6061b8a8fa69d7ce8dfd5)

[compiler-errors]

@craterbot check

[craterbot]

:ok_hand: Experiment pr-129021 created and queued. :robot: Automatically detected try build f781a78d815e6a304bf6061b8a8fa69d7ce8dfd5 :mag: You can check out the queue and this experiment's details.

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[rustbot]

changes to the core type system

cc @compiler-errors, @lcnr

[bors]

:umbrella: The latest upstream changes (presumably #129092) made this pull request unmergeable. Please resolve the merge conflicts.

[craterbot]

:construction: Experiment pr-129021 is now running

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

:tada: Experiment pr-129021 is completed! :bar_chart: 24 regressed and 4 fixed (499461 total) :newspaper: Open the full report.

:warning: If you notice any spurious failure please add them to the blacklist! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[compiler-errors]

@craterbot check crates=https://crater-reports.s3.amazonaws.com/pr-129021/retry-regressed-list.txt p=1

There seems to be no legitimate regressions, just a bunch of unsound copies of cve-rs. Lots of "no more space on disk" and segfaults that seem normal with crater, but let's give this another pass just to shake those off.

[craterbot]

:ok_hand: Experiment pr-129021-1 created and queued. :robot: Automatically detected try build f781a78d815e6a304bf6061b8a8fa69d7ce8dfd5 :warning: Try build based on commit aa3574cd04b64eee30e1a79b556b055471e76f38, but latest commit is 86a31238c969277c057991addfbfc46a91cb6047. Did you forget to make a new try build? :mag: You can check out the queue and this experiment's details.

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

#16 2.662 Building wheels for collected packages: reuse
#16 2.663   Building wheel for reuse (pyproject.toml): started
#16 2.912   Building wheel for reuse (pyproject.toml): finished with status 'done'
#16 2.913   Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132715 sha256=dfa09868353292d98f811d3efdb0d54d07389e808efc71d68e3b93c514bf8bec
#16 2.913   Stored in directory: /tmp/pip-ephem-wheel-cache-bf8_dte6/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#16 2.915 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#16 3.318 Successfully installed attrs-23.2.0 binaryornot-0.4.4 boolean-py-4.0 chardet-5.2.0 jinja2-3.1.4 license-expression-30.3.0 markupsafe-2.1.5 python-debian-0.1.49 reuse-4.0.3 tomlkit-0.13.0
#16 3.318 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#16 DONE 3.4s
---
    Checking rustc_hir_typeck v0.0.0 (/checkout/compiler/rustc_hir_typeck)
error[E0023]: this pattern has 1 field, but the corresponding tuple variant has 2 fields
    --> compiler/rustc_borrowck/src/type_check/mod.rs:1998:46
     |
1998 | ...                   && let ty::FnPtr(target_sig) = *ty.kind()
     |
    ::: /checkout/compiler/rustc_type_ir/src/ty_kind.rs:151:11
     |
     |
151  |     FnPtr(ty::Binder<I, FnSigTys<I>>, FnHeader<I>),
     |
help: use `_` to explicitly ignore each field
     |
     |
1998 |                             && let ty::FnPtr(target_sig, _) = *ty.kind()

    Checking rustc_passes v0.0.0 (/checkout/compiler/rustc_passes)
For more information about this error, try `rustc --explain E0023`.
error: could not compile `rustc_borrowck` (lib) due to 1 previous error

[craterbot]

:construction: Experiment pr-129021-1 is now running

:information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

:tada: Experiment pr-129021-1 is completed! :bar_chart: 12 regressed and 0 fixed (24 total) :newspaper: Open the full report.

:warning: If you notice any spurious failure please add them to the blacklist! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[compiler-errors]

Crater triage:

Legitimate regressions which are just forms of cve-rs:

• Bobxcat.unsafe_safe.77114513c760cf8499c8f4a0f44082f0e95924a1
• Esu0.rust-workspace.06a86d9f13a738a01a2ffd2e665f5d9aae3d3459
• Speykious.cve-rs.a4d3538111c4e7f9eeb66a1f669a204fc2433a00
• alixmacdonald10.poll-of-the-century.bbf741d4986952f91a9104849ed15bca1d7b2142
• borrowfix-0.1.4
• cve-rs-0.6.0
• expand_any_lifetime-0.1.2
• fake-static-0.1.0
• odd_is_odd-0.2.1

Unsoundly implements a type_id impl:

• Senko-heart.keisho.3676a314dc42c782c58b1452b7c9cb8d80617389

fn type_id<T>() -> TypeId {
    fn tid<'a, T: 'a>(_: PhantomData<&'static T>) -> TypeId {
        TypeId::of::<T>()
    }

    let f = tid::<T> as *const ();
    let f: fn(PhantomData<&T>) -> TypeId = unsafe { mem::transmute(f) };
    f(PhantomData)
}

Unrelated regressions from #129059

This PR relies on us being able to record the correct target of coercion in MIR. This was fixed by #129059, but that led to two unrelated bugs. Both have fixes now, so I'm not worried about them now.

• Struggleton.TitleScreenBGM.23bd2077802ae42336a788678a462d067fc9aa20 -> fixed by #129288
• kwap-0.10.0 - https://github.com/rust-lang/rust/pull/129317

[lcnr]

see the PR description and crater triage https://github.com/rust-lang/rust/pull/129021#issuecomment-2297702647. This fixes (the non-higher ranked) cases of our most famous unsoundness without causing any unintended regressions😊

@rfcbot merge

[rfcbot]

Team member @lcnr has proposed to merge this. The next step is review by the rest of the tagged team members:

• [ ] @BoxyUwU
• [x] @compiler-errors
• [x] @jackh726
• [x] @lcnr
• [ ] @nikomatsakis
• [x] @oli-obk
• [x] @spastorino

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

:bell: This is now entering its final comment period, as per the review above. :bell:

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[compiler-errors]

@bors r=lcnr

[bors]

:pushpin: Commit 67804c57e7a53b96d79b49bf6e356e3ade4d943d has been approved by lcnr

It is now in the queue for this repository.