Skip to content

Unsound projection when late-bound-region appears only in return type #32330

Closed

Description

While working on a cache for normalization, I came across this problem in the way we currently handle fn items:

#![feature(unboxed_closures)]

fn foo<'a>() -> &'a u32 { loop { } }

fn bar<T>(t: T, x: T::Output) -> T::Output
    where T: FnOnce<()>
{
    x
}

fn main() {
    let x = &22;
    let z: &'static u32 = bar(foo, x); // <-- Effectively casts stack pointer to static pointer
}

That program compiles, but clearly it should not. What's going on is kind of in the grotty details of how we handle a function like:

fn foo<'a>() -> &'a u32 { loop { } }

Currently, that is typed as if it was roughly equivalent to:

struct foo;
impl<'a> FnOnce<()> for foo {
    type Output = &'a u32;
    ...
}
...

However, if you actually tried to write that impl above, you'd get an error:

<anon>:3:6: 3:8 error: the lifetime parameter `'a` is not constrained by the impl trait, self type, or predicates [E0207]
<anon>:3 impl<'a> FnOnce<()> for foo {
              ^~

And the reason is precisely because of this kind of unsoundness. Effectively, the way we handle the function foo above allows Output to have different lifetimes each time it is projected.

I believe this problem is specific to named lifetimes like 'a that only appear in the return type and do not appear in any where clauses. This is because:

  • If the lifetime appears in an argument, then it would be constrained by the FnOnce trait's argument parameter. (Assuming it appears in a constrained position; that is, not as an input to an associated type projection.)
  • If the lifetime appears in a where-clause, it would be classified as an "early bound" lifetime, in which case it would be part of the self type (iow, we would translate to something like struct foo<'a>).

The current treatment of early- vs late-bound is something that hasn't really scaled up well to the more complex language we have now. It is also central to #25860, for example. I think it is feasible to actually revamp how we handle things in the compiler in a largely backwards compatible way to close this hole -- and the refactoring would even take us closer to HKT. Basically we'd be pulling the Binder out of FnSig and object types and moving to be around types in general.

However, in the short term, I think we could just reclassify named lifetimes that do not appear in the argument types as being early-bound. I've got an experimental branch doing that right now.

This problem is the root of what caused my projection cache not to work: I was assuming in that code that T::Output would always be the same if T is the same, and that is not currently the case!

cc @rust-lang/lang
cc @arielb1
cc @RalfJung

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

Labels

I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions