MIR-borrowck: add permisson checks to fn access_lvalue

src/librustc/ty/util.rs

@@ -10,8 +10,9 @@
 
 //! misc. type-system utilities too small to deserve their own file
 
+use hir::def::Def;
 use hir::def_id::{DefId, LOCAL_CRATE};
-use hir::map::DefPathData;
+use hir::map::{DefPathData, Node};
 use hir;
 use ich::NodeIdHashingMode;
 use middle::const_val::ConstVal;
@@ -648,6 +649,26 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             _ => bug!(),
         }
     }
+
+    /// Check if the node pointed to by def_id is a mutable static item
+    pub fn is_static_mut(&self, def_id: DefId) -> bool {
+        if let Some(node) = self.hir.get_if_local(def_id) {
+            match node {
+                Node::NodeItem(&hir::Item {
+                    node: hir::ItemStatic(_, hir::MutMutable, _), ..
+                }) => true,
+                Node::NodeForeignItem(&hir::ForeignItem {
+                    node: hir::ForeignItemStatic(_, mutbl), ..
+                }) => mutbl,
+                _ => false
+            }
+        } else {
+            match self.describe_def(def_id) {
+                Some(Def::Static(_, mutbl)) => mutbl,
+                _ => false
+            }
+        }
+    }
 }
 
 pub struct TypeIdHasher<'a, 'gcx: 'a+'tcx, 'tcx: 'a, W> {

src/librustc_mir/borrow_check.rs

@@ -10,6 +10,7 @@
 
 //! This query borrow-checks the MIR to (further) ensure it is not broken.
 
+use rustc::hir;
 use rustc::hir::def_id::{DefId};
 use rustc::infer::{InferCtxt};
 use rustc::ty::{self, TyCtxt, ParamEnv};
@@ -447,9 +448,12 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx>
                      lvalue_span: (&Lvalue<'tcx>, Span),
                      kind: (ShallowOrDeep, ReadOrWrite),
                      flow_state: &InProgress<'b, 'gcx, 'tcx>) {
-        // FIXME: also need to check permissions (e.g. reject mut
-        // borrow of immutable ref, moves through non-`Box`-ref)
+
         let (sd, rw) = kind;
+
+        // Check permissions
+        self.check_access_permissions(lvalue_span, rw);
+
         self.each_borrow_involving_path(
             context, (sd, lvalue_span.0), flow_state, |this, _index, borrow, common_prefix| {
                 match (rw, borrow.kind) {
@@ -861,6 +865,154 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx>
             }
         }
     }
+
+    /// Check the permissions for the given lvalue and read or write kind
+    fn check_access_permissions(&self, (lvalue, span): (&Lvalue<'tcx>, Span), kind: ReadOrWrite) {
+        match kind {
+            Write(WriteKind::MutableBorrow(BorrowKind::Unique)) => {
+                if let Err(_lvalue_err) = self.is_unique(lvalue) {
+                    span_bug!(span, "&unique borrow for `{}` should not fail",
+                        self.describe_lvalue(lvalue));
+                }
+            },
+            Write(WriteKind::MutableBorrow(BorrowKind::Mut)) => {
+                if let Err(lvalue_err) = self.is_mutable(lvalue) {
+                    let mut err = self.tcx.cannot_borrow_path_as_mutable(span,
+                        &format!("immutable item `{}`",
+                                  self.describe_lvalue(lvalue)),
+                        Origin::Mir);
+                    err.span_label(span, "cannot borrow as mutable");
+
+                    if lvalue != lvalue_err {
+                        err.note(&format!("Value not mutable causing this error: `{}`",
+                            self.describe_lvalue(lvalue_err)));
+                    }
+
+                    err.emit();
+                }
+            },
+            _ => {}// Access authorized
+        }
+    }
+
+    /// Can this value be written or borrowed mutably
+    fn is_mutable<'d>(&self, lvalue: &'d Lvalue<'tcx>) -> Result<(), &'d Lvalue<'tcx>> {
+        match *lvalue {
+            Lvalue::Local(local) => {
+                let local = &self.mir.local_decls[local];
+                match local.mutability {
+                    Mutability::Not => Err(lvalue),
+                    Mutability::Mut => Ok(())
+                }
+            },
+            Lvalue::Static(ref static_) => {
+                if !self.tcx.is_static_mut(static_.def_id) {
+                    Err(lvalue)
+                } else {
+                    Ok(())
+                }
+            },
+            Lvalue::Projection(ref proj) => {
+                match proj.elem {
+                    ProjectionElem::Deref => {
+                        let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
+
+                        // `Box<T>` owns its content, so mutable if its location is mutable
+                        if base_ty.is_box() {
+                            return self.is_mutable(&proj.base);
+                        }
+
+                        // Otherwise we check the kind of deref to decide
+                        match base_ty.sty {
+                            ty::TyRef(_, tnm) => {
+                                match tnm.mutbl {
+                                    // Shared borrowed data is never mutable
+                                    hir::MutImmutable => Err(lvalue),
+                                    // Mutably borrowed data is mutable, but only if we have a
+                                    // unique path to the `&mut`
+                                    hir::MutMutable => self.is_unique(&proj.base),
+                                }
+                            },
+                            ty::TyRawPtr(tnm) => {
+                                match tnm.mutbl {
+                                    // `*const` raw pointers are not mutable
+                                    hir::MutImmutable => Err(lvalue),
+                                    // `*mut` raw pointers are always mutable, regardless of context
+                                    // The users have to check by themselve.
+                                    hir::MutMutable => Ok(()),
+                                }
+                            },
+                            // Deref should only be for reference, pointers or boxes
+                            _ => bug!("Deref of unexpected type: {:?}", base_ty)
+                        }
+                    },
+                    // All other projections are owned by their base path, so mutable if
+                    // base path is mutable
+                    ProjectionElem::Field(..) |
+                    ProjectionElem::Index(..) |
+                    ProjectionElem::ConstantIndex{..} |
+                    ProjectionElem::Subslice{..} |
+                    ProjectionElem::Downcast(..) =>
+                        self.is_mutable(&proj.base)
+                }
+            }
+        }
+    }
+
+    /// Does this lvalue have a unique path
+    fn is_unique<'d>(&self, lvalue: &'d Lvalue<'tcx>) -> Result<(), &'d Lvalue<'tcx>> {
+        match *lvalue {
+            Lvalue::Local(..) => {
+                // Local variables are unique
+                Ok(())
+            },
+            Lvalue::Static(..) => {
+                // Static variables are not
+                Err(lvalue)
+            },
+            Lvalue::Projection(ref proj) => {
+                match proj.elem {
+                    ProjectionElem::Deref => {
+                        let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
+
+                        // `Box<T>` referent is unique if box is a unique spot
+                        if base_ty.is_box() {
+                            return self.is_unique(&proj.base);
+                        }
+
+                        // Otherwise we check the kind of deref to decide
+                        match base_ty.sty {
+                            ty::TyRef(_, tnm) => {
+                                match tnm.mutbl {
+                                    // lvalue represent an aliased location
+                                    hir::MutImmutable => Err(lvalue),
+                                    // `&mut T` is as unique as the context in which it is found
+                                    hir::MutMutable => self.is_unique(&proj.base),
+                                }
+                            },
+                            ty::TyRawPtr(tnm) => {
+                                match tnm.mutbl {
+                                    // `*mut` can be aliased, but we leave it to user
+                                    hir::MutMutable => Ok(()),
+                                    // `*const` is treated the same as `*mut`
+                                    hir::MutImmutable => Ok(()),
+                                }
+                            },
+                            // Deref should only be for reference, pointers or boxes
+                            _ => bug!("Deref of unexpected type: {:?}", base_ty)
+                        }
+                    },
+                    // Other projections are unique if the base is unique
+                    ProjectionElem::Field(..) |
+                    ProjectionElem::Index(..) |
+                    ProjectionElem::ConstantIndex{..} |
+                    ProjectionElem::Subslice{..} |
+                    ProjectionElem::Downcast(..) =>
+                        self.is_unique(&proj.base)
+                }
+            }
+        }
+    }
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]

src/librustc_mir/transform/check_unsafety.rs

@@ -14,9 +14,8 @@ use rustc_data_structures::indexed_vec::IndexVec;
 use rustc::ty::maps::Providers;
 use rustc::ty::{self, TyCtxt};
 use rustc::hir;
-use rustc::hir::def::Def;
 use rustc::hir::def_id::DefId;
-use rustc::hir::map::{DefPathData, Node};
+use rustc::hir::map::DefPathData;
 use rustc::lint::builtin::{SAFE_EXTERN_STATICS, UNUSED_UNSAFE};
 use rustc::mir::*;
 use rustc::mir::visit::{LvalueContext, Visitor};
@@ -189,7 +188,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
                 // locals are safe
             }
             &Lvalue::Static(box Static { def_id, ty: _ }) => {
-                if self.is_static_mut(def_id) {
+                if self.tcx.is_static_mut(def_id) {
                     self.require_unsafe("use of mutable static");
                 } else if self.tcx.is_foreign_item(def_id) {
                     let source_info = self.source_info;
@@ -208,24 +207,6 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
 }
 
 impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
-    fn is_static_mut(&self, def_id: DefId) -> bool {
-        if let Some(node) = self.tcx.hir.get_if_local(def_id) {
-            match node {
-                Node::NodeItem(&hir::Item {
-                    node: hir::ItemStatic(_, hir::MutMutable, _), ..
-                }) => true,
-                Node::NodeForeignItem(&hir::ForeignItem {
-                    node: hir::ForeignItemStatic(_, mutbl), ..
-                }) => mutbl,
-                _ => false
-            }
-        } else {
-            match self.tcx.describe_def(def_id) {
-                Some(Def::Static(_, mutbl)) => mutbl,
-                _ => false
-            }
-        }
-    }
     fn require_unsafe(&mut self,
                       description: &'static str)
     {

src/test/compile-fail/E0596.rs

@@ -8,7 +8,12 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// revisions: ast mir
+//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir
+
 fn main() {
     let x = 1;
-    let y = &mut x; //~ ERROR E0596
+    let y = &mut x; //[ast]~ ERROR [E0596]
+                    //[mir]~^ ERROR (Ast) [E0596]
+                    //[mir]~| ERROR (Mir) [E0596]
 }

src/test/compile-fail/borrowck/borrowck-access-permissions.rs

@@ -0,0 +1,76 @@
+// Copyright 2016 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.
+
+// revisions: ast mir
+//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir
+
+static static_x : i32 = 1;
+static mut static_x_mut : i32 = 1;
+
+fn main() {
+    let x = 1;
+    let mut x_mut = 1;
+
+    { // borrow of local
+        let _y1 = &mut x; //[ast]~ ERROR [E0596]
+                          //[mir]~^ ERROR (Ast) [E0596]
+                          //[mir]~| ERROR (Mir) [E0596]
+        let _y2 = &mut x_mut; // No error
+    }
+
+    { // borrow of static
+        let _y1 = &mut static_x; //[ast]~ ERROR [E0596]
+                                 //[mir]~^ ERROR (Ast) [E0596]
+                                 //[mir]~| ERROR (Mir) [E0596]
+        unsafe { let _y2 = &mut static_x_mut; } // No error
+    }
+
+    { // borrow of deref to box
+        let box_x = Box::new(1);
+        let mut box_x_mut = Box::new(1);
+
+        let _y1 = &mut *box_x; //[ast]~ ERROR [E0596]
+                              //[mir]~^ ERROR (Ast) [E0596]
+                              //[mir]~| ERROR (Mir) [E0596]
+        let _y2 = &mut *box_x_mut; // No error
+    }
+
+    { // borrow of deref to reference
+        let ref_x = &x;
+        let ref_x_mut = &mut x_mut;
+
+        let _y1 = &mut *ref_x; //[ast]~ ERROR [E0596]
+                              //[mir]~^ ERROR (Ast) [E0596]
+                              //[mir]~| ERROR (Mir) [E0596]
+        let _y2 = &mut *ref_x_mut; // No error
+    }
+
+    { // borrow of deref to pointer
+        let ptr_x : *const _ = &x;
+        let ptr_mut_x : *mut _ = &mut x_mut;
+
+        unsafe {
+            let _y1 = &mut *ptr_x; //[ast]~ ERROR [E0596]
+                                  //[mir]~^ ERROR (Ast) [E0596]
+                                  //[mir]~| ERROR (Mir) [E0596]
+            let _y2 = &mut *ptr_mut_x; // No error
+        }
+    }
+
+    { // borrowing mutably through an immutable reference
+        struct Foo<'a> { f: &'a mut i32, g: &'a i32 };
+        let mut foo = Foo { f: &mut x_mut, g: &x };
+        let foo_ref = &foo;
+        let _y = &mut *foo_ref.f; //[ast]~ ERROR [E0389]
+                                 //[mir]~^ ERROR (Ast) [E0389]
+                                 //[mir]~| ERROR (Mir) [E0596]
+                                 // FIXME: Wrong error in MIR
+    }
+}