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 permits use after move in if statements #37891

Closed
Xaeroxe opened this issue Nov 20, 2016 · 6 comments
Closed

Borrow checker permits use after move in if statements #37891

Xaeroxe opened this issue Nov 20, 2016 · 6 comments
Labels
A-borrow-checker Area: The borrow checker T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Xaeroxe
Copy link
Contributor

Xaeroxe commented Nov 20, 2016

Below code when compiled with Rust 1.13 (current stable) and Rust 1.15 (current nightly) will result in segmentation faults and memory corruption. The unwrap call in the else if should consume the value and prevent further use but it does not.

fn main() {
    let vec = get_vec();
    if let Err(err) = vec {
        println!("Got an error: {:?}", err);
    }
    else if vec.unwrap().len() == 0 {
        println!("Vec was 0 length.");
    }
    else {
        for i in vec.unwrap() {
            println!("{}", i);
        }
    }
}

fn get_vec() -> Result<Vec<i32>, i32> {
    Ok::<Vec<i32>, i32>(vec![1,2,3])
}

I expected: the borrow checker to mark the for loop as a use after move due to calling unwrap() after the value had been consumed by checking it in the else if statement

What happened: The borrow checker did not prevent this from compiling which resulted in segfaults and memory corruption.

Reproduction tip: You will get slightly different behavior of this code on each execution as memory errors are a bit finicky like that, if it works the first time try it a few more times, I'm sure you'll see it crash before you get to 5 attempts.

Meta

rustc --version --verbose:
rustc 1.15.0-nightly (ac635aa 2016-11-18)
binary: rustc
commit-hash: ac635aa
commit-date: 2016-11-18
host: x86_64-pc-windows-gnu
release: 1.15.0-nightly
LLVM version: 3.9

@eddyb eddyb added A-borrow-checker Area: The borrow checker I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 20, 2016
@TimNN
Copy link
Contributor

TimNN commented Nov 20, 2016

Related to, maybe a duplicate of, #31287.

@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented Nov 21, 2016

I agree, the issues are very similar.

@nikomatsakis
Copy link
Contributor

when desugared into HIR, this pretty-prints as:

use std::prelude::v1::*;
#[macro_use]
extern crate std as std;
fn main() {
              let vec = get_vec();
              match vec {
                  Err(err) => {

                      $crate::io::_print(::std::fmt::Arguments::new_v1({
                                                                           static __STATIC_FMTSTR:
                                                                                  &'static [&'static str]
                                                                                  =
                                                                               &["Got an error: ",
                                                                                 "\n"];
                                                                           __STATIC_FMTSTR
                                                                       },
                                                                       &match (&err,)
                                                                            {
                                                                            (__arg0,)
                                                                            =>
                                                                            [::std::fmt::ArgumentV1::new(__arg0,
                                                                                                         ::std::fmt::Debug::fmt)],
                                                                        }));
                  }
                  _ if vec.unwrap().len() == 0 => {
                      $crate::io::_print(::std::fmt::Arguments::new_v1({
                                                                           static __STATIC_FMTSTR:
                                                                                  &'static [&'static str]
                                                                                  =
                                                                               &["Vec was 0 length.\n"];
                                                                           __STATIC_FMTSTR
                                                                       },
                                                                       &match ()
                                                                            {
                                                                            ()
                                                                            =>
                                                                            [],
                                                                        }));
                  }

so I agree it's a dup of #31287. But definitely bad. We should move on fixing these. The plan of record has been MIR borrowck, however.

@nikomatsakis
Copy link
Contributor

Closing as a duplicate of #31287.

@nikomatsakis
Copy link
Contributor

Hmm, there is an argument for keeping this open, and addressing it by tweaking the desugaring, just because that can be done quickly, even if it's just a workaround.

@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented Dec 3, 2016

If you can hit two birds with one stone by fixing 31287 though why bother addressing this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrow-checker Area: The borrow checker 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

4 participants