rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.97k stars 12.79k forks source link

Segfault with enum struct and method #5625

Closed ghost closed 11 years ago

ghost commented 11 years ago

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

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:

huonw commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

Still reproduces

dim-an commented 11 years ago

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