Skip to content

Commit

Permalink
auto merge of #8797 : nikomatsakis/rust/issue-8625-assign-to-andmut-i…
Browse files Browse the repository at this point in the history 
…n-borrowed-loc-2, r=pcwalton

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
  • Loading branch information
@bors
bors committed Aug 28, 2013
2 parents d5c144a + 8c09865 commit 58d6eb5
Show file tree
Hide file tree
Showing 16 changed files with 166 additions and 79 deletions.
2 changes: 1 addition & 1 deletion src/librustc/middle/astencode.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down
71 changes: 58 additions & 13 deletions src/librustc/middle/borrowck/check_loans.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(*)) |
Expand Down Expand Up @@ -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)) => {
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

Expand Down
1 change: 0 additions & 1 deletion src/librustc/middle/borrowck/gather_loans/gather_moves.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(*)) => {
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/middle/borrowck/gather_loans/lifetime.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
Expand Down
13 changes: 6 additions & 7 deletions src/librustc/middle/borrowck/gather_loans/restrictions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
15 changes: 7 additions & 8 deletions src/librustc/middle/borrowck/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

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

Expand All @@ -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))
}
}

Expand Down Expand Up @@ -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(')');
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -854,7 +853,7 @@ impl Repr for LoanPath {
fmt!("$(%?)", id)
}

&LpExtend(lp, _, LpDeref) => {
&LpExtend(lp, _, LpDeref(_)) => {
fmt!("%s.*", lp.repr(tcx))
}

Expand Down
Loading

0 comments on commit 58d6eb5

Please sign in to comment.