Rollup merge of #129021 - compiler-errors:ptr-cast-outlives, r=lcnr

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

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
This commit is contained in:
Matthias Krüger 2024-09-06 07:33:56 +02:00 committed by GitHub
commit e903b29dc3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 145 additions and 12 deletions

View File

@ -1979,19 +1979,42 @@ fn check_rvalue(&mut self, body: &Body<'tcx>, rvalue: &Rvalue<'tcx>, location: L
match cast_kind { match cast_kind {
CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer) => { CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer) => {
let fn_sig = op.ty(body, tcx).fn_sig(tcx); let src_sig = op.ty(body, tcx).fn_sig(tcx);
// The type that we see in the fcx is like // HACK: This shouldn't be necessary... We can remove this when we actually
// `foo::<'a, 'b>`, where `foo` is the path to a // get binders with where clauses, then elaborate implied bounds into that
// function definition. When we extract the // binder, and implement a higher-ranked subtyping algorithm that actually
// signature, it comes from the `fn_sig` query, // respects these implied bounds.
// and hence may contain unnormalized results. //
let fn_sig = self.normalize(fn_sig, location); // This protects against the case where we are casting from a higher-ranked
// fn item to a non-higher-ranked fn pointer, where the cast throws away
let ty_fn_ptr_from = Ty::new_fn_ptr(tcx, fn_sig); // implied bounds that would've needed to be checked at the call site. This
// only works when we're casting to a non-higher-ranked fn ptr, since
// placeholders in the target signature could have untracked implied
// bounds, resulting in incorrect errors.
//
// We check that this signature is WF before subtyping the signature with
// the target fn sig.
if src_sig.has_bound_regions()
&& let ty::FnPtr(target_fn_tys, target_hdr) = *ty.kind()
&& let target_sig = target_fn_tys.with(target_hdr)
&& let Some(target_sig) = target_sig.no_bound_vars()
{
let src_sig = self.infcx.instantiate_binder_with_fresh_vars(
span,
BoundRegionConversionTime::HigherRankedType,
src_sig,
);
let src_ty = Ty::new_fn_ptr(self.tcx(), ty::Binder::dummy(src_sig));
self.prove_predicate(
ty::ClauseKind::WellFormed(src_ty.into()),
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
);
let src_ty = self.normalize(src_ty, location);
if let Err(terr) = self.sub_types( if let Err(terr) = self.sub_types(
ty_fn_ptr_from, src_ty,
*ty, *ty,
location.to_locations(), location.to_locations(),
ConstraintCategory::Cast { unsize_to: None }, ConstraintCategory::Cast { unsize_to: None },
@ -2000,7 +2023,41 @@ fn check_rvalue(&mut self, body: &Body<'tcx>, rvalue: &Rvalue<'tcx>, location: L
self, self,
rvalue, rvalue,
"equating {:?} with {:?} yields {:?}", "equating {:?} with {:?} yields {:?}",
ty_fn_ptr_from, target_sig,
src_sig,
terr
);
};
}
let src_ty = Ty::new_fn_ptr(tcx, src_sig);
// HACK: We want to assert that the signature of the source fn is
// well-formed, because we don't enforce that via the WF of FnDef
// types normally. This should be removed when we improve the tracking
// of implied bounds of fn signatures.
self.prove_predicate(
ty::ClauseKind::WellFormed(src_ty.into()),
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
);
// The type that we see in the fcx is like
// `foo::<'a, 'b>`, where `foo` is the path to a
// function definition. When we extract the
// signature, it comes from the `fn_sig` query,
// and hence may contain unnormalized results.
let src_ty = self.normalize(src_ty, location);
if let Err(terr) = self.sub_types(
src_ty,
*ty,
location.to_locations(),
ConstraintCategory::Cast { unsize_to: None },
) {
span_mirbug!(
self,
rvalue,
"equating {:?} with {:?} yields {:?}",
src_ty,
ty, ty,
terr terr
); );

View File

@ -0,0 +1,13 @@
//@ check-pass
//@ known-bug: #25860
static UNIT: &'static &'static () = &&();
fn foo<'a, 'b, T>(_: &'a &'b (), v: &'b T, _: &()) -> &'a T { v }
fn bad<'a, T>(x: &'a T) -> &'static T {
let f: fn(_, &'a T, &()) -> &'static T = foo;
f(UNIT, x, &())
}
fn main() {}

View File

@ -0,0 +1,13 @@
// Regression test for #129021.
static UNIT: &'static &'static () = &&();
fn foo<'a: 'a, 'b: 'b, T>(_: &'a &'b (), v: &'b T) -> &'a T { v }
fn bad<'a, T>(x: &'a T) -> &'static T {
let f: fn(_, &'a T) -> &'static T = foo;
//~^ ERROR lifetime may not live long enough
f(UNIT, x)
}
fn main() {}

View File

@ -0,0 +1,10 @@
error: lifetime may not live long enough
--> $DIR/implied-bounds-on-nested-references-plus-variance-early-bound.rs:8:12
|
LL | fn bad<'a, T>(x: &'a T) -> &'static T {
| -- lifetime `'a` defined here
LL | let f: fn(_, &'a T) -> &'static T = foo;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static`
error: aborting due to 1 previous error

View File

@ -0,0 +1,19 @@
// Regression test for #129021.
trait ToArg<T> {
type Arg;
}
impl<T, U> ToArg<T> for U {
type Arg = T;
}
fn extend_inner<'a, 'b>(x: &'a str) -> <&'b &'a () as ToArg<&'b str>>::Arg { x }
fn extend<'a, 'b>(x: &'a str) -> &'b str {
(extend_inner as fn(_) -> _)(x)
//~^ ERROR lifetime may not live long enough
}
fn main() {
let y = extend(&String::from("Hello World"));
println!("{}", y);
}

View File

@ -0,0 +1,14 @@
error: lifetime may not live long enough
--> $DIR/implied-bounds-on-nested-references-plus-variance-unnormalized.rs:12:5
|
LL | fn extend<'a, 'b>(x: &'a str) -> &'b str {
| -- -- lifetime `'b` defined here
| |
| lifetime `'a` defined here
LL | (extend_inner as fn(_) -> _)(x)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a`
|
= help: consider adding the following bound: `'a: 'b`
error: aborting due to 1 previous error

View File

@ -1,8 +1,4 @@
//@ check-pass // Regression test for #129021.
//@ known-bug: #25860
// Should fail. The combination of variance and implied bounds for nested
// references allows us to infer a longer lifetime than we can prove.
static UNIT: &'static &'static () = &&(); static UNIT: &'static &'static () = &&();
@ -10,6 +6,7 @@ fn foo<'a, 'b, T>(_: &'a &'b (), v: &'b T) -> &'a T { v }
fn bad<'a, T>(x: &'a T) -> &'static T { fn bad<'a, T>(x: &'a T) -> &'static T {
let f: fn(_, &'a T) -> &'static T = foo; let f: fn(_, &'a T) -> &'static T = foo;
//~^ ERROR lifetime may not live long enough
f(UNIT, x) f(UNIT, x)
} }

View File

@ -0,0 +1,10 @@
error: lifetime may not live long enough
--> $DIR/implied-bounds-on-nested-references-plus-variance.rs:8:12
|
LL | fn bad<'a, T>(x: &'a T) -> &'static T {
| -- lifetime `'a` defined here
LL | let f: fn(_, &'a T) -> &'static T = foo;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ type annotation requires that `'a` must outlive `'static`
error: aborting due to 1 previous error