Skip to content

core dump: fn pointer in iterator #13595

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

Closed
phildawes opened this issue Apr 18, 2014 · 12 comments · Fixed by #15961
Closed

core dump: fn pointer in iterator #13595

phildawes opened this issue Apr 18, 2014 · 12 comments · Fixed by #15961
Labels
A-lifetimes Area: Lifetimes / regions I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Milestone

Comments

@phildawes
Copy link
Contributor

Hello! The following dumps core when run on my machine (Ubuntu 12.04 LTS, rust-nightly downloaded today (18th Apr)). Ziad Hatahet confirmed the same on a slightly older version of rust.

struct StateMachineIter<'a> {
    statefn: &'a fn(&mut StateMachineIter<'a>) -> Option<&'static str>
}

impl<'a> Iterator<&'static str> for StateMachineIter<'a> {
    fn next(&mut self) -> Option<&'static str> {
        return  (*self.statefn)(self);
    }
}

fn state1(self_: &mut StateMachineIter) -> Option<&'static str> {
    self_.statefn = &state2;
    return Some("state1");
}

fn state2(self_: &mut StateMachineIter) -> Option<(&'static str)> {
    self_.statefn = &state3;
    return Some("state2");
}

fn state3(self_: &mut StateMachineIter) -> Option<(&'static str)> {
    self_.statefn = &finished;
    return Some("state3");
}

fn finished(_: &mut StateMachineIter) -> Option<(&'static str)> {
    return None;
}

fn state_iter() -> StateMachineIter {
    StateMachineIter { statefn: &state1 }
}


fn main() {
    let mut it = state_iter();
    println!("{}",it.next());
    println!("{}",it.next());
    println!("{}",it.next());
    println!("{}",it.next());
    println!("{}",it.next());
}
@sfackler
Copy link
Member

There's an extra layer of indirection in that struct definition. A &'a fn() is a pointer to a pointer to a function. A statement like self_.statefn = &finished; expands to let foo = finished; self_.statefn = &foo. That's obviously creating a dangling pointer onto the stack which is why it ends up crashing. Adjusting the struct definition to

struct StateMachineIter {
    statefn: fn(&mut StateMachineIter) -> Option<&'static str>
}

should make things work as you want. There's also a bug in rustc for even letting that program compile. I'm not sure if it's already been run into and filed.

@alexcrichton
Copy link
Member

cc @nikomatsakis

@edwardw
Copy link
Contributor

edwardw commented Apr 18, 2014

This fixes it:

...
fn finished(self_: &mut StateMachineIter) -> Option<(&'static str)> {
    self_.statefn = &finished;
    return None;
}
...

But I didn't quite understand why.

@sfackler
Copy link
Member

@edwardw it probably happens to reset that part of the stack to the right value.

@artella-coding
Copy link

@alexcrichton @nikomatsakis @huonw p.s. This is still crashing in rustc 0.12.0-pre-nightly (f50e4ee559c0e6253e9d46a8e6763e25f7b90d90 2014-07-18 00:01:22 +0000). Thanks

@alexcrichton
Copy link
Member

Nominating, something fishy is going on here.

@zwarich
Copy link

zwarich commented Jul 19, 2014

Here's the simplest possible reduction I could make:

struct A<'a> {
    func: &'a fn() -> Option<int>
}

impl<'a> A<'a> {
    fn call(&self) -> Option<int> {
        (*self.func)()
    }
}

fn foo() -> Option<int> {
    None
}

fn create() -> A<'static> {
    A { func: &foo }
}

fn main() {
    let a = create();
    a.call();
}

@huonw
Copy link
Member

huonw commented Jul 19, 2014

Is the compiler mixing up the function pointer with the actual function code?

That is,

fn foo() {}

fn main() { let x = foo; }

is like

static foo_code: [u8, .. N] = [/*machine code*/];

fn main() { let x = &foo_code; } // *not* let x = foo_code;

So writing &foo is like &(&foo_code), i.e. a non-'static reference to the 'static function pointer.

@zwarich
Copy link

zwarich commented Jul 19, 2014

@huonw It's just creating an alloca, storing the function pointer into it, borrowing that, and returning the borrowed pointer. For example, on the program

fn foo() { }

fn bar() -> &'static fn() {
    &'static foo
}

I get this LLVM IR:

define internal void @_ZN3foo20h98be24cee3d646d9eaa4v0.0E() unnamed_addr #0 {
entry-block:
  ret void
}

define internal nonnull void ()** @_ZN3bar20hb2bd4683ab9952d6haa4v0.0E() unnamed_addr #0 {
entry-block:
  %addr_of = alloca void ()*
  store void ()* @_ZN3foo20h98be24cee3d646d9eaa4v0.0E, void ()** %addr_of
  ret void ()** %addr_of
}

Basically, the borrow checker doesn't seem to be in on the joke that &foo is referring to a local alloca.

@huonw
Copy link
Member

huonw commented Jul 19, 2014

That is the behaviour I would expect if my statement were true. (The double ** in the LLVM signature indicates what I was talking about: a function value foo is already the 'static pointer, and so the foo name doesn't have 'static lifetime.)

@brson brson added this to the 1.0 milestone Jul 24, 2014
@brson
Copy link
Contributor

brson commented Jul 24, 2014

1.0 backcompat-lang

@nikomatsakis
Copy link
Contributor

This is probably a mismatch between trans/mem-categorization. I expect mem-categorization is saying that the "reference to the fn" is a static item, but trans is translating it into an rvalue (and hence on the stack).

pcwalton added a commit to pcwalton/rust that referenced this issue Jul 24, 2014
This breaks code like:

    struct A<'a> {
        func: &'a fn() -> Option<int>
    }

    fn foo() -> Option<int> { ... }

    fn create() -> A<'static> {
        A {
            func: &foo
        }
    }

Change this code to not take functions by reference. For example:

    struct A {
        func: extern "Rust" fn() -> Option<int>
    }

    fn foo() -> Option<int> { ... }

    fn create() -> A {
        A {
            func: foo
        }
    }

Closes rust-lang#13595.

[breaking-change]
bors added a commit that referenced this issue Jul 25, 2014
This breaks code like:

    struct A<'a> {
        func: &'a fn() -> Option<int>
    }

    fn foo() -> Option<int> { ... }

    fn create() -> A<'static> {
        A {
            func: &foo
        }
    }

Change this code to not take functions by reference. For example:

    struct A {
        func: extern "Rust" fn() -> Option<int>
    }

    fn foo() -> Option<int> { ... }

    fn create() -> A {
        A {
            func: foo
        }
    }

Closes #13595.

[breaking-change]

r? @huonw
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lifetimes Area: Lifetimes / regions I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants