From 0ea2a20397497a33af4d5d602e51cc50a8e15b4f Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 20 Aug 2013 17:37:49 -0400 Subject: [PATCH 1/2] Add PointerKind to LpDeref --- .../borrowck/gather_loans/restrictions.rs | 12 +++++------ src/librustc/middle/borrowck/mod.rs | 12 +++++------ src/librustc/middle/mem_categorization.rs | 20 +++++++++---------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index 46bb23e400ee5..3b518cb15904b 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -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,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 @@ -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. @@ -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 @@ -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 diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 95eae32922b7f..a84716bd73f8d 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -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 } @@ -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)) } } @@ -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(')'); @@ -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); } @@ -854,7 +854,7 @@ impl Repr for LoanPath { fmt!("$(%?)", id) } - &LpExtend(lp, _, LpDeref) => { + &LpExtend(lp, _, LpDeref(_)) => { fmt!("%s.*", lp.repr(tcx)) } diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 283724447f831..2cd2d94b9daf8 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -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 } @@ -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), @@ -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), } @@ -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(_) => ~"@", From 6b23d20452ef7c2d2eb79baf074bcda04fad9a66 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 21 Aug 2013 11:39:17 -0400 Subject: [PATCH 2/2] Prohibit assignment to `&mut` pointers that are found in frozen or borrowed locations. Fixes #8625. --- src/librustc/middle/borrowck/check_loans.rs | 15 +++++++-- ...rrowck-assign-to-andmut-in-borrowed-loc.rs | 31 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 src/test/compile-fail/borrowck-assign-to-andmut-in-borrowed-loc.rs diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 620a1e9efe33f..a2529261aafc3 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -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 @@ -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; } diff --git a/src/test/compile-fail/borrowck-assign-to-andmut-in-borrowed-loc.rs b/src/test/compile-fail/borrowck-assign-to-andmut-in-borrowed-loc.rs new file mode 100644 index 0000000000000..dcef74b6c2bc2 --- /dev/null +++ b/src/test/compile-fail/borrowck-assign-to-andmut-in-borrowed-loc.rs @@ -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 or the MIT license +// , 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; + } +}