Skip to content

Segfault with enum struct and method #5625

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
ghost opened this issue Mar 29, 2013 · 5 comments
Closed

Segfault with enum struct and method #5625

ghost opened this issue Mar 29, 2013 · 5 comments
Labels
A-codegen Area: Code generation I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.

Comments

@ghost
Copy link

ghost commented Mar 29, 2013

Running this test.rs causes a segfault on Ubuntu 12.10 (compiled with incoming bda4dd3):

enum Foo {
    Bar,
    Baz {a: uint}
}

impl Foo {
    fn equals(&self, other: &Foo) -> bool {
        match *self {
            Bar => match *other { Bar => true, _ => false },
            Baz {a: ref sa} => match *other {
                Baz {a: ref oa} => sa == oa,
                _ => false
            }
        }
    }
}

pub fn main() {
    let x = Baz {a: 4};
    let y = Bar;
    x.equals(&y);
}

Valgrind says this about it:

==16637== Thread 2:
==16637== Use of uninitialised value of size 8
==16637==    at 0x400AE8: uint::__extensions__::meth_2509::eq::_693b4a7b2ad824e5::_00 (test.rs:1)
==16637==    by 0x400A9D: ptr::__extensions__::eq_2502::_37b8589111841e1::_00 (test.rs:1)
==16637==    by 0x400A3B: __extensions__::meth_2459::equals::_e3a12daf95a36916::_00 (test.rs:11)
==16637==    by 0x400B68: main::_6706ae2286769fe::_00 (test.rs:21)
==16637==    by 0x400B9D: _rust_main (test.rs:12)
==16637==    by 0x535E4C3: task_start_wrapper(spawn_args*) (rust_task.cpp:162)
==16637== 
==16637== Invalid read of size 8
==16637==    at 0x400AE8: uint::__extensions__::meth_2509::eq::_693b4a7b2ad824e5::_00 (test.rs:1)
==16637==    by 0x400A9D: ptr::__extensions__::eq_2502::_37b8589111841e1::_00 (test.rs:1)
==16637==    by 0x400A3B: __extensions__::meth_2459::equals::_e3a12daf95a36916::_00 (test.rs:11)
==16637==    by 0x400B68: main::_6706ae2286769fe::_00 (test.rs:21)
==16637==    by 0x400B9D: _rust_main (test.rs:12)
==16637==    by 0x535E4C3: task_start_wrapper(spawn_args*) (rust_task.cpp:162)
==16637==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==16637== 
==16637== 
==16637== Process terminating with default action of signal 11 (SIGSEGV)
==16637==  Access not within mapped region at address 0x0
==16637==    at 0x400AE8: uint::__extensions__::meth_2509::eq::_693b4a7b2ad824e5::_00 (test.rs:1)
==16637==    by 0x400A9D: ptr::__extensions__::eq_2502::_37b8589111841e1::_00 (test.rs:1)
==16637==    by 0x400A3B: __extensions__::meth_2459::equals::_e3a12daf95a36916::_00 (test.rs:11)
==16637==    by 0x400B68: main::_6706ae2286769fe::_00 (test.rs:21)
==16637==    by 0x400B9D: _rust_main (test.rs:12)
==16637==    by 0x535E4C3: task_start_wrapper(spawn_args*) (rust_task.cpp:162)

Note that the following variants of the last line of main do not cause a segfault:

  • x.equals(&x);
  • y.equals(&y);
  • y.equals(&x);
@huonw
Copy link
Member

huonw commented Apr 2, 2013

Similar code that segfaults, except when _x2 is replaced by ref _x2, or when compiling with opt-level > 0.

enum AB {
    A {x: int},
    B
}

fn main() {
    let a = A {x : 1};
    let b = B;

    match a {
        A {x : _x1} => {
            match b {
                A {x : _x2} => io::println("AA"),
                _ => io::println("A_")
            }
        },
        _ => io::println("_")
    }
}

#5530 is almost certainly related (and quite probably the same issue), since both of these examples don't segfault when the _ patterns are replaced by the appropriate variant.

@Aatch
Copy link
Contributor

Aatch commented Jun 5, 2013

Hmm, seems like it's an issue with struct variants. There are two things to note, 1) this code works as expected:

enum AB {
    A {x: int},
    B
}

fn main() {
    let a = A { x: 1 };
    let b = B;

    match a {
        A {x : _x1} => {
            match b {
                A {x : _x2} => println("AA"),
                _ => println("A_")
            }
        },
        _ => println("_")
    }
}

and 2) even though the optimized example doesn't segfault, it still fails because it prints out AA instead of A_.

@pnkfelix
Copy link
Member

The original test case also segfaults for me on Mac OS X. (Noting this because the comments from tjc in #5530 indicate that the failure there might be Linux-specific. But then again, I just saw the test from #5530 fail as well on my Mac.)

@emberian
Copy link
Member

emberian commented Aug 5, 2013

Still reproduces

@dim-an
Copy link
Contributor

dim-an commented Aug 6, 2013

There is a simpler testcase:

enum AB {
    A {x: int},
    B
}

fn main() {
    let a = B;
    match a {
        A {x : _x} => {fail!()}
        _ => {}
    }
}

This code gets compiled to the following llvm-assembly:

; Function Attrs: uwtable
define void @"_ZN4main17_23295c4eb32a43657_0$x2e0E"({ i64, %tydesc*, i8*, i8*, i8 }*) #0 {
"function top level":
  %a = alloca %enum.AB
  %__llmatch = alloca i64*
  %_x = alloca i64
  %__trans_ret_slot = alloca {}
  %1 = alloca %str_slice
  %2 = alloca %str_slice
  %3 = getelementptr inbounds %enum.AB* %a, i32 0, i32 0
  store i64 1, i64* %3
  %4 = getelementptr inbounds %enum.AB* %a, i32 0, i32 0
  %5 = load i64* %4, !range !0
  switch i64 %5, label %match_else [
    i64 0, label %match_case
  ]

case_body:                                        ; preds = %match_else, %match_case
  %6 = load i64** %__llmatch
  %7 = load i64* %6
  store i64 %7, i64* %_x
  %8 = getelementptr inbounds %str_slice* %1, i32 0, i32 0
  store i8* getelementptr inbounds ([17 x i8]* @str3379, i32 0, i32 0), i8** %8
  %9 = getelementptr inbounds %str_slice* %1, i32 0, i32 1
  store i64 17, i64* %9
  %10 = getelementptr inbounds %str_slice* %2, i32 0, i32 0
  store i8* getelementptr inbounds ([11 x i8]* @str3380, i32 0, i32 0), i8** %10
  %11 = getelementptr inbounds %str_slice* %2, i32 0, i32 1
  store i64 11, i64* %11
  call void @"_ZN3sys14__extensions__10meth_131479fail_with16_db4c44d01ce411614_0$x2e8$x2dpreE"({}* %__trans_ret_slot, { i64, %tydesc*, i8*, i8*, i8 }* undef, %str_slice* %1, %str_slice* %2, i64 9)
  unreachable

case_body1:                                       ; No predecessors!
  br label %join

match_else:                                       ; preds = %"function top level"
  br label %case_body

match_case:                                       ; preds = %"function top level"
  %12 = bitcast %enum.AB* %a to { i64, i64 }*
  %13 = getelementptr inbounds { i64, i64 }* %12, i32 0, i32 1
  store i64* %13, i64** %__llmatch
  br label %case_body

join:                                             ; preds = %case_body1
  ret void
}

as you can see both match_case and match_else lables jumps (brs) to case_body, while match_else should really jump to case_body1

bors added a commit that referenced this issue Aug 9, 2013
Code that collects fields in struct-like patterns used to ignore
wildcard patterns like `Foo{_}`. But `enter_defaults` considered
struct-like patterns as default in order to overcome this
(accoring to my understanding of situation).

However such behaviour caused code like this:
```
enum E {
    Foo{f: int},
    Bar
}
let e = Bar;
match e {
    Foo{f: _f} => { /* do something (1) */ }
    _ => { /* do something (2) */ }
}
```
consider pattern `Foo{f: _f}` as default. That caused inproper behaviour
and even segfaults while trying to destruct `Bar` as `Foo{f: _f}`.
Issues: #5625 , #5530.

This patch fixes `collect_record_or_struct_fields` to split cases of
single wildcard struct-like pattern and no struct-like pattern at all.
Former case resolved with `enter_rec_or_struct` (and not with
`enter_defaults`).

Closes #5625.
Closes #5530.
@bors bors closed this as completed in 0fadfc5 Aug 9, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation 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.

5 participants