Skip to content

Unsafe &mut using RefCell #18566

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
murarth opened this issue Nov 3, 2014 · 8 comments
Closed

Unsafe &mut using RefCell #18566

murarth opened this issue Nov 3, 2014 · 8 comments
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@murarth
Copy link
Contributor

murarth commented Nov 3, 2014

The following code will compile and run (and fail an assertion), when it should not compile:

use std::cell::RefCell;

struct A {
    a: RefCell<Vec<int>>,
}

fn main() {
    let mut a = A{a: RefCell::new(vec![1])};

    for i in a.a.borrow().iter() {
        let r = &mut a;
        r.a = RefCell::new(vec![2]);
        println!("{}", i);
    }
}

Notably, this version properly produces a compile error, recognizing the existing immutable borrow:

use std::cell::RefCell;

struct A {
    a: RefCell<Vec<int>>,
}

fn main() {
    let mut a = A{a: RefCell::new(vec![1])};
    let b = a.a.borrow();

    for i in b.iter() {
        let r = &mut a;
        r.a = RefCell::new(vec![2]);
        println!("{}", i);
    }
}
@emberian
Copy link
Member

emberian commented Nov 3, 2014

Nominating for P-high, this is an obvious soundness issue.

@arielb1
Copy link
Contributor

arielb1 commented Nov 3, 2014

Minimized:

#![feature(tuple_indexing)]

struct MyPtr<'a>(&'a mut uint);
impl<'a> Deref<uint> for MyPtr<'a> {
    fn deref<'b>(&'b self) -> &'b uint { self.0 }
}

trait Tr {
  fn poke(&self, s: &mut uint);
}
impl Tr for uint {
  fn poke(&self, s: &mut uint)  {
      println!("{}", self);
      *s = 2;
      println!("{}", self);
  }
}

fn main() {
    let s = &mut 1u;

    MyPtr(s).poke(s);
}

It seems that deref doesn't properly create a borrow (I didn't manage to use this to create a regionck violation). The bug remains if I use DerefMut and test takes an &uint.

@arielb1
Copy link
Contributor

arielb1 commented Nov 5, 2014

This is a regression. It errors properly in Rust 0.12

@nikomatsakis nikomatsakis self-assigned this Nov 6, 2014
@brson brson added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Nov 6, 2014
@nikomatsakis
Copy link
Contributor

I get an error using the latest master.

@pnkfelix
Copy link
Member

pnkfelix commented Nov 6, 2014

believed fixed, needs test.

@nikomatsakis nikomatsakis removed their assignment Nov 6, 2014
@murarth murarth closed this as completed Nov 11, 2014
@huonw
Copy link
Member

huonw commented Nov 11, 2014

Reopening because the test has not yet landed. 😄

@huonw huonw reopened this Nov 11, 2014
@murarth
Copy link
Contributor Author

murarth commented Nov 11, 2014

Oh. My mistake.

@murarth
Copy link
Contributor Author

murarth commented Nov 13, 2014

Now it's time to close. 😄

@murarth murarth closed this as completed Nov 13, 2014
lnicola added a commit to lnicola/rust that referenced this issue Nov 28, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
Development

No branches or pull requests

7 participants