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.

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

segmentation fault: moved value not detected due to trait usage #14061

Closed
dhardy opened this issue May 9, 2014 · 8 comments
Closed

segmentation fault: moved value not detected due to trait usage #14061

dhardy opened this issue May 9, 2014 · 8 comments
Labels
I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Milestone

Comments

@dhardy
Copy link
Contributor

dhardy commented May 9, 2014

With a little bit of hacking, I found a memory safety violation:

// This causes a seg-fault.

struct S<T> { val : T }
trait Gettable<T>{
    fn get(&self) -> T;
}
impl<T : Copy> Gettable<T> for S<T> {
    // Note: ~str does not satisfy Copy yet this compiles
    fn get(&self) -> T { self.val }
}

fn main() {
    let t : Box<S<~str>> = box S { val: "one".to_owned() };
    let a = t as Box<Gettable<~str>>;
    let b = a.get();    // causes a move
    //Note: without use of a trait, doing this results in:
    //error: instantiating a type parameter with an incompatible type `~str`, which does not fulfill `Pod`

    println!( "a: {}", a.get() );       // another move, crash ensues
    println!( "b: {}", b );
}

Note: what's the best way to "get" a value via a trait? Copy and restrict to POD types? Reference, even if most of the values accessed are just ints? Or use clone − and risk issues on complex types?

@huonw huonw added the I-crash label May 9, 2014
@edwardw
Copy link
Contributor

edwardw commented May 9, 2014

A workaround is to define the built-in bound on the trait instead of the implementation:

trait Gettable<T:Copy>{
    fn get(&self) -> T;
}

Then the "not fulfill Pod" error is caught as expected. This could be a backward compatibility hazard. Nominate?

@dhardy
Copy link
Contributor Author

dhardy commented May 10, 2014

So a fix might be for the compiler to notice that if an impl is defined on a more restricted type than the trait then either additional impl(s) are needed or the implementation is invalid?

@nikomatsakis
Copy link
Contributor

cc me

@pnkfelix
Copy link
Member

assigning P-high, Milestone 1.

@nikomatsakis
Copy link
Contributor

this looks related to #5527 etc

@pnkfelix pnkfelix added this to the 1.0 milestone May 15, 2014
@pcwalton
Copy link
Contributor

Nominating for closing as a dupe of #2687. If this is not a dupe, then it must be P-backcompat-lang.

@nikomatsakis
Copy link
Contributor

I am not sure this is a dup of #2687 -- that is specific to bounds on methods.

@brson
Copy link
Contributor

brson commented Jun 19, 2014

1.0, backcompat-lang.

@brson brson removed the I-nominated label Jun 19, 2014
pcwalton added a commit to pcwalton/rust that referenced this issue Jul 3, 2014
This can break code that looked like:

    struct S<T> {
        val: T,
    }
    trait Gettable<T> {
        ...
    }
    impl<T: Copy> Gettable<T> for S<T> {
        ...
    }
    let t: Box<S<String>> = box S {
        val: "one".to_string(),
    };
    let a = t as Box<Gettable<String>>;
    //                        ^ note no `Copy` bound

Change this code to:

    impl<T> Gettable<T> for S<T> {
    //   ^ remove `Copy` bound
        ...
    }

Closes rust-lang#14061.

[breaking-change]
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 13, 2023
Make tt generic over the span data

This also fixes up our delimiter representation in tt, it is no longer optional (we use invisible delims in the same way as before, that is still incorrectly) and we now store two spans instead of one.

These changes should help with adjusting our token map. Though this will probably break proc-macros in some ways, will need to test that for now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

8 participants