Auto merge of #45436 - zilbuz:issue-44837, r=nikomatsakis

MIR-borrowck: add permisson checks to `fn access_lvalue`

WIP : Some FIXME left and some broken tests.

Fix #44837
This commit is contained in:
bors 2017-11-13 23:36:07 +00:00
commit 212d74f1e7
5 changed files with 260 additions and 25 deletions

View File

@ -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> {

View File

@ -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)]

View File

@ -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)
{

View File

@ -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]
}

View File

@ -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
}
}