diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2024-09-06 07:33:56 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-09-06 07:33:56 +0200 |
| commit | e903b29dc3ba90dff571ad8ce6be6967075752c0 (patch) | |
| tree | d7fffb81f33357b68da698b7e0d6e3c40096efb9 /compiler | |
| parent | d678b81485b05680801554d744ea2696949f0135 (diff) | |
| parent | 67804c57e7a53b96d79b49bf6e356e3ade4d943d (diff) | |
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
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_borrowck/src/type_check/mod.rs | 71 |
1 files changed, 64 insertions, 7 deletions
diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index 224f8d5c893..3c9a43883ad 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -1979,19 +1979,76 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { match cast_kind { CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer) => { - let fn_sig = op.ty(body, tcx).fn_sig(tcx); + let src_sig = op.ty(body, tcx).fn_sig(tcx); + + // HACK: This shouldn't be necessary... We can remove this when we actually + // get binders with where clauses, then elaborate implied bounds into that + // binder, and implement a higher-ranked subtyping algorithm that actually + // respects these implied bounds. + // + // 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 + // 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( + src_ty, + *ty, + location.to_locations(), + ConstraintCategory::Cast { unsize_to: None }, + ) { + span_mirbug!( + self, + rvalue, + "equating {:?} with {:?} yields {:?}", + 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 fn_sig = self.normalize(fn_sig, location); - - let ty_fn_ptr_from = Ty::new_fn_ptr(tcx, fn_sig); - + let src_ty = self.normalize(src_ty, location); if let Err(terr) = self.sub_types( - ty_fn_ptr_from, + src_ty, *ty, location.to_locations(), ConstraintCategory::Cast { unsize_to: None }, @@ -2000,7 +2057,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { self, rvalue, "equating {:?} with {:?} yields {:?}", - ty_fn_ptr_from, + src_ty, ty, terr ); |
