Skip to content

Prohibit assiginments to &mut pointers in borrowed locations #8667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/librustc/middle/borrowck/check_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,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 @@ -513,9 +519,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
12 changes: 6 additions & 6 deletions src/librustc/middle/borrowck/gather_loans/restrictions.rs
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,7 +110,7 @@ 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
Expand All @@ -129,7 +129,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 +170,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 +187,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
12 changes: 6 additions & 6 deletions src/librustc/middle/borrowck/mod.rs
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 Down Expand Up @@ -295,9 +295,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 +728,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 +776,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 +854,7 @@ impl Repr for LoanPath {
fmt!("$(%?)", id)
}

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

Expand Down
20 changes: 10 additions & 10 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,18 @@ 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
}
Expand All @@ -82,8 +82,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),
Expand Down Expand Up @@ -147,7 +147,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),
}

Expand Down Expand Up @@ -1233,7 +1233,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(_) => ~"@",
Expand Down
31 changes: 31 additions & 0 deletions src/test/compile-fail/borrowck-assign-to-andmut-in-borrowed-loc.rs
Original file line number Diff line number Diff line change
@@ -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;
}
}