Skip to content

Borrow Checker allowing mutable and immutable reference out on a vec [Nightly] #41774

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
mooman219 opened this issue May 5, 2017 · 5 comments
Labels
I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mooman219
Copy link

mooman219 commented May 5, 2017

When using array indexing, the borrow checking is allowing for both an immutable and mutable reference to be taken out on a vec.

Playground: https://play.rust-lang.org/?gist=558d7e98834c5e269eed35b335823220&version=nightly&backtrace=0

Version: rustc 1.19.0-nightly (59f1a2f 2017-05-04)

Offending Code:

#[derive(Debug)]
struct Data {
    x: i32,
    y: i32,
}

impl Data {
    fn add(&mut self, other: &Data) -> &mut Data {
        self.x += other.x;
        self.y += other.y;
        return self;
    }
}

fn main() {
    let mut v = vec![
        Data{x: 0, y: 1},
        Data{x: 2, y: 3},
        Data{x: 4, y: 5},
    ];
    v[0].add(v[1].add(&v[2]));

    println!("{:?}", v);
}

Expected Result:

error[E0499]: cannot borrow `v` as mutable more than once at a time
  --> <anon>:21:14
   |
21 |     v[0].add(v[1].add(&v[2]));
   |     -        ^              - first borrow ends here
   |     |        |
   |     |        second mutable borrow occurs here
   |     first mutable borrow occurs here

error[E0502]: cannot borrow `v` as immutable because it is also borrowed as mutable
  --> <anon>:21:24
   |
21 |     v[0].add(v[1].add(&v[2]));
   |     -                  ^    - mutable borrow ends here
   |     |                  |
   |     |                  immutable borrow occurs here
   |     mutable borrow occurs here

Actual Result:
[Data { x: 6, y: 9 }, Data { x: 6, y: 8 }, Data { x: 4, y: 5 }]

@mooman219 mooman219 changed the title Nightly - Borrow Checker allowing mutable and immutable reference out Borrow Checker allowing mutable and immutable reference out on a vec [Nightly] May 5, 2017
@SpaceManiac
Copy link

Here's an even more compressed example demonstrating the bad behavior:

struct Data(i32);

impl Data {
    fn oh_no(&mut self, other: &mut Data) {
        // shouldn't ever succeed, but does
        assert!(std::ptr::eq(self, other));
    }
}

fn main() {
    let mut v = vec![Data(0)];
    v[0].oh_no(&mut v[0]);
}

@aturon aturon added I-nominated I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 5, 2017
@nikomatsakis nikomatsakis added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label May 5, 2017
@nikomatsakis
Copy link
Contributor

Seems to have been introduced between nightly on 2017-04-30 (correctly errors) and 2017-05-01 (no error).

@nikomatsakis
Copy link
Contributor

lunch-box. git log --oneline afa1240e57330d85a372db4e28cd8bc8fa528ccb..06fb4d25642a3f223db1441972dd5962085cfba1
06fb4d2 Auto merge of #41651 - arielb1:missing-adjustment-2, r=eddyb
b7b3c23 refactor the handling of lvalue ops
c0f86f5 Auto merge of #41602 - hsivonen:explainnonnull, r=steveklabnik
78f6318 Auto merge of #41643 - frewsxcv:rollup, r=frewsxcv
43cb7c4 Rollup merge of #41637 - eddyb:used-not-dead, r=petrochenkov
eab2af9 Rollup merge of #41636 - moosingin3space:fix/process-exit-in-forget-doc, r=sfackler
c9f5a47 Rollup merge of #41613 - cuviper:fix-release-links, r=aturon
b146940 Rollup merge of #41608 - cuviper:distcheck-rust-src, r=alexcrichton
3c10706 Rollup merge of #41509 - froydnj:float-stack-reduction, r=nagisa
ab99c9b Rollup merge of #41449 - Eh2406:master, r=aturon
ba5b911 Auto merge of #41593 - achernyak:def_span, r=eddyb
c054b2a Don't ever warn about #[used] items being dead code.
087b838 process:exit -> process::exit in mem::forget docs
d3b7af0 removed custom functions and comment
00d02cf chaned dep nodes impl
ad1959e added dep nodes and comment
2f73b17 Merge branch 'master' into def_span
ba90718 found the stack overflow culprit
0e2571b FromIterator and Extend Cow for String
93ac5df review updateds
b2c3102 fmt: use mem::uninitialized for float formatting buffers
2499d81 fmt: use the minimum parts array size
a21f616 fmt: reduce the stack space required by float formatting
5a00785 num: add minimal benchmarks for full floating-point formatting
e8c4b7a Fix links in RELEASES.md for 1.10.0 through 1.12.0
932d251 query for def_span
e36f59e Explain why zero-length slices require a non-null pointer
c5cd4cb Add a distcheck for rust-src completeness

@nikomatsakis
Copy link
Contributor

Based purely on the log, most likely candidate seems to be #41651 (cc @arielb1).

@nikomatsakis
Copy link
Contributor

OK, I can confirm that #41651 is at fault (tested before/after). I don't have time now to look in more depth at the patch itself.

arielb1 added a commit to arielb1/rust that referenced this issue May 8, 2017
Hopefully this is the last PR needed.

Fixes rust-lang#41726.
Fixes rust-lang#41742.
Fixes rust-lang#41774.
frewsxcv added a commit to frewsxcv/rust that referenced this issue May 9, 2017
try to fix lvalue ops for real

Hopefully this is the last PR needed.

Fixes rust-lang#41726.
Fixes rust-lang#41742.
Fixes rust-lang#41774.
nikomatsakis pushed a commit to nikomatsakis/rust that referenced this issue May 22, 2017
Hopefully this is the last PR needed.

Fixes rust-lang#41726.
Fixes rust-lang#41742.
Fixes rust-lang#41774.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. 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