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

The Cell type may be unsound as it is currently implemented #10231

Closed
alexcrichton opened this issue Nov 2, 2013 · 8 comments
Closed

The Cell type may be unsound as it is currently implemented #10231

alexcrichton opened this issue Nov 2, 2013 · 8 comments
Assignees

Comments

@alexcrichton
Copy link
Member

use std::cell::Cell;

struct A;

impl A {
    fn id<'a>(&'a mut self) -> &'a mut A { self }

    fn bar(&mut self) {
        println!("oh no!");
    }
}

fn foo(f: &fn()) { f() }

fn main() {
    let c = Cell::new_empty();
    let mut a = A;
    let d = Cell::new(&mut a);
    do d.with_mut_ref |this| {
        c.put_back(this.id());
    }
    do d.with_mut_ref |this1| {
        do c.with_mut_ref |this2| {
            this1.bar();
            this2.bar();
        }
    }
}

Near the end this and this2 are two mutable pointers to the same object, which I believe is unsound.

Nominating, this is not good.

@alexcrichton
Copy link
Member Author

Hm, thinking about this, I feel like this may be a dupe of #8624. @nikomatsakis, can you confirm? (if so I'll close)

@nikomatsakis
Copy link
Contributor

I'll take a look and confirm. I am planning to fix #8624 this week, so ...

@nikomatsakis
Copy link
Contributor

could also be #5781

@ghost ghost assigned nikomatsakis Nov 2, 2013
@pnkfelix
Copy link
Member

pnkfelix commented Nov 7, 2013

just-a-bug

@nikomatsakis
Copy link
Contributor

I...don't think this is still an issue. I tried to translate this test to modern syntax as best I could:

use std::cell::RefCell;

struct A;

impl A {
    fn id<'a>(&'a mut self) -> &'a mut A { self }

    fn bar(&mut self) {
        println!("oh no!");
    }
}

fn foo(f: ||) { f() }

fn main() {
    let mut v1 = A;
    let mut v2 = A;
    let c = RefCell::new(&mut v1);
    let d = RefCell::new(&mut v2);
    let mut r = d.borrow_mut();
    let mut p: &mut &mut A = r.get();
    c.set(&mut **p);
    p.bar(); //~ ERROR cannot borrow
}

@nikomatsakis
Copy link
Contributor

I'm not precisely sure what was the issue originally, it may have been #6801, since the older cell api involved a lot more closures. Might be worth digging up the original version of Rust and trying to track it down, since unsoundness is a big concern.

@nikomatsakis
Copy link
Contributor

Actually, quite likely this was #11385 (though in another guise) -- that is, an unsound result of variance inference. It "feels" right though without having reference to the original cell type I'm not able to completely spell out the chain of events.

@alexcrichton
Copy link
Member Author

Closing, Cell in this form is long gone and I believe that the usage of the new marker types has fixed this bug, closing as outdated.

flip1995 pushed a commit to flip1995/rust that referenced this issue Feb 10, 2023
`invalid_regex`: Show full error when string value doesn't match source

changelog: [`invalid_regex`]: Show full error when parsing non-literals or regular strings containing escape sequences

Fixes rust-lang#4170, the escape sequence there causes the span to be incorrect which will have caused most of the confusion
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

No branches or pull requests

3 participants