Auto merge of #24392 - seanmonstar:lint-transmute-mut, r=alexcrichton

The [UnsafeCell documentation says it is undefined behavior](http://doc.rust-lang.org/nightly/std/cell/struct.UnsafeCell.html), so people shouldn't do it.

This happened to catch one case in libstd that was doing this, and I switched that to use an UnsafeCell internally.

Closes #13146

Author: bors

src/librustc_lint/builtin.rs

@@ -2121,6 +2121,72 @@ impl LintPass for InvalidNoMangleItems {
     }
 }
 
+#[derive(Clone, Copy)]
+pub struct MutableTransmutes;
+
+declare_lint! {
+    MUTABLE_TRANSMUTES,
+    Deny,
+    "mutating transmuted &mut T from &T may cause undefined behavior"
+}
+
+impl LintPass for MutableTransmutes {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(MUTABLE_TRANSMUTES)
+    }
+
+    fn check_expr(&mut self, cx: &Context, expr: &ast::Expr) {
+        use syntax::ast::DefId;
+        use syntax::abi::RustIntrinsic;
+        let msg = "mutating transmuted &mut T from &T may cause undefined behavior,\
+                   consider instead using an UnsafeCell";
+        match get_transmute_from_to(cx, expr) {
+            Some((&ty::ty_rptr(_, from_mt), &ty::ty_rptr(_, to_mt))) => {
+                if to_mt.mutbl == ast::Mutability::MutMutable
+                    && from_mt.mutbl == ast::Mutability::MutImmutable {
+                    cx.span_lint(MUTABLE_TRANSMUTES, expr.span, msg);
+                }
+            }
+            _ => ()
+        }
+
+        fn get_transmute_from_to<'a, 'tcx>(cx: &Context<'a, 'tcx>, expr: &ast::Expr)
+            -> Option<(&'tcx ty::sty<'tcx>, &'tcx ty::sty<'tcx>)> {
+            match expr.node {
+                ast::ExprPath(..) => (),
+                _ => return None
+            }
+            if let DefFn(did, _) = ty::resolve_expr(cx.tcx, expr) {
+                if !def_id_is_transmute(cx, did) {
+                    return None;
+                }
+                let typ = ty::node_id_to_type(cx.tcx, expr.id);
+                match typ.sty {
+                    ty::ty_bare_fn(_, ref bare_fn) if bare_fn.abi == RustIntrinsic => {
+                        if let ty::FnConverging(to) = bare_fn.sig.0.output {
+                            let from = bare_fn.sig.0.inputs[0];
+                            return Some((&from.sty, &to.sty));
+                        }
+                    },
+                    _ => ()
+                }
+            }
+            None
+        }
+
+        fn def_id_is_transmute(cx: &Context, def_id: DefId) -> bool {
+            match ty::lookup_item_type(cx.tcx, def_id).ty.sty {
+                ty::ty_bare_fn(_, ref bfty) if bfty.abi == RustIntrinsic => (),
+                _ => return false
+            }
+            ty::with_path(cx.tcx, def_id, |path| match path.last() {
+                Some(ref last) => last.name().as_str() == "transmute",
+                _ => false
+            })
+        }
+    }
+}
+
 /// Forbids using the `#[feature(...)]` attribute
 #[derive(Copy, Clone)]
 pub struct UnstableFeatures;

src/librustc_lint/lib.rs

@@ -109,6 +109,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
                  InvalidNoMangleItems,
                  PluginAsLibrary,
                  DropWithReprExtern,
+                 MutableTransmutes,
                  );
 
     add_builtin_with_new!(sess,

src/libstd/sync/mpsc/select.rs

@@ -58,7 +58,7 @@
 
 use core::prelude::*;
 
-use core::cell::Cell;
+use core::cell::{Cell, UnsafeCell};
 use core::marker;
 use core::mem;
 use core::ptr;
@@ -70,9 +70,13 @@ use sync::mpsc::blocking::{self, SignalToken};
 /// The "receiver set" of the select interface. This structure is used to manage
 /// a set of receivers which are being selected over.
 pub struct Select {
+    inner: UnsafeCell<SelectInner>,
+    next_id: Cell<usize>,
+}
+
+struct SelectInner {
     head: *mut Handle<'static, ()>,
     tail: *mut Handle<'static, ()>,
-    next_id: Cell<usize>,
 }
 
 impl !marker::Send for Select {}
@@ -84,7 +88,7 @@ pub struct Handle<'rx, T:Send+'rx> {
     /// The ID of this handle, used to compare against the return value of
     /// `Select::wait()`
     id: usize,
-    selector: &'rx Select,
+    selector: *mut SelectInner,
     next: *mut Handle<'static, ()>,
     prev: *mut Handle<'static, ()>,
     added: bool,
@@ -127,8 +131,10 @@ impl Select {
     /// ```
     pub fn new() -> Select {
         Select {
-            head: ptr::null_mut(),
-            tail: ptr::null_mut(),
+            inner: UnsafeCell::new(SelectInner {
+                head: ptr::null_mut(),
+                tail: ptr::null_mut(),
+            }),
             next_id: Cell::new(1),
         }
     }
@@ -141,7 +147,7 @@ impl Select {
         self.next_id.set(id + 1);
         Handle {
             id: id,
-            selector: self,
+            selector: self.inner.get(),
             next: ptr::null_mut(),
             prev: ptr::null_mut(),
             added: false,
@@ -250,7 +256,7 @@ impl Select {
         }
     }
 
-    fn iter(&self) -> Packets { Packets { cur: self.head } }
+    fn iter(&self) -> Packets { Packets { cur: unsafe { &*self.inner.get() }.head } }
 }
 
 impl<'rx, T: Send> Handle<'rx, T> {
@@ -271,7 +277,7 @@ impl<'rx, T: Send> Handle<'rx, T> {
     /// while it is added to the `Select` set.
     pub unsafe fn add(&mut self) {
         if self.added { return }
-        let selector: &mut Select = mem::transmute(&*self.selector);
+        let selector = &mut *self.selector;
         let me: *mut Handle<'static, ()> = mem::transmute(&*self);
 
         if selector.head.is_null() {
@@ -292,7 +298,7 @@ impl<'rx, T: Send> Handle<'rx, T> {
     pub unsafe fn remove(&mut self) {
         if !self.added { return }
 
-        let selector: &mut Select = mem::transmute(&*self.selector);
+        let selector = &mut *self.selector;
         let me: *mut Handle<'static, ()> = mem::transmute(&*self);
 
         if self.prev.is_null() {
@@ -317,8 +323,10 @@ impl<'rx, T: Send> Handle<'rx, T> {
 
 impl Drop for Select {
     fn drop(&mut self) {
-        assert!(self.head.is_null());
-        assert!(self.tail.is_null());
+        unsafe {
+            assert!((&*self.inner.get()).head.is_null());
+            assert!((&*self.inner.get()).tail.is_null());
+        }
     }
 }
 

src/test/auxiliary/issue_8401.rs

@@ -21,6 +21,6 @@ impl A for B {}
 fn bar<T>(_: &mut A, _: &T) {}
 
 fn foo<T>(t: &T) {
-    let b = B;
-    bar(unsafe { mem::transmute(&b as &A) }, t)
+    let mut b = B;
+    bar(&mut b as &mut A, t)
 }

src/test/compile-fail/transmute-imut-to-mut.rs

@@ -0,0 +1,20 @@
+// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Tests that transmuting from &T to &mut T is Undefined Behavior.
+
+use std::mem::transmute;
+
+fn main() {
+    let _a: &mut u8 = unsafe { transmute(&1u8) };
+    //~^ ERROR mutating transmuted &mut T from &T may cause undefined behavior
+}
+
+

src/test/run-pass/issue-2718.rs

@@ -170,7 +170,7 @@ pub mod pipes {
             unsafe {
                 if self.p != None {
                     let self_p: &mut Option<*const packet<T>> =
-                        mem::transmute(&self.p);
+                        mem::transmute(&mut self.p);
                     let p = replace(self_p, None);
                     sender_terminate(p.unwrap())
                 }
@@ -199,7 +199,7 @@ pub mod pipes {
             unsafe {
                 if self.p != None {
                     let self_p: &mut Option<*const packet<T>> =
-                        mem::transmute(&self.p);
+                        mem::transmute(&mut self.p);
                     let p = replace(self_p, None);
                     receiver_terminate(p.unwrap())
                 }