mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #23859 - pnkfelix:fsk-lesser-box, r=nikomatsakis

Browse Source 
Disallow writing through mutable pointers stored in non-mut Box.

Fix #14270 

The fix works by making `cmt::freely_aliasable` result more fine-grained.

Instead of encoding the aliasability (i.e. whether the cmt is uniquely writable or not) as an option, now pass back an enum indicating either: 1. freely-aliasable (thus not uniquely-writable), 2. non-aliasable (thus uniquely writable), or 3. unique but immutable (and thus not uniquely writable, according to proposal from issue #14270.)

This is all of course a giant hack that will hopefully go away with an eventually removal of special treatment of `Box<T>` (aka `ty_unique`) from the compiler.
This commit is contained in:
Manish Goregaokar 2015-03-31 09:04:38 +05:30
commit b4457fb8a2
6 changed files with 230 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/librustc/middle/expr_use_visitor.rs
View File

@ -885,6 +885,11 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> {
}
}
// When this returns true, it means that the expression *is* a
// method-call (i.e. via the operator-overload). This true result
// also implies that walk_overloaded_operator already took care of
// recursively processing the input arguments, and thus the caller
// should not do so.
fn walk_overloaded_operator(&mut self,
expr: &ast::Expr,
receiver: &ast::Expr,

169
src/librustc/middle/mem_categorization.rs
View File

@ -71,6 +71,8 @@ pub use self::Note::*;
pub use self::deref_kind::*;
pub use self::categorization::*;
use self::Aliasability::*;
use middle::check_const;
use middle::def;
use middle::region;
@ -295,23 +297,29 @@ pub trait Typer<'tcx> : ty::ClosureTyper<'tcx> {
impl MutabilityCategory {
pub fn from_mutbl(m: ast::Mutability) -> MutabilityCategory {
match m {
let ret = match m {
MutImmutable => McImmutable,
MutMutable => McDeclared
}
};
debug!("MutabilityCategory::{}({:?}) => {:?}",
"from_mutbl", m, ret);
ret
}
pub fn from_borrow_kind(borrow_kind: ty::BorrowKind) -> MutabilityCategory {
match borrow_kind {
let ret = match borrow_kind {
ty::ImmBorrow => McImmutable,
ty::UniqueImmBorrow => McImmutable,
ty::MutBorrow => McDeclared,
}
};
debug!("MutabilityCategory::{}({:?}) => {:?}",
"from_borrow_kind", borrow_kind, ret);
ret
}
pub fn from_pointer_kind(base_mutbl: MutabilityCategory,
ptr: PointerKind) -> MutabilityCategory {
match ptr {
fn from_pointer_kind(base_mutbl: MutabilityCategory,
ptr: PointerKind) -> MutabilityCategory {
let ret = match ptr {
Unique => {
base_mutbl.inherit()
}
@ -321,11 +329,14 @@ impl MutabilityCategory {
UnsafePtr(m) => {
MutabilityCategory::from_mutbl(m)
}
}
};
debug!("MutabilityCategory::{}({:?}, {:?}) => {:?}",
"from_pointer_kind", base_mutbl, ptr, ret);
ret
}
fn from_local(tcx: &ty::ctxt, id: ast::NodeId) -> MutabilityCategory {
match tcx.map.get(id) {
let ret = match tcx.map.get(id) {
ast_map::NodeLocal(p) | ast_map::NodeArg(p) => match p.node {
ast::PatIdent(bind_mode, _, _) => {
if bind_mode == ast::BindByValue(ast::MutMutable) {
@ -337,30 +348,39 @@ impl MutabilityCategory {
_ => tcx.sess.span_bug(p.span, "expected identifier pattern")
},
_ => tcx.sess.span_bug(tcx.map.span(id), "expected identifier pattern")
}
};
debug!("MutabilityCategory::{}(tcx, id={:?}) => {:?}",
"from_local", id, ret);
ret
}
pub fn inherit(&self) -> MutabilityCategory {
match *self {
let ret = match *self {
McImmutable => McImmutable,
McDeclared => McInherited,
McInherited => McInherited,
}
};
debug!("{:?}.inherit() => {:?}", self, ret);
ret
}
pub fn is_mutable(&self) -> bool {
match *self {
let ret = match *self {
McImmutable => false,
McInherited => true,
McDeclared => true,
}
};
debug!("{:?}.is_mutable() => {:?}", self, ret);
ret
}
pub fn is_immutable(&self) -> bool {
match *self {
let ret = match *self {
McImmutable => true,
McDeclared | McInherited => false
}
};
debug!("{:?}.is_immutable() => {:?}", self, ret);
ret
}
pub fn to_user_str(&self) -> &'static str {
@ -733,7 +753,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
}
};
Ok(Rc::new(cmt_result))
let ret = Rc::new(cmt_result);
debug!("cat_upvar ret={}", ret.repr(self.tcx()));
Ok(ret)
}
fn env_deref(&self,
@ -794,14 +816,18 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
McDeclared | McInherited => { }
}
cmt_ {
let ret = cmt_ {
id: id,
span: span,
cat: cat_deref(Rc::new(cmt_result), 0, env_ptr),
mutbl: deref_mutbl,
ty: var_ty,
note: NoteClosureEnv(upvar_id)
}
};
debug!("env_deref ret {}", ret.repr(self.tcx()));
ret
}
pub fn cat_rvalue_node(&self,
@ -831,7 +857,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
}
}
};
self.cat_rvalue(id, span, re, expr_ty)
let ret = self.cat_rvalue(id, span, re, expr_ty);
debug!("cat_rvalue_node ret {}", ret.repr(self.tcx()));
ret
}
pub fn cat_rvalue(&self,
@ -839,14 +867,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
span: Span,
temp_scope: ty::Region,
expr_ty: Ty<'tcx>) -> cmt<'tcx> {
Rc::new(cmt_ {
let ret = Rc::new(cmt_ {
id:cmt_id,
span:span,
cat:cat_rvalue(temp_scope),
mutbl:McDeclared,
ty:expr_ty,
note: NoteNone
})
});
debug!("cat_rvalue ret {}", ret.repr(self.tcx()));
ret
}
pub fn cat_field<N:ast_node>(&self,
@ -855,14 +885,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
f_name: ast::Name,
f_ty: Ty<'tcx>)
-> cmt<'tcx> {
Rc::new(cmt_ {
let ret = Rc::new(cmt_ {
id: node.id(),
span: node.span(),
mutbl: base_cmt.mutbl.inherit(),
cat: cat_interior(base_cmt, InteriorField(NamedField(f_name))),
ty: f_ty,
note: NoteNone
})
});
debug!("cat_field ret {}", ret.repr(self.tcx()));
ret
}
pub fn cat_tup_field<N:ast_node>(&self,
@ -871,14 +903,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
f_idx: usize,
f_ty: Ty<'tcx>)
-> cmt<'tcx> {
Rc::new(cmt_ {
let ret = Rc::new(cmt_ {
id: node.id(),
span: node.span(),
mutbl: base_cmt.mutbl.inherit(),
cat: cat_interior(base_cmt, InteriorField(PositionalField(f_idx))),
ty: f_ty,
note: NoteNone
})
});
debug!("cat_tup_field ret {}", ret.repr(self.tcx()));
ret
}
fn cat_deref<N:ast_node>(&self,
@ -913,10 +947,14 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
};
let base_cmt_ty = base_cmt.ty;
match ty::deref(base_cmt_ty, true) {
Some(mt) => self.cat_deref_common(node, base_cmt, deref_cnt,
Some(mt) => {
let ret = self.cat_deref_common(node, base_cmt, deref_cnt,
mt.ty,
deref_context,
/* implicit: */ false),
/* implicit: */ false);
debug!("cat_deref ret {}", ret.repr(self.tcx()));
ret
}
None => {
debug!("Explicit deref of non-derefable type: {}",
base_cmt_ty.repr(self.tcx()));
@ -954,14 +992,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
(base_cmt.mutbl.inherit(), cat_interior(base_cmt, interior))
}
};
Ok(Rc::new(cmt_ {
let ret = Rc::new(cmt_ {
id: node.id(),
span: node.span(),
cat: cat,
mutbl: m,
ty: deref_ty,
note: NoteNone
}))
});
debug!("cat_deref_common ret {}", ret.repr(self.tcx()));
Ok(ret)
}
pub fn cat_index<N:ast_node>(&self,
@ -1009,8 +1049,10 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
};
let m = base_cmt.mutbl.inherit();
return Ok(interior(elt, base_cmt.clone(), base_cmt.ty,
m, context, element_ty));
let ret = interior(elt, base_cmt.clone(), base_cmt.ty,
m, context, element_ty);
debug!("cat_index ret {}", ret.repr(self.tcx()));
return Ok(ret);
fn interior<'tcx, N: ast_node>(elt: &N,
of_cmt: cmt<'tcx>,
@ -1039,14 +1081,14 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
context: InteriorOffsetKind)
-> McResult<cmt<'tcx>>
{
match try!(deref_kind(base_cmt.ty, Some(context))) {
let ret = match try!(deref_kind(base_cmt.ty, Some(context))) {
deref_ptr(ptr) => {
// for unique ptrs, we inherit mutability from the
// owning reference.
let m = MutabilityCategory::from_pointer_kind(base_cmt.mutbl, ptr);
// the deref is explicit in the resulting cmt
Ok(Rc::new(cmt_ {
Rc::new(cmt_ {
id:elt.id(),
span:elt.span(),
cat:cat_deref(base_cmt.clone(), 0, ptr),
@ -1056,13 +1098,15 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
None => self.tcx().sess.bug("Found non-derefable type")
},
note: NoteNone
}))
})
}
deref_interior(_) => {
Ok(base_cmt)
base_cmt
}
}
};
debug!("deref_vec ret {}", ret.repr(self.tcx()));
Ok(ret)
}
/// Given a pattern P like: `[_, ..Q, _]`, where `vec_cmt` is the cmt for `P`, `slice_pat` is
@ -1112,14 +1156,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
interior_ty: Ty<'tcx>,
interior: InteriorKind)
-> cmt<'tcx> {
Rc::new(cmt_ {
let ret = Rc::new(cmt_ {
id: node.id(),
span: node.span(),
mutbl: base_cmt.mutbl.inherit(),
cat: cat_interior(base_cmt, interior),
ty: interior_ty,
note: NoteNone
})
});
debug!("cat_imm_interior ret={}", ret.repr(self.tcx()));
ret
}
pub fn cat_downcast<N:ast_node>(&self,
@ -1128,14 +1174,16 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
downcast_ty: Ty<'tcx>,
variant_did: ast::DefId)
-> cmt<'tcx> {
Rc::new(cmt_ {
let ret = Rc::new(cmt_ {
id: node.id(),
span: node.span(),
mutbl: base_cmt.mutbl.inherit(),
cat: cat_downcast(base_cmt, variant_did),
ty: downcast_ty,
note: NoteNone
})
});
debug!("cat_downcast ret={}", ret.repr(self.tcx()));
ret
}
pub fn cat_pattern<F>(&self, cmt: cmt<'tcx>, pat: &ast::Pat, mut op: F) -> McResult<()>
@ -1341,17 +1389,25 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
}
}
#[derive(Copy)]
#[derive(Copy, Clone, Debug)]
pub enum InteriorSafety {
InteriorUnsafe,
InteriorSafe
}
#[derive(Copy)]
#[derive(Clone, Debug)]
pub enum Aliasability {
FreelyAliasable(AliasableReason),
NonAliasable,
ImmutableUnique(Box<Aliasability>),
}
#[derive(Copy, Clone, Debug)]
pub enum AliasableReason {
AliasableBorrowed,
AliasableClosure(ast::NodeId), // Aliasable due to capture Fn closure env
AliasableOther,
UnaliasableImmutable, // Created as needed upon seeing ImmutableUnique
AliasableStatic(InteriorSafety),
AliasableStaticMut(InteriorSafety),
}
@ -1380,9 +1436,9 @@ impl<'tcx> cmt_<'tcx> {
}
}
/// Returns `Some(_)` if this lvalue represents a freely aliasable pointer type.
/// Returns `FreelyAliasable(_)` if this lvalue represents a freely aliasable pointer type.
pub fn freely_aliasable(&self, ctxt: &ty::ctxt<'tcx>)
-> Option<AliasableReason> {
-> Aliasability {
// Maybe non-obvious: copied upvars can only be considered
// non-aliasable in once closures, since any other kind can be
// aliased and eventually recused.
@ -1393,17 +1449,27 @@ impl<'tcx> cmt_<'tcx> {
cat_deref(ref b, _, BorrowedPtr(ty::UniqueImmBorrow, _)) |
cat_deref(ref b, _, Implicit(ty::UniqueImmBorrow, _)) |
cat_downcast(ref b, _) |
cat_deref(ref b, _, Unique) |
cat_interior(ref b, _) => {
// Aliasability depends on base cmt
b.freely_aliasable(ctxt)
}
cat_deref(ref b, _, Unique) => {
let sub = b.freely_aliasable(ctxt);
if b.mutbl.is_mutable() {
// Aliasability depends on base cmt alone
sub
} else {
// Do not allow mutation through an immutable box.
ImmutableUnique(Box::new(sub))
}
}
cat_rvalue(..) |
cat_local(..) |
cat_upvar(..) |
cat_deref(_, _, UnsafePtr(..)) => { // yes, it's aliasable, but...
None
NonAliasable
}
cat_static_item(..) => {
@ -1414,17 +1480,18 @@ impl<'tcx> cmt_<'tcx> {
};
if self.mutbl.is_mutable() {
Some(AliasableStaticMut(int_safe))
FreelyAliasable(AliasableStaticMut(int_safe))
} else {
Some(AliasableStatic(int_safe))
FreelyAliasable(AliasableStatic(int_safe))
}
}
cat_deref(ref base, _, BorrowedPtr(ty::ImmBorrow, _)) |
cat_deref(ref base, _, Implicit(ty::ImmBorrow, _)) => {
match base.cat {
cat_upvar(Upvar{ id, .. }) => Some(AliasableClosure(id.closure_expr_id)),
_ => Some(AliasableBorrowed)
cat_upvar(Upvar{ id, .. }) =>
FreelyAliasable(AliasableClosure(id.closure_expr_id)),
_ => FreelyAliasable(AliasableBorrowed)
}
}
}

13
src/librustc_borrowck/borrowck/check_loans.rs
View File

@ -943,13 +943,20 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
cmt: mc::cmt<'tcx>)
-> bool {
match cmt.freely_aliasable(this.tcx()) {
None => {
mc::Aliasability::NonAliasable => {
return true;
}
Some(mc::AliasableStaticMut(..)) => {
mc::Aliasability::FreelyAliasable(mc::AliasableStaticMut(..)) => {
return true;
}
Some(cause) => {
mc::Aliasability::ImmutableUnique(_) => {
this.bccx.report_aliasability_violation(
span,
MutabilityViolation,
mc::AliasableReason::UnaliasableImmutable);
return false;
}
mc::Aliasability::FreelyAliasable(cause) => {
this.bccx.report_aliasability_violation(
span,
MutabilityViolation,

33
src/librustc_borrowck/borrowck/gather_loans/mod.rs
View File

@ -151,10 +151,11 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for GatherLoanCtxt<'a, 'tcx> {
assignee_cmt: mc::cmt<'tcx>,
mode: euv::MutateMode)
{
debug!("mutate(assignment_id={}, assignee_cmt={})",
assignment_id, assignee_cmt.repr(self.tcx()));
let opt_lp = opt_loan_path(&assignee_cmt);
debug!("mutate(assignment_id={}, assignee_cmt={}) opt_lp={:?}",
assignment_id, assignee_cmt.repr(self.tcx()), opt_lp);
match opt_loan_path(&assignee_cmt) {
match opt_lp {
Some(lp) => {
gather_moves::gather_assignment(self.bccx, &self.move_data,
assignment_id, assignment_span,
@ -181,12 +182,16 @@ fn check_aliasability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
req_kind: ty::BorrowKind)
-> Result<(),()> {
match (cmt.freely_aliasable(bccx.tcx), req_kind) {
(None, _) => {
let aliasability = cmt.freely_aliasable(bccx.tcx);
debug!("check_aliasability aliasability={:?} req_kind={:?}",
aliasability, req_kind);
match (aliasability, req_kind) {
(mc::Aliasability::NonAliasable, _) => {
/* Uniquely accessible path -- OK for `&` and `&mut` */
Ok(())
}
(Some(mc::AliasableStatic(safety)), ty::ImmBorrow) => {
(mc::Aliasability::FreelyAliasable(mc::AliasableStatic(safety)), ty::ImmBorrow) => {
// Borrow of an immutable static item:
match safety {
mc::InteriorUnsafe => {
@ -202,13 +207,20 @@ fn check_aliasability<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
}
}
}
(Some(mc::AliasableStaticMut(..)), _) => {
(mc::Aliasability::FreelyAliasable(mc::AliasableStaticMut(..)), _) => {
// Even touching a static mut is considered unsafe. We assume the
// user knows what they're doing in these cases.
Ok(())
}
(Some(alias_cause), ty::UniqueImmBorrow) |
(Some(alias_cause), ty::MutBorrow) => {
(mc::Aliasability::ImmutableUnique(_), ty::MutBorrow) => {
bccx.report_aliasability_violation(
borrow_span,
BorrowViolation(loan_cause),
mc::AliasableReason::UnaliasableImmutable);
Err(())
}
(mc::Aliasability::FreelyAliasable(alias_cause), ty::UniqueImmBorrow) |
(mc::Aliasability::FreelyAliasable(alias_cause), ty::MutBorrow) => {
bccx.report_aliasability_violation(
borrow_span,
BorrowViolation(loan_cause),
@ -376,7 +388,8 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> {
req_kind: ty::BorrowKind)
-> Result<(),()> {
//! Implements the M-* rules in README.md.
debug!("check_mutability(cause={:?} cmt={} req_kind={:?}",
cause, cmt.repr(bccx.tcx), req_kind);
match req_kind {
ty::UniqueImmBorrow | ty::ImmBorrow => {
match cmt.mutbl {

6
src/librustc_borrowck/borrowck/mod.rs
View File

@ -844,6 +844,12 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
&format!("{} in an aliasable location",
prefix));
}
mc::AliasableReason::UnaliasableImmutable => {
self.tcx.sess.span_err(
span,
&format!("{} in an immutable container",
prefix));
}
mc::AliasableClosure(id) => {
self.tcx.sess.span_err(span,
&format!("{} in a captured outer \

76
src/test/compile-fail/borrowck-issue-14498.rs
View File

@ -9,26 +9,48 @@
// except according to those terms.
// This tests that we can't modify Box<&mut T> contents while they
// are borrowed.
// are borrowed (#14498).
//
// Also includes tests of the errors reported when the Box in question
// is immutable (#14270).
#![feature(box_syntax)]
struct A { a: isize }
struct B<'a> { a: Box<&'a mut isize> }
fn indirect_write_to_imm_box() {
let mut x: isize = 1;
let y: Box<_> = box &mut x;
let p = &y;
***p = 2; //~ ERROR cannot assign to data in an immutable container
drop(p);
}
fn borrow_in_var_from_var() {
let mut x: isize = 1;
let mut y: Box<_> = box &mut x;
let p = &y;
let q = &***p;
**y = 2; //~ ERROR cannot assign to `**y` because it is borrowed
drop(p);
drop(q);
}
fn borrow_in_var_from_var_via_imm_box() {
let mut x: isize = 1;
let y: Box<_> = box &mut x;
let p = &y;
let q = &***p;
**y = 2; //~ ERROR cannot assign to `**y` because it is borrowed
//~^ ERROR cannot assign to data in an immutable container
drop(p);
drop(q);
}
fn borrow_in_var_from_field() {
let mut x = A { a: 1 };
let y: Box<_> = box &mut x.a;
let mut y: Box<_> = box &mut x.a;
let p = &y;
let q = &***p;
**y = 2; //~ ERROR cannot assign to `**y` because it is borrowed
@ -36,19 +58,41 @@ fn borrow_in_var_from_field() {
drop(q);
}
fn borrow_in_var_from_field_via_imm_box() {
let mut x = A { a: 1 };
let y: Box<_> = box &mut x.a;
let p = &y;
let q = &***p;
**y = 2; //~ ERROR cannot assign to `**y` because it is borrowed
//~^ ERROR cannot assign to data in an immutable container
drop(p);
drop(q);
}
fn borrow_in_field_from_var() {
let mut x: isize = 1;
let mut y = B { a: box &mut x };
let p = &y.a;
let q = &***p;
**y.a = 2; //~ ERROR cannot assign to `**y.a` because it is borrowed
drop(p);
drop(q);
}
fn borrow_in_field_from_var_via_imm_box() {
let mut x: isize = 1;
let y = B { a: box &mut x };
let p = &y.a;
let q = &***p;
**y.a = 2; //~ ERROR cannot assign to `**y.a` because it is borrowed
//~^ ERROR cannot assign to data in an immutable container
drop(p);
drop(q);
}
fn borrow_in_field_from_field() {
let mut x = A { a: 1 };
let y = B { a: box &mut x.a };
let mut y = B { a: box &mut x.a };
let p = &y.a;
let q = &***p;
**y.a = 2; //~ ERROR cannot assign to `**y.a` because it is borrowed
@ -56,9 +100,25 @@ fn borrow_in_field_from_field() {
drop(q);
}
fn main() {
borrow_in_var_from_var();
borrow_in_var_from_field();
borrow_in_field_from_var();
borrow_in_field_from_field();
fn borrow_in_field_from_field_via_imm_box() {
let mut x = A { a: 1 };
let y = B { a: box &mut x.a };
let p = &y.a;
let q = &***p;
**y.a = 2; //~ ERROR cannot assign to `**y.a` because it is borrowed
//~^ ERROR cannot assign to data in an immutable container
drop(p);
drop(q);
}
fn main() {
indirect_write_to_imm_box();
borrow_in_var_from_var();
borrow_in_var_from_var_via_imm_box();
borrow_in_var_from_field();
borrow_in_var_from_field_via_imm_box();
borrow_in_field_from_var();
borrow_in_field_from_var_via_imm_box();
borrow_in_field_from_field();
borrow_in_field_from_field_via_imm_box();
}