The Unsafe<T> reference problem

[arielb1]

Currently, in Rust, undefined behaviour can only appear within an unsafe block, with one exception: one can take a reference to the body of an Unsafe block when unsafe code had borrowed the inside of the block, as in the following Rust code:

use std::ty::Unsafe;
use std::kinds::marker::InvariantType;

fn mess_up_memory() {
  let mut result : Vec<Box<uint>> = vec![];
  for _ in range(0, 10000u) { result.push(box 0xcccccccc); }
}

fn main() {
    // Create an unsafe object
    let x = Unsafe { value: box 1u, marker1: InvariantType };
    let mut_alias : &mut Box<uint> = unsafe { &mut *x.get() };

    // The behaviour up to here was completely defined.
    // The unsafe introduced no undefined behaviour
    // Now, lets introduce some UB
    let alias : &Box<uint> = &x.value;
    // Now we have aliased & and &mut pointers
    // Have fun
    let internal : &uint = &**alias;
    println!("internal={}", internal);
    *mut_alias = box 2;
    mess_up_memory();
    println!("internal={}", internal);
}

This was confused with some issues with statics in rust-lang/rfcs#177 and #14862, but, as shown here, has nothing to do with them.

This is not an RFC because I don't have a plan to fix this (and would prefer to first talk about the issue here).

[lilyball]

I'm not sure this issue as described is really a problem. The bug here is that the unsafe {} block is buggy. Yes, the actual crash would occur in code outside of the unsafe {} block, but that can happen via a great number of ways, without Unsafe. The easiest way is to construct a & ref that refers to invalid memory.

fn bad_code<'a>() -> &'a str {
    let x = "allocated string".to_string();
    unsafe { mem::transmute(x.as_slice()) }
}

The above returns a &str that refers to now-deallocated memory, so any safe code that uses this reference may crash.

[arielb1]

Your example is different - the drop glue invokes the unsafe function free on an aliased pointer - which is already UB before safe code can touch the invalid pointer (Of course, because UB, the crash can occur anytime not strictly before the return). Of course the buggy unsafe block is the transmute - but the unsafety occurs within an unsafe block but this is irrelevant.

[lilyball]

@arielb1 Seems to me that's roughly equivalent to your code with Unsafe though. Yes, the &mut you create does not alias an already-exsiting &T at the time it's created. But it does alias a publicly-accessible value field on the Unsafe value (which is the field used to subsequently construct the &T). Remember that "aliasing" doesn't mean "contains a pointer value equal to a pointer value stored elsewhere" but really means "allows for accessing a value that is accessible via a separate path". The real bug here is that the unsafe code has returned a &mut T reference that does not borrow the Unsafe value. And thus, the bug still lies within the unsafe block.

Of course, Unsafe exposes a *mut T instead of a &mut T precisely because it's producing the mutability from a &Unsafe<T> reference, which means it cannot know whether it's already aliased, and thus it's not safe to create the &mut T. That's why it's up to a wrapping type to provide the non-aliasing guarantees sufficient to allow valid creation of a &mut T.

Ultimately, the public value field is an issue, but as it stands today, it's not the fundamental safety-breaking issue that you seem to be suggesting. Just like a *mut T pointer, using Unsafe requires restricting access to the value such that all code that can use the value can make sure to maintain the appropriate invariants for safe access.

Of course, if static items change as suggested in rust-lang/rfcs#177 such that you can then say static FOO: Unsafe<uint> then this does become a more serious issue. As long as static mut exists it's still not as bad as I think that RFC's proposal is (because we would still not expect, and would in fact discourage, normal Rust users from using Unsafe) but if static mut is dropped in favor of telling people to use static FOO: Unsafe<uint> then it becomes much more important to fix the public value field issue.

[arielb1]

@kballard

Not really. I agree that the bug is within the unsafe block - however this is exactly in the same manner that the bug in a hypothetical method within std::vec that conjures a bogus vector and pushes something into it (causing UB) is not within push but within the method that conjured the vector.

I'm quite sure that privacy should not be related to undefined behaviour, except in the trivial manner that it can reduce the amount of code that needs to be checked. Keeping this in mind, you can notice that RefCell ALSO creates an &mut T alias to an not-mutably-borrowed Unsafe (which is externally safe because the Unsafe is behind a private field and the code accessing it is careful not to create an additional reference).

I never said that this was an unsoundness in Rust because it isn't - only an ugly point. Of course the issue would become less hidden if mut statics are removed - but it is still an issue even with them.

By the way, I currently think the best way to fix this would be to make accessing the fields of an Unsafe<T> unsafe and will probably write an RFC tomorrow.

[lilyball]

We already had an RFC for unsafe fields: rust-lang/rfcs#80. It was closed largely because it was felt that Unsafe already provided the desired functionality. But obviously, as it turns out, Unsafe itself actually needs this qualifier for its own use. Given that, I am in favor of you writing a new RFC (just be sure to read the old one first).

The alternative is to somehow mark a field as being usable in a struct literal expression, but private everywhere else. I can't think of a good way to denote that, though, or any use for that outside of Unsafe<T>, whereas an unsafe-qualified field is usable elsewhere.

[arielb1]

This is now RFC PR rust-lang/rfcs#182

[steveklabnik]

Closing in favor of the RFC thread.