Pre-RFC: Unsafe Types

RalfJung · 2016-01-16T09:56:24.518Z

So I guess all this is to say that I found the first few paragraphs of your pre-rfc somewhat misleading, in that it seemed to suggest it solved this problem, rather than simply making it somewhat less likely.

Point taken, I did not think enough about the trouble caused by functionally incorrect dependencies.

If you declare a field as unsafe, and that field is private to some module M, then all code within M is suspect (and, by extension, all code that it can reach is part of your trusted codebase, whether that code is marked safe or not).

This is an over-approximation though. It is perfectly conceivable, and I assume it happens all the time, that safety of some unsafe code relies solely on the safety of its dependency - which, if that dependency is safe, would mean there is nothing to trust.

nikomatsakis · 2016-01-16T10:30:58.758Z

RalfJung:

This is an over-approximation though. It is perfectly conceivable, and I assume it happens all the time, that safety of some unsafe code relies solely on the safety of its dependency - which, if that dependency is safe, would mean there is nothing to trust.

Yes, I agree it is an over-approximation. Sorry if I suggested otherwise.

withoutboats · 2016-01-17T02:49:51.265Z

To me unsafe fields don’t seem different at all from unsafe traits, which Rust already has. In both cases the original declarer identifies that manipulating this thing (whether that’s implementing a trait or assigning to a field) could break an invariant that an unsafe block relies on. So these fields should behave the same way I think - if you declare a field as unsafe, writing to that field requires a unsafe block, but not reading from it, analogous to the fact that implementing Send is unsafe, but not bounding by it.

I also don’t know why pub unsafe fields should inherently be disallowed.

Gankro · 2016-01-17T05:49:34.069Z

Unsafe traits are materially different because they’re handling the problem that generic code can’t trust anything to be implemented correctly. That is, we’re dealing with an open set of implementations, and you can’t expect everyone to do things right (which is the C(++) approach). Unsafe traits are basically overriding this and saying “yes, I really need to trust that my implementors are correct”. Note that this is exceedingly rare; literally Send and Sync are the only stable consumers of this functionality, and that’s because there’s no way to guard against a type being thread-unsafe if you’re explicitly trying to do threaded things.

With unsafe fields, you’re generally only concerned with a closed set of implementations: the code in the same module as the struct. This is more controlled, and more reasonable to expect correct behaviour from. I agree with Niko’s analysis that this doesn’t really solve the problem of unsafe code needing to trust safe code to be implemented correctly.

As a simpler example, we need only look at the wonders of this PR that was just trying to make a style change. It was just trying to clean up an if-statement checking if a pointer had two sentinel values – totally safe – but it was guarding some subsequent unsafe code. An error in the refactoring subsequently lead to segfaults.

The only real guarantee we have around unsafe is that if you completely forgo it you’re good to go. As soon as you start using unsafe, you need to be incredibly careful.

withoutboats · 2016-01-17T19:27:01.962Z

Gankro:

Unsafe traits are materially different because they’re handling the problem that generic code can’t trust anything to be implemented correctly. That is, we’re dealing with an open set of implementations, and you can’t expect everyone to do things right (which is the C(++) approach). Unsafe traits are basically overriding this and saying “yes, I really need to trust that my implementors are correct”. Note that this is exceedingly rare; literally Send and Sync are the only stable consumers of this functionality, and that’s because there’s no way to guard against a type being thread-unsafe if you’re explicitly trying to do threaded things.

With unsafe fields, you’re generally only concerned with a closed set of implementations: the code in the same module as the struct. This is more controlled, and more reasonable to expect correct behaviour from. I agree with Niko’s analysis that this doesn’t really solve the problem of unsafe code needing to trust safe code to be implemented correctly.

The bolded is only true if those fields are private.

But yes, this is the difference - unsafe traits can be implemented outside of their declaring module whereas unsafe fields usually would not be able to be assigned to outside of theirs. However to me that only introduces a difference in degree of urgency, and not a difference in the worthiness of the idea.

Gankro · 2016-01-17T19:52:44.504Z

This is an accurate assessment if you consider

The only reason why Safe Rust can be memory safe with threading, without all code being 100% thread-safe

vs

A self-imposed lint for failing to realize that some fields are trusted by unsafe code

to merely be a matter of urgency, and not worthiness. I disagree. There is clearly a qualitative difference here. Unsafe traits made Rust as we know it possible. Unsafe types/fields can be completely replaced by getters/setters at the cost of some ergonomics. In fact, that’s already the case today: we have NonZero, which is exactly an unsafe type.

withoutboats · 2016-01-17T20:06:06.980Z

Gankro:

Unsafe traits made Rust as we know it possible.

Sorry, but this seems very exaggerated to me. What makes Send and Sync work are their OIBIT-nature - that they are implemented automatically for most types and explicitly not implemented for certain types (and that this non-implementation carries through to types that have fields of that type). It being unsafe to explicitly implement them is a very valuable roadblock for errors, but it doesn’t play a fundamental role.

Gankro · 2016-01-17T20:14:38.652Z

The important part is that safe code can’t explicitly implement it. This allows unsafe code to trust the implementation to be correct. Autoderiving is of course important for making this at all tolerable. However all of the following objects would have to be magical langitems if unsafe code couldn’t either:

src/liballoc/arc.rs:unsafe impl<T: ?Sized + Sync + Send> Send for Arc<T> {}
src/liballoc/arc.rs:unsafe impl<T: ?Sized + Sync + Send> Sync for Arc<T> {}
src/liballoc/arc.rs:unsafe impl<T: ?Sized + Sync + Send> Send for Weak<T> {}
src/liballoc/arc.rs:unsafe impl<T: ?Sized + Sync + Send> Sync for Weak<T> {}
src/liballoc/arc.rs:unsafe impl<T: ?Sized + Sync + Send> Send for ArcInner<T> {}
src/liballoc/arc.rs:unsafe impl<T: ?Sized + Sync + Send> Sync for ArcInner<T> {}
src/libcollections/btree/node.rs:unsafe impl<K: Sync, V: Sync> Sync for MoveTraversalImpl<K, V> {}
src/libcollections/btree/node.rs:unsafe impl<K: Send, V: Send> Send for MoveTraversalImpl<K, V> {}
src/libcollections/linked_list.rs:unsafe impl<T: Send> Send for Rawlink<T> {}
src/libcollections/linked_list.rs:unsafe impl<T: Sync> Sync for Rawlink<T> {}
src/libcollections/string.rs:unsafe impl<'a> Sync for Drain<'a> {}
src/libcollections/string.rs:unsafe impl<'a> Send for Drain<'a> {}
src/libcollections/vec.rs:unsafe impl<T: Send> Send for IntoIter<T> {}
src/libcollections/vec.rs:unsafe impl<T: Sync> Sync for IntoIter<T> {}
src/libcollections/vec.rs:unsafe impl<'a, T: Sync> Sync for Drain<'a, T> {}
src/libcollections/vec.rs:unsafe impl<'a, T: Send> Send for Drain<'a, T> {}
src/libcollections/vec_deque.rs:unsafe impl<'a, T: Sync> Sync for Drain<'a, T> {}
src/libcollections/vec_deque.rs:unsafe impl<'a, T: Send> Send for Drain<'a, T> {}
src/libcore/array.rs:unsafe impl<T, A: Unsize<[T]>> FixedSizeArray<T> for A {
src/libcore/cell.rs:unsafe impl<T> Send for Cell<T> where T: Send {}
src/libcore/cell.rs:unsafe impl<T: ?Sized> Send for RefCell<T> where T: Send {}
src/libcore/marker.rs:    unsafe impl<'a, T: Sync + ?Sized> Send for &'a T {}
src/libcore/marker.rs:    unsafe impl<'a, T: Send + ?Sized> Send for &'a mut T {}
src/libcore/nonzero.rs:unsafe impl<T:?Sized> Zeroable for *const T {}
src/libcore/nonzero.rs:unsafe impl<T:?Sized> Zeroable for *mut T {}
src/libcore/nonzero.rs:unsafe impl Zeroable for isize {}
src/libcore/nonzero.rs:unsafe impl Zeroable for usize {}
src/libcore/nonzero.rs:unsafe impl Zeroable for i8 {}
src/libcore/nonzero.rs:unsafe impl Zeroable for u8 {}
src/libcore/nonzero.rs:unsafe impl Zeroable for i16 {}
src/libcore/nonzero.rs:unsafe impl Zeroable for u16 {}
src/libcore/nonzero.rs:unsafe impl Zeroable for i32 {}
src/libcore/nonzero.rs:unsafe impl Zeroable for u32 {}
src/libcore/nonzero.rs:unsafe impl Zeroable for i64 {}
src/libcore/nonzero.rs:unsafe impl Zeroable for u64 {}
src/libcore/ptr.rs:unsafe impl<T: Send + ?Sized> Send for Unique<T> { }
src/libcore/ptr.rs:unsafe impl<T: Sync + ?Sized> Sync for Unique<T> { }
src/libcore/raw.rs:unsafe impl<T> Repr<Slice<T>> for [T] {}
src/libcore/raw.rs:unsafe impl Repr<Slice<u8>> for str {}
src/libcore/slice.rs:unsafe impl<'a, T: Sync> Sync for Iter<'a, T> {}
src/libcore/slice.rs:unsafe impl<'a, T: Sync> Send for Iter<'a, T> {}
src/libcore/slice.rs:unsafe impl<'a, T: Sync> Sync for IterMut<'a, T> {}
src/libcore/slice.rs:unsafe impl<'a, T: Send> Send for IterMut<'a, T> {}
src/libcore/str/pattern.rs:unsafe impl<'a, C: CharEq> Searcher<'a> for CharEqSearcher<'a, C> {
src/libcore/str/pattern.rs:unsafe impl<'a, C: CharEq> ReverseSearcher<'a> for CharEqSearcher<'a, C> {
src/libcore/str/pattern.rs:unsafe impl<'a> Searcher<'a> for CharSearcher<'a> {
src/libcore/str/pattern.rs:unsafe impl<'a> ReverseSearcher<'a> for CharSearcher<'a> {
src/libcore/str/pattern.rs:unsafe impl<'a, 'b> Searcher<'a> for CharSliceSearcher<'a, 'b> {
src/libcore/str/pattern.rs:unsafe impl<'a, 'b> ReverseSearcher<'a> for CharSliceSearcher<'a, 'b> {
src/libcore/str/pattern.rs:unsafe impl<'a, F> Searcher<'a> for CharPredicateSearcher<'a, F>
src/libcore/str/pattern.rs:unsafe impl<'a, F> ReverseSearcher<'a> for CharPredicateSearcher<'a, F>
src/libcore/str/pattern.rs:unsafe impl<'a, 'b> Searcher<'a> for StrSearcher<'a, 'b> {
src/libcore/str/pattern.rs:unsafe impl<'a, 'b> ReverseSearcher<'a> for StrSearcher<'a, 'b> {
src/libcore/sync/atomic.rs:unsafe impl Sync for AtomicBool {}
src/libcore/sync/atomic.rs:unsafe impl Sync for AtomicIsize {}
src/libcore/sync/atomic.rs:unsafe impl Sync for AtomicUsize {}
src/libcore/sync/atomic.rs:unsafe impl<T> Send for AtomicPtr<T> {}
src/libcore/sync/atomic.rs:unsafe impl<T> Sync for AtomicPtr<T> {}
src/librustc/diagnostics.rs:unsafe impl<T> Sync for NotThreadSafe<T> {}
src/librustc_trans/back/msvc/registry.rs:unsafe impl Sync for RegistryKey {}
src/librustc_trans/back/msvc/registry.rs:unsafe impl Send for RegistryKey {}
src/librustc_trans/back/write.rs:unsafe impl Send for ModuleConfig { }
src/librustc_trans/trans/mod.rs:unsafe impl Send for ModuleTranslation { }
src/librustc_trans/trans/mod.rs:unsafe impl Sync for ModuleTranslation { }
src/libstd/collections/hash/table.rs:unsafe impl<K: Send, V: Send> Send for RawTable<K, V> {}
src/libstd/collections/hash/table.rs:unsafe impl<K: Sync, V: Sync> Sync for RawTable<K, V> {}
src/libstd/collections/hash/table.rs:unsafe impl<'a, K: Sync, V: Sync> Sync for Iter<'a, K, V> {}
src/libstd/collections/hash/table.rs:unsafe impl<'a, K: Sync, V: Sync> Send for Iter<'a, K, V> {}
src/libstd/collections/hash/table.rs:unsafe impl<'a, K: Sync, V: Sync> Sync for IterMut<'a, K, V> {}
src/libstd/collections/hash/table.rs:unsafe impl<'a, K: Send, V: Send> Send for IterMut<'a, K, V> {}
src/libstd/collections/hash/table.rs:unsafe impl<K: Sync, V: Sync> Sync for IntoIter<K, V> {}
src/libstd/collections/hash/table.rs:unsafe impl<K: Send, V: Send> Send for IntoIter<K, V> {}
src/libstd/collections/hash/table.rs:unsafe impl<'a, K: Sync, V: Sync> Sync for Drain<'a, K, V> {}
src/libstd/collections/hash/table.rs:unsafe impl<'a, K: Send, V: Send> Send for Drain<'a, K, V> {}
src/libstd/io/lazy.rs:unsafe impl<T> Sync for Lazy<T> {}
src/libstd/sync/mpsc/blocking.rs:unsafe impl Send for Inner {}
src/libstd/sync/mpsc/blocking.rs:unsafe impl Sync for Inner {}
src/libstd/sync/mpsc/mod.rs:unsafe impl<T: Send> Send for Receiver<T> { }
src/libstd/sync/mpsc/mod.rs:unsafe impl<T: Send> Send for Sender<T> { }
src/libstd/sync/mpsc/mod.rs:unsafe impl<T: Send> Send for SyncSender<T> {}
src/libstd/sync/mpsc/mpsc_queue.rs:unsafe impl<T: Send> Send for Queue<T> { }
src/libstd/sync/mpsc/mpsc_queue.rs:unsafe impl<T: Send> Sync for Queue<T> { }
src/libstd/sync/mpsc/spsc_queue.rs:unsafe impl<T: Send> Send for Queue<T> { }
src/libstd/sync/mpsc/spsc_queue.rs:unsafe impl<T: Send> Sync for Queue<T> { }
src/libstd/sync/mpsc/sync.rs:unsafe impl<T: Send> Send for Packet<T> { }
src/libstd/sync/mpsc/sync.rs:unsafe impl<T: Send> Sync for Packet<T> { }
src/libstd/sync/mpsc/sync.rs:unsafe impl<T: Send> Send for State<T> {}
src/libstd/sync/mpsc/sync.rs:unsafe impl Send for Node {}
src/libstd/sync/mutex.rs:unsafe impl<T: ?Sized + Send> Send for Mutex<T> { }
src/libstd/sync/mutex.rs:unsafe impl<T: ?Sized + Send> Sync for Mutex<T> { }
src/libstd/sync/mutex.rs:unsafe impl Sync for Dummy {}
src/libstd/sync/mutex.rs:    unsafe impl<T: Send> Send for Packet<T> {}
src/libstd/sync/mutex.rs:    unsafe impl<T> Sync for Packet<T> {}
src/libstd/sync/rwlock.rs:unsafe impl<T: ?Sized + Send + Sync> Send for RwLock<T> {}
src/libstd/sync/rwlock.rs:unsafe impl<T: ?Sized + Send + Sync> Sync for RwLock<T> {}
src/libstd/sync/rwlock.rs:unsafe impl Sync for Dummy {}
src/libstd/sys/common/mutex.rs:unsafe impl Sync for Mutex {}
src/libstd/sys/common/net.rs:unsafe impl Sync for LookupHost {}
src/libstd/sys/common/net.rs:unsafe impl Send for LookupHost {}
src/libstd/sys/common/poison.rs:unsafe impl Send for Flag {}
src/libstd/sys/common/poison.rs:unsafe impl Sync for Flag {}
src/libstd/sys/common/remutex.rs:unsafe impl<T: Send> Send for ReentrantMutex<T> {}
src/libstd/sys/common/remutex.rs:unsafe impl<T: Send> Sync for ReentrantMutex<T> {}
src/libstd/sys/unix/condvar.rs:unsafe impl Send for Condvar {}
src/libstd/sys/unix/condvar.rs:unsafe impl Sync for Condvar {}
src/libstd/sys/unix/fs.rs:unsafe impl Send for Dir {}
src/libstd/sys/unix/fs.rs:unsafe impl Sync for Dir {}
src/libstd/sys/unix/mutex.rs:unsafe impl Send for Mutex {}
src/libstd/sys/unix/mutex.rs:unsafe impl Sync for Mutex {}
src/libstd/sys/unix/mutex.rs:unsafe impl Send for ReentrantMutex {}
src/libstd/sys/unix/mutex.rs:unsafe impl Sync for ReentrantMutex {}
src/libstd/sys/unix/rwlock.rs:unsafe impl Send for RWLock {}
src/libstd/sys/unix/rwlock.rs:unsafe impl Sync for RWLock {}
src/libstd/sys/unix/thread.rs:unsafe impl Send for Thread {}
src/libstd/sys/unix/thread.rs:unsafe impl Sync for Thread {}
src/libstd/sys/windows/condvar.rs:unsafe impl Send for Condvar {}
src/libstd/sys/windows/condvar.rs:unsafe impl Sync for Condvar {}
src/libstd/sys/windows/fs.rs:unsafe impl Send for FindNextFileHandle {}
src/libstd/sys/windows/fs.rs:unsafe impl Sync for FindNextFileHandle {}
src/libstd/sys/windows/handle.rs:unsafe impl Send for RawHandle {}
src/libstd/sys/windows/handle.rs:unsafe impl Sync for RawHandle {}
src/libstd/sys/windows/mutex.rs:unsafe impl Send for Mutex {}
src/libstd/sys/windows/mutex.rs:unsafe impl Sync for Mutex {}
src/libstd/sys/windows/mutex.rs:unsafe impl Send for ReentrantMutex {}
src/libstd/sys/windows/mutex.rs:unsafe impl Sync for ReentrantMutex {}
src/libstd/sys/windows/rwlock.rs:unsafe impl Send for RWLock {}
src/libstd/sys/windows/rwlock.rs:unsafe impl Sync for RWLock {}
src/libstd/thread/local.rs:    unsafe impl<T> ::marker::Sync for Key<T> { }
src/libstd/thread/local.rs:    unsafe impl<T> ::marker::Sync for Key<T> { }
src/libstd/thread/mod.rs:unsafe impl<T: Send> Send for Packet<T> {}
src/libstd/thread/mod.rs:unsafe impl<T: Sync> Sync for Packet<T> {}
src/libstd/thread/scoped_tls.rs:    unsafe impl<T> ::marker::Sync for KeyInner<T> { }
src/libstd/thread/scoped_tls.rs:    unsafe impl<T> marker::Sync for KeyInner<T> { }

And it would be impossible for safe threading primitives to be efficiently implemented outside of std (e.g. crossbeam), or for any unsafe code outside std to declare that it is indeed thread-safe (e.g. contain-rs).

withoutboats · 2016-01-17T20:21:27.373Z

Gankro:

The important part is that safe code can’t explicitly implement it. This allows unsafe code to trust the implementation to be correct.

Of course that is important. But how is it different from the fact that unsafe code has to trust the value of the len field?

I think we must be miscommunicating enormously though if you think I’m suggesting that Send and Sync should have required lang items to implement.

Gankro · 2016-01-17T20:40:33.495Z

The distinction is entirely in the matter of open vs closed implementations. Generic code (and higher-order code) has to operate under the suspicion that the implementations it consumes are horribly busted. Concrete code doesn’t.

This is analogous to the difference between giving your best friend a key to your house (concrete, closed trust), and giving everyone you meet a key to your house (generic, open trust). Yes, your best friend can be an idiot and mess everything up, but it’s probably not something worth worrying about too much. Trusting everyone in the world to do the right thing, however, is how we end up with the dangers of C(++).

Note that the Pre-RFC doesn’t propose exposing pub-unsafe fields (which I believe @reem had at least one use-case for as an ergonomics thing). I’m focusing only on the “using it for private stuff” aspect, which is the only motivation given.

withoutboats · 2016-01-17T21:21:29.820Z

Well I think we have the same understanding of the matter, at least, and are just disagreeing over a subjective measure of the importance of the difference. I don’t see a difference in kind (only a difference in degree) between trusting that the vec module is upholding its invariants and trusting that all of the Rust code I depend on is upholding its invariants. We both agree that unsafe traits are a bigger deal than unsafe fields, but we disagree about how much bigger, coming to consensus on that won’t really impact this RFC at all.

RalfJung · 2016-01-19T20:47:29.939Z

I’m sorry I have to get into this discussion, even if it is somewhat off-topic

I absolutely agree with @Gankro here, there is a fundamental difference between “trusting the rest of the universe” and “trusting your own implementation”. This difference is pretty clear when it comes to proving stuff.

Right now, when I want to prove Vec correct - where correct means “no safe clients can cause crashes or data races” - then I will consider the entire module Vec is defined in. I assume the rest of the world to be some (safe!) Rust program that does just about whatever it wants with my module. I can now prove that Vec is correct, because I know all the code that ever touches len, and can verify that every such access maintains the invariant. This is possible right now even though there is no such thing as an unsafe field, because len is private.

Now, let’s try to prove that spawn is correct, and lets assume we look at the entire libstd. Again we assume that the rest of the world (i.e., everything outside libstd), calling us, is some safe Rust program. Because it is a safe program, I know it does not contain any implementation of Send or Sync. I can thus prove that there will be no data-races, since I know all the types in libstd actually are thread-safe. (Really, rather than considering the entire libstd at once, one will first try to figure out what Send and Sync actually concretely promise to the rest of the world, and then prove this locally for every implementation, while relying on it in every consumer.)

Now, if you take away unsafe traits - there is just no way to have spawn. Note that we crucially relied above on the fact that the safe Rust program calling us does not implement Send or Sync. Without unsafe traits, we cannot make such an assumption. We would either have to remove spawn from libstd, or give up on our goal of having a “correct” libstd in the sense that safe code can cause no havoc (in other words, we’d be in C++).

So: Unsafe traits are needed to fulfill Rust’s promise of safety with respect to an arbitrary, safe client. Unsafe fields are not.

nikomatsakis · 2016-01-28T02:02:50.856Z

I’ve started to feel pretty strongly that we ought to have unsafe fields (not types). This is based on some recent experience trying to better document unsafety in rayon. Basically, I am trying to be systematic about labeling the assumptions that (e.g.) an unsafe fn requires of its caller as well as, in the case of unsafe blocks, labelling the assumptions within. To this end, I found it pretty helpful to have the compiler guiding me to the places I needed to check (and document) assumptions.

One thing I noticed though was that modifications to unsafe fields, which might violate the invariants I was trying to label, were not caught (duh) and hence it was easy to overlook them. This, combined with the fact that it provides an obvious hook for documentation (“this field is unsafe because you must maintain invariant blah”), seems like an overall win in my book.

oli-obk · 2016-01-28T08:33:17.273Z

Unsafe fields could be (mostly) emulated without compiler support by a newtype with a private value and a setter and a getter which are both unsafe. So as a first step to evaluate the usefulness, we could simply add an unstable UnsafeWrapper type to libstd.

Of course this doesn’t give the full guarantees of an unsafe field, since you can swap two fields of type UnsafeWrapper without any problems, but it should catch most usages.

Playground

RalfJung · 2016-01-28T08:53:45.478Z

Niko, I’m not sure if you followed https://github.com/rust-lang/rfcs/issues/381#issuecomment-174651558 which is where a lot of discussion has been happening recently.

nikomatsakis · 2016-01-28T09:18:34.460Z

Thanks for the tip.

Centril · 2017-07-29T07:12:01.300Z

What is the relation of this Pre-RFC and dependent types? Can’t that be used instead? If not - what effect would it have in conjunction with full dependent types?

RalfJung · 2017-08-02T23:14:53.972Z

I… don’t see any relation to be honest. Or are you suggesting that we embed an interactive theorem prover in Rust and have people prove that their unsafe code upholds the invariants they specified for their type? That seems… rather far-fetched, to say the least.

Ixrec · 2017-08-02T23:28:37.169Z

I believe unsafe types/fields and dependent types are totally unrelated in terms of their syntax, semantics and implementation. But there is some overlap in the use cases these features target (both might help with the len <= capacity invariant, for instance), which I’m guessing is what @Centril was getting at.

See also https://github.com/rust-lang/rfcs/issues/381#issuecomment-175120933 in which @Gankro says “Really, it seems to me that we all just want dependent types and/or design-by-contract invariants/constraints. All this unsafe field/type stuff is just a horribly poor version of those features.”

system · 2019-03-25T08:25:40.005Z

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.