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

stack allocated vecs can be returned from functions #3243

Closed
erickt opened this Issue Aug 21, 2012 · 14 comments

Comments

Projects
None yet
8 participants
@erickt
Contributor

erickt commented Aug 21, 2012

See this gist: https://gist.github.com/3419521, with the attached llvm bitcode. Rust compiles it fine, but looking through the bitcode, it appears that rust is returning a stack pointer.

@ghost ghost assigned nikomatsakis Aug 21, 2012

@catamorphism

This comment has been minimized.

Show comment
Hide comment
@catamorphism

catamorphism Dec 7, 2012

Contributor

Reproduced as of d2ad028

Contributor

catamorphism commented Dec 7, 2012

Reproduced as of d2ad028

@graydon

This comment has been minimized.

Show comment
Hide comment
@graydon

graydon Dec 11, 2012

Contributor

Hm. I think this is just returning the slice, which is ok, no? The lifetime is static, and it outlives the caller lifetime as inferred..

Contributor

graydon commented Dec 11, 2012

Hm. I think this is just returning the slice, which is ok, no? The lifetime is static, and it outlives the caller lifetime as inferred..

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Dec 12, 2012

@graydon Well, that example is a bit unfortunate but the following compiles too:

fn main() {
    for (|| &[1, 2, 3])().each |i| {
        io::println(fmt!("%d", *i));
    }
}

and prints out rubbish for me.

ghost commented Dec 12, 2012

@graydon Well, that example is a bit unfortunate but the following compiles too:

fn main() {
    for (|| &[1, 2, 3])().each |i| {
        io::println(fmt!("%d", *i));
    }
}

and prints out rubbish for me.

@graydon

This comment has been minimized.

Show comment
Hide comment
@graydon

graydon Dec 13, 2012

Contributor

That's also fine. &[1,2,3] is a slice to a static array (i.e. with unlimited lifetime). Or .. it should be!

Contributor

graydon commented Dec 13, 2012

That's also fine. &[1,2,3] is a slice to a static array (i.e. with unlimited lifetime). Or .. it should be!

@graydon

This comment has been minimized.

Show comment
Hide comment
@graydon

graydon Dec 13, 2012

Contributor

Oh. I guess if it prints out rubbish it's probably compiling as non-static. Yeah, that's bad.

Contributor

graydon commented Dec 13, 2012

Oh. I guess if it prints out rubbish it's probably compiling as non-static. Yeah, that's bad.

@thestinger

This comment has been minimized.

Show comment
Hide comment
@thestinger

thestinger Mar 20, 2013

Contributor

Stack arrays should be returned by-value (so through an output parameter). They can't be static because they're mutable.

Contributor

thestinger commented Mar 20, 2013

Stack arrays should be returned by-value (so through an output parameter). They can't be static because they're mutable.

@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix Mar 25, 2013

Member

Reproduced as of c202430

Here is a slightly elaborated version of fawek's example that I used to better understand the scenario:

const some_array : &'static [int] = &'static [1,2,3];

fn main() {
    io::println("Direct");
    for [1, 2, 3].each |i| {
        io::println(fmt!("%d", *i));
    }

    io::println("Indirect global");
    for (|| &some_array)().each |i| {
        io::println(fmt!("%d", *i));
    }

    io::println("Indirect local");
    for (|| &[1, 2, 3])().each |i| {
        io::println(fmt!("%d", *i));
    }
}

On my machine, it prints:

Direct
1
2
3
Indirect global
1
2
3
Indirect local
24
140552821560312
24
Member

pnkfelix commented Mar 25, 2013

Reproduced as of c202430

Here is a slightly elaborated version of fawek's example that I used to better understand the scenario:

const some_array : &'static [int] = &'static [1,2,3];

fn main() {
    io::println("Direct");
    for [1, 2, 3].each |i| {
        io::println(fmt!("%d", *i));
    }

    io::println("Indirect global");
    for (|| &some_array)().each |i| {
        io::println(fmt!("%d", *i));
    }

    io::println("Indirect local");
    for (|| &[1, 2, 3])().each |i| {
        io::println(fmt!("%d", *i));
    }
}

On my machine, it prints:

Direct
1
2
3
Indirect global
1
2
3
Indirect local
24
140552821560312
24
@graydon

This comment has been minimized.

Show comment
Hide comment
@graydon

graydon Mar 26, 2013

Contributor

mhm. definitely disappointing and worth fixing (keeping those importance labels) but I don't think it's a showstopper for 0.6. removing milestone.

Contributor

graydon commented Mar 26, 2013

mhm. definitely disappointing and worth fixing (keeping those importance labels) but I don't think it's a showstopper for 0.6. removing milestone.

@metajack

This comment has been minimized.

Show comment
Hide comment
@metajack

metajack May 16, 2013

Contributor

Still reproducible,, thought I get slightly different output.

Nominating for production ready.

Contributor

metajack commented May 16, 2013

Still reproducible,, thought I get slightly different output.

Nominating for production ready.

@graydon

This comment has been minimized.

Show comment
Hide comment
@graydon

graydon Jun 20, 2013

Contributor

accepted for production-ready milestone

Contributor

graydon commented Jun 20, 2013

accepted for production-ready milestone

@kballard

This comment has been minimized.

Show comment
Hide comment
@kballard

kballard Aug 26, 2013

Contributor

This is a pretty nasty and subtle bug. Just spent a while trying to figure out how the heck I was getting [0] back, and finally tracked it down to returning the results of bytes!(".") and having that transform into &[0] behind my back. Given how easy it is to accidentally trigger (especially with bytes!()) I think it should be a bit higher priority.

Contributor

kballard commented Aug 26, 2013

This is a pretty nasty and subtle bug. Just spent a while trying to figure out how the heck I was getting [0] back, and finally tracked it down to returning the results of bytes!(".") and having that transform into &[0] behind my back. Given how easy it is to accidentally trigger (especially with bytes!()) I think it should be a bit higher priority.

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Aug 26, 2013

Contributor

@kballard yes, you're right. I'll take a look, I suspect the fix is fairly straightforward.

Contributor

nikomatsakis commented Aug 26, 2013

@kballard yes, you're right. I'll take a look, I suspect the fix is fairly straightforward.

@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix Jan 16, 2014

Member

accepted for P-backcompat-lang.

Member

pnkfelix commented Jan 16, 2014

accepted for P-backcompat-lang.

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Jan 28, 2014

Contributor

This was fixed as part of #3511. However, no test case. I'll add one.

Contributor

nikomatsakis commented Jan 28, 2014

This was fixed as part of #3511. However, no test case. I'll add one.

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Jan 28, 2014

Add test case for rust-lang#3243, which was fixed as part of fix for r…
…ust-lang#3511.

(Lifetime of stack allocated vectors was not being enforced)

Closes rust-lang#3243.

bors added a commit that referenced this issue Jan 29, 2014

auto merge of #11889 : nikomatsakis/rust/issue-3243-stack-alloc-vec, …
…r=nikomatsakis

(Lifetime of stack allocated vectors was not being enforced)

Closes #3243.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment