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

Borrow checker incorrectly accepts code when calling function with complex lifetime bounds through a fn pointer #57170

Closed
ennis opened this issue Dec 28, 2018 · 5 comments
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness NLL-sound Working towards the "invalid code does not compile" goal P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ennis
Copy link

ennis commented Dec 28, 2018

The following code compiles and prints a bogus value:

fn shorten_lifetime<'a, 'b, 'min>(a: &'a i32, b: &'b i32) -> &'min i32
where 'a: 'min, 'b: 'min
{
    if *a < *b {
        &a
    } else {
        &b
    }
}


fn main()
{
    let fnptr = &shorten_lifetime;
    
    let a = &5;
    let ptr = {
        let b = 3;
        let b = &b;
        //let c = shorten_lifetime(a, b);
        let c = fnptr(a, b);
        c
    };
    
    println!("ptr = {:?}", ptr);
    
}

Output (release mode):

ptr = -1876488848

(playground: https://play.rust-lang.org/?version=stable&mode=release&edition=2018&gist=048d856b4689599b1e35f0a44ebe629f)

Expected result: a borrow checker error, the same as the one obtained when calling shorten_lifetime directly.

I accidentally ran into this issue when trying to spell the fn pointer type of fnptr (I'm not sure how to add constraints to the lifetimes of a higher-ranked fn type like for<'a, 'b> fn(...) -> T)

@sfackler sfackler added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness labels Dec 28, 2018
@jonas-schievink
Copy link
Contributor

The 2015 edition correctly rejects this (which doesn't yet have NLL)

@nagisa nagisa added the A-NLL Area: Non-lexical lifetimes (NLL) label Dec 28, 2018
@nagisa
Copy link
Member

nagisa commented Dec 28, 2018

cc @pnkfelix

@nagisa
Copy link
Member

nagisa commented Dec 28, 2018

@rust-lang/wg-compiler-nll

@nagisa nagisa added the regression-from-stable-to-stable Performance or correctness regression from one stable version to another. label Dec 28, 2018
@matthewjasper matthewjasper added the NLL-sound Working towards the "invalid code does not compile" goal label Dec 28, 2018
@matthewjasper
Copy link
Contributor

matthewjasper commented Dec 28, 2018

It appears that we lose the function bounds when we promote shorten_lifetime. Relavant FIXME:

// FIXME -- promoted MIR return types reference
// various "free regions" (e.g., scopes and things)
// that they ought not to do. We have to figure out
// how best to handle that -- probably we want treat
// promoted MIR much like closures, renumbering all
// their free regions and propagating constraints
// upwards. We have the same acyclic guarantees, so
// that should be possible. But for now, ignore them.
//
// let promoted_mir = &self.mir.promoted[index];
// promoted_mir.return_ty()

@pnkfelix
Copy link
Member

triage: P-high. Hopefully fix in PR #57202 lands very soon.

@pnkfelix pnkfelix added the P-high High priority label Feb 27, 2019
bors added a commit that referenced this issue Mar 2, 2019
Include bounds from promoted constants in NLL

Previously a promoted function wouldn't have its bound propagated out to
the main function body.

When we visit a promoted, we now type check the MIR of the promoted
and transfer any lifetime constraints to back to the main function's MIR.

Fixes #57170

r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness NLL-sound Working towards the "invalid code does not compile" goal P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants