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

Box<&mut int> allows for constructing an &int aliasing the &mut, while the &mut is still modifyable #14498

Closed
huonw opened this issue May 28, 2014 · 10 comments · Fixed by #14536
Milestone

Comments

@huonw
Copy link
Member

huonw commented May 28, 2014

fn main() {
    let mut a = 3i;
    let b = box &mut a;
    let c = &b;
    let d = &***c; // point to the data
    **b = 4i; // modify the data!!!

    println!("{} {}", c, d); // both 4
}
@pcwalton
Copy link
Contributor

Nominating for P-backcompat-lang as a borrow check bug

@pcwalton
Copy link
Contributor

cc @nikomatsakis

@pnkfelix
Copy link
Member

cc me

@brson brson added this to the 1.0 milestone May 29, 2014
@brson
Copy link
Contributor

brson commented May 29, 2014

1.0 P-backcompat-lang.

@huonw
Copy link
Member Author

huonw commented May 29, 2014

Also, it doesn't fail when using &mut instead of Box:

fn main() {
    let mut a = 3i;
    let b = &mut &mut a;
    let c = &b;
    let d = &***c;
    **b = 4i;

    println!("{} {}", c, d);
}
14498.rs:6:5: 6:8 error: cannot assign to `**b` because it is borrowed
14498.rs:6     **b = 4i;
               ^~~
14498.rs:4:14: 4:15 note: borrow of `**b` occurs here
14498.rs:4     let c = &b;
                        ^

@anasazi
Copy link
Contributor

anasazi commented May 30, 2014

I think d is actually irrelevant to the real bug here. The assignment shouldn't be allowed while c exists since the immutable borrow freezes everything. It seems to still work with &mut, so maybe the owned pointer version just got lost in the ~ to box transition.

@anasazi
Copy link
Contributor

anasazi commented May 30, 2014

Side note: the current Redex model correctly rejects this code (yay!).

@zwarich
Copy link

zwarich commented May 30, 2014

I have a simple fix for this: https://gist.github.com/zwarich/6f6bace941c87aeb7587. I'm testing on top of a lot of other borrowck changes, but this code in particular wasn't modified. I'll add a test, try again on master, and submit a PR.

@huonw
Copy link
Member Author

huonw commented May 30, 2014

@anasazi theoretically the ~ to box transition was just renaming the syntax, I believe Box is still represented in the compiler in exactly the same way that ~ was.

(Maybe @cmr could try this code on the last pre-~-removal compiler (or anything before that) to see if it was rejected then.)

@anasazi
Copy link
Contributor

anasazi commented May 30, 2014

@huonw I just figured we would have run into & not actually freezing ~ pointers a long time ago, but it's possible I suppose.

bors added a commit that referenced this issue May 30, 2014
Make check_for_assignment_to_restricted_or_frozen_location treat
mutation through an owning pointer the same way it treats mutation
through an &mut pointer, where mutability must be inherited from the
base path.

I also included GC pointers in this check, as that is what the
corresponding code in gather_loans/restrictions.rs does, but I don't
think there is a way to test this with the current language.

Fixes #14498.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants