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

Rust compiles invalid program which segfaults - borrowed pointer to trait object outlives original value #11374

Closed
netvl opened this issue Jan 7, 2014 · 4 comments · Fixed by #17199
Labels
A-typesystem Area: The type system E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@netvl
Copy link
Contributor

netvl commented Jan 7, 2014

Here is a very simple program which employs trait objects:

c.rc:

#[crate_id = "c#0.1"];
#[crate_type = "bin"];

mod m1;
mod m2;

m1.rs:

use std::io;

pub struct Container<'a> {
    priv reader: &'a mut Reader
}

impl<'a> Container<'a> {
    pub fn wrap<'s>(reader: &'s mut Reader) -> Container<'s> {
        Container { reader: reader }
    }

    pub fn read_to(&self, vec: &mut [u8]) {
        self.reader.read(vec);
    }
}

pub fn for_stdin<'a>() -> Container<'a> {
    let mut r = io::stdin();
    Container::wrap(&mut r as &mut Reader)
}

m2.rs:

use std::vec;

use m1;

#[main]
fn main() {
    let c = m1::for_stdin();
    let mut v = vec::from_elem(10, 0u8);
    c.read_to(v.as_mut_slice());
}

When this program is compiled and run, it fails with segmentation fault:

% rustc c.rc
% ./c
zsh: segmentation fault (core dumped)  ./c

This program is not correct. You can see that m1::for_stdin() function produces a Container<'a, ...> for arbitrary lifetime 'a, but the local variable r is valid only for for_stdin() body. A pointer to r, however, is saved to the Container, and when it is accessed inside m2::main(), a segmentation fault occurs. However, Rust still compiles it. I thought that borrowing checker should prevent this kind of errors.

BTW, I can write &mut r or &mut r as &mut Reader and it will compile either way, but I thought that when going from plain pointers to trait objects I should always specify exact kind of trait object. Not sure that I'm right here thought.

@alexcrichton
Copy link
Member

cc @nikomatsakis

@pnkfelix
Copy link
Member

pnkfelix commented Jan 7, 2014

Sounds similar to #5723; potential duplicate?

@jfager
Copy link
Contributor

jfager commented Jul 26, 2014

This appears to be fixed:

$ rustc -v
rustc 0.12.0-pre-nightly (cf1381c1d 2014-07-26 00:46:16 +0000)

$ rustc c.rs 
m1.rs:13:9: 13:20 error: cannot borrow data mutably in a `&` reference 
m1.rs:13         self.reader.read(vec);
                 ^~~~~~~~~~~ 
m1.rs:19:26: 19:27 error: `r` does not live long enough
m1.rs:19     Container::wrap(&mut r as &mut Reader)
                                  ^
m1.rs:17:41: 20:2 note: reference must be valid for the lifetime 'a as defined on the block at 17:40...
m1.rs:17 pub fn for_stdin<'a>() -> Container<'a> { 
m1.rs:18     let mut r = io::stdin();
m1.rs:19     Container::wrap(&mut r as &mut Reader)
m1.rs:20 }
m1.rs:17:41: 20:2 note: ...but borrowed value is only valid for the block at 17:40
m1.rs:17 pub fn for_stdin<'a>() -> Container<'a> { 
m1.rs:18     let mut r = io::stdin();
m1.rs:19     Container::wrap(&mut r as &mut Reader)
m1.rs:20 }
error: aborting due to 2 previous errors

Original example code updated to current rust:

c.rs:

#![crate_name = "c"]
#![crate_type = "bin"]

mod m1;
mod m2;

m1.rs:

use std::io;

pub struct Container<'a> {
    reader: &'a mut Reader
}

impl<'a> Container<'a> {
    pub fn wrap<'s>(reader: &'s mut Reader) -> Container<'s> {
        Container { reader: reader }
    }

    pub fn read_to(&self, vec: &mut [u8]) {
        self.reader.read(vec);
    }
}

pub fn for_stdin<'a>() -> Container<'a> {
    let mut r = io::stdin();
    Container::wrap(&mut r as &mut Reader)
}

m2.rs:

use std::vec;

use m1;

#[main]
fn main() {
    let c = m1::for_stdin();
    let mut v = vec::Vec::from_elem(10, 0u8);
    c.read_to(v.as_mut_slice());
}

@alexcrichton
Copy link
Member

Awesome! flagging as needstest (not sure if one already exists)

bors added a commit that referenced this issue Sep 15, 2014
…hton

Closes #7813.
Closes #10902.
Closes #11374.
Closes #11714.
Closes #12920.
Closes #13202.
Closes #13624.
Closes #14039.
Closes #15730.
Closes #15783.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-typesystem Area: The type system 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

Successfully merging a pull request may close this issue.

5 participants