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.

Sign up for GitHub

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

Jump to bottom

Mutating matched value in pattern guard causes segfault in safe code #14684

Closed
zwarich opened this issue Jun 5, 2014 · 6 comments
Closed

Mutating matched value in pattern guard causes segfault in safe code #14684

zwarich opened this issue Jun 5, 2014 · 6 comments
Milestone
1.0

Comments

@zwarich
Copy link

zwarich commented Jun 5, 2014

The following example causes a segfault for me on OS X / x86_64:

enum Enum<'a> {
    A(&'a int),
    B(bool),
}

fn foo() -> int {
    let mut n = 42;
    let mut x = A(&mut n);
    match x {
        A(_) if { x = B(false); false } => 1,
        A(p) => *p,
        B(_) => 2,
    }
}

fn main() {
    foo();
}
@alexcrichton
Copy link
Member

Oh dear, nominating.

@zwarich
Copy link
Author

zwarich commented Jun 5, 2014

I guess the right thing to do is to restrict mutation and mutable borrows of the head expression in guards, but not in the arm expressions themselves.

@zwarich
Copy link
Author

zwarich commented Jun 6, 2014

The same restrictions also need to apply to any mutable references bound by the patterns.

@nikomatsakis
Copy link
Contributor

cc me

@nikomatsakis
Copy link
Contributor

related to #6393, because the best thing would be to say that the match induces a loan that begins after evaluating head expr and terminates when entering each arm body

@pnkfelix
Copy link
Member

P-backcompat-lang, 1.0.

@pnkfelix pnkfelix added this to the 1.0 milestone Jun 12, 2014
@pcwalton pcwalton mentioned this issue Jul 25, 2014
Closed
bors added a commit that referenced this issue Jul 29, 2014
@bors
auto merge of #15989 : pcwalton/rust/borrowck-pattern-guards, r=pnkfelix
6635fe7 
the CFG for match statements.

There were two bugs in issue #14684. One was simply that the borrow
check didn't know about the correct CFG for match statements: the
pattern must be a predecessor of the guard. This disallows the bad
behavior if there are bindings in the pattern. But it isn't enough to
prevent the memory safety problem, because of wildcards; thus, this
patch introduces a more restrictive rule, which disallows assignments
and mutable borrows inside guards outright.

I discussed this with Niko and we decided this was the best plan of
action.

This breaks code that performs mutable borrows in pattern guards. Most
commonly, the code looks like this:

    impl Foo {
        fn f(&mut self, ...) {}
        fn g(&mut self, ...) {
            match bar {
                Baz if self.f(...) => { ... }
                _ => { ... }
            }
        }
    }

Change this code to not use a guard. For example:

    impl Foo {
        fn f(&mut self, ...) {}
        fn g(&mut self, ...) {
            match bar {
                Baz => {
                    if self.f(...) {
                        ...
                    } else {
                        ...
                    }
                }
                _ => { ... }
            }
        }
    }

Sometimes this can result in code duplication, but often it illustrates
a hidden memory safety problem.

Closes #14684.

[breaking-change]

r? @pnkfelix
@zwarich zwarich removed their assignment Jan 31, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0
Development

Successfully merging a pull request may close this issue.

librustc: Disallow mutation and assignment in pattern guards, and modify pcwalton/rust
4 participants
@alexcrichton @nikomatsakis @pnkfelix @zwarich