Auto-coercing from &T to *T is unsafe

[erickt]

I'm working on a *libc::c_char wrapper library, and I wanted to make sure that people didn't accidentally grab the inner pointer, deallocate the CStr, then pass the pointer to a function. I thought this would be a great place to use regions, so instead of returning *libc::c_char I returned &'self libc::c_char. This works in all cases except for when we auto-coerce from a &T to a *T.

You can find a demonstration of this problem in this gist. While CStr is full of unsafe code, I believe the interface is safe. The error is demonstrated in the bar/baz functions. bar properly reports that the lifetime of the &'self libc::c_char does not live long enough, but baz doesn't mention a problem at all.

I can think of a couple options to fixing this:

• Force users to cast a &T to a *T. This at least adds a small roadblock force the end user to think about this cast, but it won't save you from shooting yourself in the foot if you so choose.
• Allow raw pointers to live in a region. I don't really care about the coercion, it's just that I want to use rust's borrow checker to prevent me from holding a reference to a dead pointer. If I want to shoot myself in the foot, I can always use cast::transmute() to forget the region.
• Change all the libc functions to take &libc::c_char instead of *libc::c_char. This is a nice short term bandaid for the stdlib, but doesn't really help out end users writing C bindings that don't know about this behavior.

[thestinger]

I don't really see how it could be unsafe, because you can already create arbitrary dangling raw pointers from integers. Any interface taking a raw pointer and then dereferencing it at some point has to be marked unsafe.

The CStr2 interface is unsafe because you can pass a dangling pointer in to new_with_ownership and it will call free on it.

[erickt]

@thestinger: I've updated my gist to make the CStr2 simpler and safe because it's not dealing with a dangling pointer. None of the functions are unsafe, but we still end up passing baz a dangling pointer.

Yes, the unsafe pointer is unsafe, and you need to be in an unsafe block to dereference it or pass it to a C function, but I don't think that means we should turn off all the safety features of Rust when in an unsafe block. If we can prevent this dangling pointer being created, I feel we should even in unsafe blocks. I'd rather force people to use cast::transmute() when they really know better than the borrow checker.

So my vote right now is that we:

• Allow regions to be placed on a *T pointer.
• Remove the ability to auto-coerce from &'a T to *T, but instead auto-coerce to *'a T.
• Force users to use cast::transmute() to forget the region in *'a T.

edit: I submitted this early and incomplete by accident.

[nikomatsakis]

I don't quite see the point of *'a T. How is this different from &'a T? It feels like the proper thing is to wrap the various C functions you want to expose so that they take &'a T.

Also, I think that the current "rules" regarding the lifetime of temporaries are too short, which makes it easier to footgun in examples like this one. All temporaries ought to live at least as love as the current statement, I think, what's unclear to me is if we should possibly infer a longer lifetime. (#3511)

[erickt]

@nikomatsakis: We still have a footgun even if we make temporaries live to the end of the current statement. For example, if we take my gist and set main to:

fn main() {
    let dangling_ptr: *u8 = CStr2::new(5).as_ref();
    std::io::println(fmt!("here: %?", dangling_ptr));
}

Even if that temporary lives as long as the statement we'll still have a dangling pointer because of the auto-coercion from a &int to a *int.

As far as I know, there are two main differences between a &T and a *T. First, if it's safe do dereference the pointer. Or expressed another way, *T may be null, whereas &T should never be null. Second, if a pointer survives past the destruction of value it's pointing at. I don't see much value of that second property. Sure, once we pass that pointer to C a whole host of dumb things can happen to it, but up until we do pass it to C I'd like as much assistance as possible from Rust to protect me from myself, and only allow me to shoot myself in the foot if I explicitly ask to convert a *'a T to a *T with a cast::transmute() or something.

[nikomatsakis]

@erickt I agree it's not foolproof, but I don't consider that example to be the same level of footgun.

Still, having a lifetime associated with unsafe pointers might have value.

[emberian]

@erickt do you think this can be closed?

[treeman]

Triage bump.

@erickt status?

[thestinger]

It's not actually memory unsafe by Rust's definition. It's part of the normal unsafe model where creating raw pointers is allowed in safe code, since they can become dangling for various reasons at after creation anyway. A language change that's not strictly a bug fix would need to go through an RFC now.

[erickt]

@thestinger is right in remembering why I thought this ticket was closed a long time ago. This unsafety just needs to be well documented.