OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

auto merge of #8797 : nikomatsakis/rust/issue-8625-assign-to-andmut-in-borrowed-loc-2, r=pcwalton

Browse Source 
Fixes for #8625 to prevent assigning to `&mut` in borrowed or aliasable locations. The old code was insufficient in that it failed to catch bizarre cases like `& &mut &mut`. 

r? @pnkfelix
This commit is contained in:
bors 2013-08-27 17:05:46 -07:00
commit 58d6eb5048
16 changed files with 166 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/librustc/middle/astencode.rs
View File

@ -383,7 +383,7 @@ impl tr for ast::def {
ast::def_method(did0.tr(xcx), did1.map(|did1| did1.tr(xcx)))
}
ast::def_self_ty(nid) => { ast::def_self_ty(xcx.tr_id(nid)) }
ast::def_self(nid, i) => { ast::def_self(xcx.tr_id(nid), i) }
ast::def_self(nid) => { ast::def_self(xcx.tr_id(nid)) }
ast::def_mod(did) => { ast::def_mod(did.tr(xcx)) }
ast::def_foreign_mod(did) => { ast::def_foreign_mod(did.tr(xcx)) }
ast::def_static(did, m) => { ast::def_static(did.tr(xcx), m) }

71
src/librustc/middle/borrowck/check_loans.rs
View File

@ -367,7 +367,6 @@ impl<'self> CheckLoanCtxt<'self> {
mc::cat_rvalue(*) |
mc::cat_static_item |
mc::cat_implicit_self |
mc::cat_copied_upvar(*) |
mc::cat_deref(_, _, mc::unsafe_ptr(*)) |
mc::cat_deref(_, _, mc::gc_ptr(*)) |
@ -406,15 +405,7 @@ impl<'self> CheckLoanCtxt<'self> {
mc::cat_deref(b, _, mc::region_ptr(m_mutbl, _)) => {
// Statically prohibit writes to `&mut` when aliasable
match b.freely_aliasable() {
None => {}
Some(cause) => {
this.bccx.report_aliasability_violation(
expr.span,
MutabilityViolation,
cause);
}
}
check_for_aliasability_violation(this, expr, b);
}
mc::cat_deref(_, deref_count, mc::gc_ptr(ast::m_mutbl)) => {
@ -434,6 +425,51 @@ impl<'self> CheckLoanCtxt<'self> {
return true; // no errors reported
}
fn check_for_aliasability_violation(this: &CheckLoanCtxt,
expr: @ast::expr,
cmt: mc::cmt) -> bool {
let mut cmt = cmt;
loop {
match cmt.cat {
mc::cat_deref(b, _, mc::region_ptr(m_mutbl, _)) |
mc::cat_downcast(b) |
mc::cat_stack_upvar(b) |
mc::cat_deref(b, _, mc::uniq_ptr) |
mc::cat_interior(b, _) |
mc::cat_discr(b, _) => {
// Aliasability depends on base cmt
cmt = b;
}
mc::cat_copied_upvar(_) |
mc::cat_rvalue(*) |
mc::cat_local(*) |
mc::cat_arg(_) |
mc::cat_self(*) |
mc::cat_deref(_, _, mc::unsafe_ptr(*)) |
mc::cat_static_item(*) |
mc::cat_deref(_, _, mc::gc_ptr(_)) |
mc::cat_deref(_, _, mc::region_ptr(m_const, _)) |
mc::cat_deref(_, _, mc::region_ptr(m_imm, _)) => {
// Aliasability is independent of base cmt
match cmt.freely_aliasable() {
None => {
return true;
}
Some(cause) => {
this.bccx.report_aliasability_violation(
expr.span,
MutabilityViolation,
cause);
return false;
}
}
}
}
}
}
fn check_for_assignment_to_restricted_or_frozen_location(
this: &CheckLoanCtxt,
expr: @ast::expr,
@ -511,6 +547,12 @@ impl<'self> CheckLoanCtxt<'self> {
// path, and check that the super path was not lent out as
// mutable or immutable (a const loan is ok).
//
// Mutability of a path can be dependent on the super path
// in two ways. First, it might be inherited mutability.
// Second, the pointee of an `&mut` pointer can only be
// mutated if it is found in an unaliased location, so we
// have to check that the owner location is not borrowed.
//
// Note that we are *not* checking for any and all
// restrictions. We are only interested in the pointers
// that the user created, whereas we add restrictions for
@ -528,9 +570,12 @@ impl<'self> CheckLoanCtxt<'self> {
let mut loan_path = loan_path;
loop {
match *loan_path {
// Peel back one layer if `loan_path` has
// inherited mutability
LpExtend(lp_base, mc::McInherited, _) => {
// Peel back one layer if, for `loan_path` to be
// mutable, `lp_base` must be mutable. This occurs
// with inherited mutability and with `&mut`
// pointers.
LpExtend(lp_base, mc::McInherited, _) |
LpExtend(lp_base, _, LpDeref(mc::region_ptr(ast::m_mutbl, _))) => {
loan_path = lp_base;
}

1
src/librustc/middle/borrowck/gather_loans/gather_moves.rs
View File

@ -100,7 +100,6 @@ fn check_is_legal_to_move_from(bccx: @BorrowckCtxt,
cmt0: mc::cmt,
cmt: mc::cmt) -> bool {
match cmt.cat {
mc::cat_implicit_self(*) |
mc::cat_deref(_, _, mc::region_ptr(*)) |
mc::cat_deref(_, _, mc::gc_ptr(*)) |
mc::cat_deref(_, _, mc::unsafe_ptr(*)) => {

3
src/librustc/middle/borrowck/gather_loans/lifetime.rs
View File

@ -68,7 +68,6 @@ impl GuaranteeLifetimeContext {
match cmt.cat {
mc::cat_rvalue(*) |
mc::cat_implicit_self |
mc::cat_copied_upvar(*) | // L-Local
mc::cat_local(*) | // L-Local
mc::cat_arg(*) | // L-Local
@ -301,7 +300,6 @@ impl GuaranteeLifetimeContext {
}
mc::cat_rvalue(*) |
mc::cat_static_item |
mc::cat_implicit_self |
mc::cat_copied_upvar(*) |
mc::cat_deref(*) => {
false
@ -328,7 +326,6 @@ impl GuaranteeLifetimeContext {
mc::cat_rvalue(cleanup_scope_id) => {
ty::re_scope(cleanup_scope_id)
}
mc::cat_implicit_self |
mc::cat_copied_upvar(_) => {
ty::re_scope(self.item_scope_id)
}

13
src/librustc/middle/borrowck/gather_loans/restrictions.rs
View File

@ -101,7 +101,7 @@ impl RestrictionsContext {
self.extend(result, cmt.mutbl, LpInterior(i), restrictions)
}
mc::cat_deref(cmt_base, _, mc::uniq_ptr) => {
mc::cat_deref(cmt_base, _, pk @ mc::uniq_ptr) => {
// R-Deref-Send-Pointer
//
// When we borrow the interior of an owned pointer, we
@ -110,12 +110,11 @@ impl RestrictionsContext {
let result = self.restrict(
cmt_base,
restrictions | RESTR_MUTATE | RESTR_CLAIM);
self.extend(result, cmt.mutbl, LpDeref, restrictions)
self.extend(result, cmt.mutbl, LpDeref(pk), restrictions)
}
mc::cat_copied_upvar(*) | // FIXME(#2152) allow mutation of upvars
mc::cat_static_item(*) |
mc::cat_implicit_self(*) |
mc::cat_deref(_, _, mc::region_ptr(m_imm, _)) |
mc::cat_deref(_, _, mc::gc_ptr(m_imm)) => {
// R-Deref-Imm-Borrowed
@ -129,7 +128,7 @@ impl RestrictionsContext {
Safe
}
mc::cat_deref(cmt_base, _, mc::gc_ptr(m_mutbl)) => {
mc::cat_deref(cmt_base, _, pk @ mc::gc_ptr(m_mutbl)) => {
// R-Deref-Managed-Borrowed
//
// Technically, no restrictions are *necessary* here.
@ -170,14 +169,14 @@ impl RestrictionsContext {
match opt_loan_path(cmt_base) {
None => Safe,
Some(lp_base) => {
let lp = @LpExtend(lp_base, cmt.mutbl, LpDeref);
let lp = @LpExtend(lp_base, cmt.mutbl, LpDeref(pk));
SafeIf(lp, ~[Restriction {loan_path: lp,
set: restrictions}])
}
}
}
mc::cat_deref(cmt_base, _, mc::region_ptr(m_mutbl, _)) => {
mc::cat_deref(cmt_base, _, pk @ mc::region_ptr(m_mutbl, _)) => {
// Because an `&mut` pointer does not inherit its
// mutability, we can only prevent mutation or prevent
// freezing if it is not aliased. Therefore, in such
@ -187,7 +186,7 @@ impl RestrictionsContext {
let result = self.restrict(
cmt_base,
RESTR_ALIAS | RESTR_MUTATE | RESTR_CLAIM);
self.extend(result, cmt.mutbl, LpDeref, restrictions)
self.extend(result, cmt.mutbl, LpDeref(pk), restrictions)
} else {
// R-Deref-Mut-Borrowed-2
Safe

15
src/librustc/middle/borrowck/mod.rs
View File

@ -261,7 +261,7 @@ pub enum LoanPath {
#[deriving(Eq, IterBytes)]
pub enum LoanPathElem {
LpDeref, // `*LV` in doc.rs
LpDeref(mc::PointerKind), // `*LV` in doc.rs
LpInterior(mc::InteriorKind) // `LV.f` in doc.rs
}
@ -284,8 +284,7 @@ pub fn opt_loan_path(cmt: mc::cmt) -> Option<@LoanPath> {
match cmt.cat {
mc::cat_rvalue(*) |
mc::cat_static_item |
mc::cat_copied_upvar(_) |
mc::cat_implicit_self => {
mc::cat_copied_upvar(_) => {
None
}
@ -295,9 +294,9 @@ pub fn opt_loan_path(cmt: mc::cmt) -> Option<@LoanPath> {
Some(@LpVar(id))
}
mc::cat_deref(cmt_base, _, _) => {
mc::cat_deref(cmt_base, _, pk) => {
do opt_loan_path(cmt_base).map_move |lp| {
@LpExtend(lp, cmt.mutbl, LpDeref)
@LpExtend(lp, cmt.mutbl, LpDeref(pk))
}
}
@ -728,7 +727,7 @@ impl BorrowckCtxt {
loan_path: &LoanPath,
out: &mut ~str) {
match *loan_path {
LpExtend(_, _, LpDeref) => {
LpExtend(_, _, LpDeref(_)) => {
out.push_char('(');
self.append_loan_path_to_str(loan_path, out);
out.push_char(')');
@ -776,7 +775,7 @@ impl BorrowckCtxt {
out.push_str("[]");
}
LpExtend(lp_base, _, LpDeref) => {
LpExtend(lp_base, _, LpDeref(_)) => {
out.push_char('*');
self.append_loan_path_to_str(lp_base, out);
}
@ -854,7 +853,7 @@ impl Repr for LoanPath {
fmt!("$(%?)", id)
}
&LpExtend(lp, _, LpDeref) => {
&LpExtend(lp, _, LpDeref(_)) => {
fmt!("%s.*", lp.repr(tcx))
}

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

@ -59,18 +59,17 @@ use syntax::print::pprust;
#[deriving(Eq)]
pub enum categorization {
cat_rvalue(ast::NodeId), // temporary val, argument is its scope
cat_rvalue(ast::NodeId), // temporary val, argument is its scope
cat_static_item,
cat_implicit_self,
cat_copied_upvar(CopiedUpvar), // upvar copied into @fn or ~fn env
cat_stack_upvar(cmt), // by ref upvar from &fn
cat_local(ast::NodeId), // local variable
cat_arg(ast::NodeId), // formal argument
cat_deref(cmt, uint, ptr_kind), // deref of a ptr
cat_local(ast::NodeId), // local variable
cat_arg(ast::NodeId), // formal argument
cat_deref(cmt, uint, PointerKind), // deref of a ptr
cat_interior(cmt, InteriorKind), // something interior: field, tuple, etc
cat_downcast(cmt), // selects a particular enum variant (*)
cat_discr(cmt, ast::NodeId), // match discriminant (see preserve())
cat_self(ast::NodeId), // explicit `self`
cat_discr(cmt, ast::NodeId), // match discriminant (see preserve())
cat_self(ast::NodeId), // explicit `self`
// (*) downcast is only required if the enum has more than one variant
}
@ -82,8 +81,8 @@ pub struct CopiedUpvar {
}
// different kinds of pointers:
#[deriving(Eq)]
pub enum ptr_kind {
#[deriving(Eq, IterBytes)]
pub enum PointerKind {
uniq_ptr,
gc_ptr(ast::mutability),
region_ptr(ast::mutability, ty::Region),
@ -147,7 +146,7 @@ pub type cmt = @cmt_;
// We pun on *T to mean both actual deref of a ptr as well
// as accessing of components:
pub enum deref_kind {
deref_ptr(ptr_kind),
deref_ptr(PointerKind),
deref_interior(InteriorKind),
}
@ -493,17 +492,11 @@ impl mem_categorization_ctxt {
}
}
ast::def_self(self_id, is_implicit) => {
let cat = if is_implicit {
cat_implicit_self
} else {
cat_self(self_id)
};
ast::def_self(self_id) => {
@cmt_ {
id:id,
span:span,
cat:cat,
cat:cat_self(self_id),
mutbl: McImmutable,
ty:expr_ty
}
@ -1016,9 +1009,6 @@ impl mem_categorization_ctxt {
cat_static_item => {
~"static item"
}
cat_implicit_self => {
~"self reference"
}
cat_copied_upvar(_) => {
~"captured outer variable in a heap closure"
}
@ -1121,7 +1111,6 @@ impl cmt_ {
match self.cat {
cat_rvalue(*) |
cat_static_item |
cat_implicit_self |
cat_copied_upvar(*) |
cat_local(*) |
cat_self(*) |
@ -1146,9 +1135,10 @@ impl cmt_ {
}
pub fn freely_aliasable(&self) -> Option<AliasableReason> {
//! True if this lvalue resides in an area that is
//! freely aliasable, meaning that rustc cannot track
//! the alias//es with precision.
/*!
* Returns `Some(_)` if this lvalue represents a freely aliasable
* pointer type.
*/
// Maybe non-obvious: copied upvars can only be considered
// non-aliasable in once closures, since any other kind can be
@ -1166,8 +1156,7 @@ impl cmt_ {
}
cat_copied_upvar(CopiedUpvar {onceness: ast::Many, _}) |
cat_static_item(*) |
cat_implicit_self(*) => {
cat_static_item(*) => {
Some(AliasableOther)
}
@ -1180,12 +1169,12 @@ impl cmt_ {
Some(AliasableBorrowed(m))
}
cat_downcast(b) |
cat_stack_upvar(b) |
cat_deref(b, _, uniq_ptr) |
cat_interior(b, _) |
cat_discr(b, _) => {
b.freely_aliasable()
cat_downcast(*) |
cat_stack_upvar(*) |
cat_deref(_, _, uniq_ptr) |
cat_interior(*) |
cat_discr(*) => {
None
}
}
}
@ -1205,7 +1194,6 @@ impl Repr for categorization {
fn repr(&self, tcx: ty::ctxt) -> ~str {
match *self {
cat_static_item |
cat_implicit_self |
cat_rvalue(*) |
cat_copied_upvar(*) |
cat_local(*) |
@ -1233,7 +1221,7 @@ impl Repr for categorization {
}
}
pub fn ptr_sigil(ptr: ptr_kind) -> ~str {
pub fn ptr_sigil(ptr: PointerKind) -> ~str {
match ptr {
uniq_ptr => ~"~",
gc_ptr(_) => ~"@",

2
src/librustc/middle/moves.rs
View File

@ -228,7 +228,7 @@ pub fn moved_variable_node_id_from_def(def: def) -> Option<NodeId> {
def_binding(nid, _) |
def_arg(nid, _) |
def_local(nid, _) |
def_self(nid, _) => Some(nid),
def_self(nid) => Some(nid),
_ => None
}

9
src/librustc/middle/resolve.rs
View File

@ -125,7 +125,7 @@ pub enum Mutability {
pub enum SelfBinding {
NoSelfBinding,
HasSelfBinding(NodeId, bool /* is implicit */)
HasSelfBinding(NodeId)
}
struct ResolveVisitor {
@ -3771,9 +3771,8 @@ impl Resolver {
NoSelfBinding => {
// Nothing to do.
}
HasSelfBinding(self_node_id, is_implicit) => {
let def_like = dl_def(def_self(self_node_id,
is_implicit));
HasSelfBinding(self_node_id) => {
let def_like = dl_def(def_self(self_node_id));
*function_value_rib.self_binding = Some(def_like);
}
}
@ -3917,7 +3916,7 @@ impl Resolver {
// we only have self ty if it is a non static method
let self_binding = match method.explicit_self.node {
sty_static => { NoSelfBinding }
_ => { HasSelfBinding(method.self_id, false) }
_ => { HasSelfBinding(method.self_id) }
};
self.resolve_function(rib_kind,

2
src/librustc/middle/trans/expr.rs
View File

@ -1066,7 +1066,7 @@ pub fn trans_local_var(bcx: @mut Block, def: ast::def) -> Datum {
ast::def_local(nid, _) | ast::def_binding(nid, _) => {
take_local(bcx, bcx.fcx.lllocals, nid)
}
ast::def_self(nid, _) => {
ast::def_self(nid) => {
let self_info: ValSelfData = match bcx.fcx.llself {
Some(ref self_info) => *self_info,
None => {

2
src/librustc/middle/typeck/check/mod.rs
View File

@ -3093,7 +3093,7 @@ pub fn ty_param_bounds_and_ty_for_def(fcx: @mut FnCtxt,
defn: ast::def)
-> ty_param_bounds_and_ty {
match defn {
ast::def_arg(nid, _) | ast::def_local(nid, _) | ast::def_self(nid, _) |
ast::def_arg(nid, _) | ast::def_local(nid, _) | ast::def_self(nid) |
ast::def_binding(nid, _) => {
let typ = fcx.local_ty(sp, nid);
return no_params(typ);

2
src/librustc/middle/typeck/check/regionck.rs
View File

@ -58,7 +58,7 @@ fn encl_region_of_def(fcx: @mut FnCtxt, def: ast::def) -> ty::Region {
let tcx = fcx.tcx();
match def {
def_local(node_id, _) | def_arg(node_id, _) |
def_self(node_id, _) | def_binding(node_id, _) => {
def_self(node_id) | def_binding(node_id, _) => {
tcx.region_maps.encl_region(node_id)
}
def_upvar(_, subdef, closure_id, body_id) => {

2
src/libsyntax/ast.rs
View File

@ -171,7 +171,7 @@ pub enum def {
def_static_method(/* method */ def_id,
/* trait */ Option<def_id>,
purity),
def_self(NodeId, bool /* is_implicit */),
def_self(NodeId),
def_self_ty(/* trait id */ NodeId),
def_mod(def_id),
def_foreign_mod(def_id),

2
src/libsyntax/ast_util.rs
View File

@ -64,7 +64,7 @@ pub fn def_id_of_def(d: def) -> def_id {
def_use(id) | def_struct(id) | def_trait(id) | def_method(id, _) => {
id
}
def_arg(id, _) | def_local(id, _) | def_self(id, _) | def_self_ty(id)
def_arg(id, _) | def_local(id, _) | def_self(id) | def_self_ty(id)
| def_upvar(id, _, _, _) | def_binding(id, _) | def_region(id)
| def_typaram_binder(id) | def_label(id) => {
local_def(id)

30
src/test/compile-fail/borrowck-assign-to-andmut-in-aliasable-loc.rs Normal file
View File

@ -0,0 +1,30 @@
// Copyright 2012 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.
// Test that assignments to an `&mut` pointer which is found in a
// borrowed (but otherwise non-aliasable) location is illegal.
struct S<'self> {
pointer: &'self mut int
}
fn a(s: &S) {
*s.pointer += 1; //~ ERROR cannot assign
}
fn b(s: &mut S) {
*s.pointer += 1;
}
fn c(s: & &mut S) {
*s.pointer += 1; //~ ERROR cannot assign
}
fn main() {}

31
src/test/compile-fail/borrowck-assign-to-andmut-in-borrowed-loc.rs Normal file
View File

@ -0,0 +1,31 @@
// Copyright 2012 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.
// Test that assignments to an `&mut` pointer which is found in a
// borrowed (but otherwise non-aliasable) location is illegal.
struct S<'self> {
pointer: &'self mut int
}
fn copy_borrowed_ptr<'a>(p: &'a mut S<'a>) -> S<'a> {
S { pointer: &mut *p.pointer }
}
fn main() {
let mut x = 1;
{
let mut y = S { pointer: &mut x };
let z = copy_borrowed_ptr(&mut y);
*y.pointer += 1; //~ ERROR cannot assign
*z.pointer += 1;
}
}