Make &mut borrows restrict aliasing

Fixes #11913
This commit is contained in:
Niko Matsakis 2014-02-07 17:28:56 -05:00
parent 3df1eb2c2b
commit e9b9067560
8 changed files with 161 additions and 123 deletions

View File

@ -681,7 +681,7 @@ fn encode_explicit_self(ebml_w: &mut writer::Encoder, explicit_self: ast::Explic
ebml_w.end_tag();
fn encode_mutability(ebml_w: &writer::Encoder,
fn encode_mutability(ebml_w: &mut writer::Encoder,
m: ast::Mutability) {
match m {
MutImmutable => { ebml_w.writer.write(&[ 'i' as u8 ]); }

View File

@ -22,7 +22,6 @@ use mc = middle::mem_categorization;
use middle::borrowck::*;
use middle::moves;
use middle::ty;
use syntax::ast::{MutImmutable, MutMutable};
use syntax::ast;
use syntax::ast_map;
use syntax::ast_util;
@ -220,8 +219,8 @@ impl<'a> CheckLoanCtxt<'a> {
// Restrictions that would cause the new loan to be illegal:
let illegal_if = match loan2.mutbl {
MutableMutability => RESTR_ALIAS | RESTR_FREEZE | RESTR_CLAIM,
ImmutableMutability => RESTR_ALIAS | RESTR_FREEZE,
MutableMutability => RESTR_FREEZE | RESTR_CLAIM,
ImmutableMutability => RESTR_FREEZE,
};
debug!("illegal_if={:?}", illegal_if);
@ -423,7 +422,7 @@ impl<'a> CheckLoanCtxt<'a> {
debug!("check_for_aliasable_mutable_writes(cmt={}, guarantor={})",
cmt.repr(this.tcx()), guarantor.repr(this.tcx()));
match guarantor.cat {
mc::cat_deref(b, _, mc::region_ptr(MutMutable, _)) => {
mc::cat_deref(b, _, mc::region_ptr(ast::MutMutable, _)) => {
// Statically prohibit writes to `&mut` when aliasable
check_for_aliasability_violation(this, expr, b);
@ -437,43 +436,18 @@ impl<'a> CheckLoanCtxt<'a> {
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(MutMutable, _)) |
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_deref(_, _, mc::unsafe_ptr(..)) |
mc::cat_static_item(..) |
mc::cat_deref(_, _, mc::gc_ptr) |
mc::cat_deref(_, _, mc::region_ptr(MutImmutable, _)) => {
// 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;
}
}
}
cmt: mc::cmt)
-> bool {
match cmt.freely_aliasable() {
None => {
return true;
}
Some(cause) => {
this.bccx.report_aliasability_violation(
expr.span,
MutabilityViolation,
cause);
return false;
}
}
}

View File

@ -151,14 +151,13 @@ that the value `(*x).f` may be mutated via the newly created reference
restrictions `RS` that accompany the loan.
The first restriction `((*x).f, [MUTATE, CLAIM, FREEZE])` states that
the lender may not mutate nor freeze `(*x).f`. Mutation is illegal
because `(*x).f` is only supposed to be mutated via the new reference,
not by mutating the original path `(*x).f`. Freezing is
the lender may not mutate, freeze, nor alias `(*x).f`. Mutation is
illegal because `(*x).f` is only supposed to be mutated via the new
reference, not by mutating the original path `(*x).f`. Freezing is
illegal because the path now has an `&mut` alias; so even if we the
lender were to consider `(*x).f` to be immutable, it might be mutated
via this alias. Both of these restrictions are temporary. They will be
enforced for the lifetime `'a` of the loan. After the loan expires,
the restrictions no longer apply.
via this alias. They will be enforced for the lifetime `'a` of the
loan. After the loan expires, the restrictions no longer apply.
The second restriction on `*x` is interesting because it does not
apply to the path that was lent (`(*x).f`) but rather to a prefix of
@ -188,11 +187,9 @@ The kinds of expressions which in-scope loans can render illegal are:
against mutating `lv`;
- *moves*: illegal if there is any in-scope restriction on `lv` at all;
- *mutable borrows* (`&mut lv`): illegal there is an in-scope restriction
against mutating `lv` or aliasing `lv`;
against claiming `lv`;
- *immutable borrows* (`&lv`): illegal there is an in-scope restriction
against freezing `lv` or aliasing `lv`;
- *read-only borrows* (`&const lv`): illegal there is an in-scope restriction
against aliasing `lv`.
against freezing `lv`.
## Formal rules
@ -238,19 +235,23 @@ live. (This is done via restrictions, read on.)
We start with the `gather_loans` pass, which walks the AST looking for
borrows. For each borrow, there are three bits of information: the
lvalue `LV` being borrowed and the mutability `MQ` and lifetime `LT`
of the resulting pointer. Given those, `gather_loans` applies three
of the resulting pointer. Given those, `gather_loans` applies four
validity tests:
1. `MUTABILITY(LV, MQ)`: The mutability of the reference is
compatible with the mutability of `LV` (i.e., not borrowing immutable
data as mutable).
2. `LIFETIME(LV, LT, MQ)`: The lifetime of the borrow does not exceed
2. `ALIASABLE(LV, MQ)`: The aliasability of the reference is
compatible with the aliasability of `LV`. The goal is to prevent
`&mut` borrows of aliasability data.
3. `LIFETIME(LV, LT, MQ)`: The lifetime of the borrow does not exceed
the lifetime of the value being borrowed. This pass is also
responsible for inserting root annotations to keep managed values
alive.
3. `RESTRICTIONS(LV, LT, ACTIONS) = RS`: This pass checks and computes the
4. `RESTRICTIONS(LV, LT, ACTIONS) = RS`: This pass checks and computes the
restrictions to maintain memory safety. These are the restrictions
that will go into the final loan. We'll discuss in more detail below.
@ -313,6 +314,47 @@ be borrowed if MQ is immutable or const:
MUTABILITY(*LV, MQ) // M-Deref-Borrowed-Mut
TYPE(LV) = &mut Ty
## Checking aliasability
The goal of the aliasability check is to ensure that we never permit
`&mut` borrows of aliasable data. Formally we define a predicate
`ALIASABLE(LV, MQ)` which if defined means that
"borrowing `LV` with mutability `MQ` is ok". The
Rust code corresponding to this predicate is the function
`check_aliasability()` in `middle::borrowck::gather_loans`.
### Checking aliasability of variables
Local variables are never aliasable as they are accessible only within
the stack frame.
ALIASABLE(X, MQ) // M-Var-Mut
### Checking aliasable of owned content
Owned content is aliasable if it is found in an aliasable location:
ALIASABLE(LV.f, MQ) // M-Field
ALIASABLE(LV, MQ)
ALIASABLE(*LV, MQ) // M-Deref-Unique
ALIASABLE(LV, MQ)
### Checking mutability of immutable pointer types
Immutable pointer types like `&T` are aliasable, and hence can only be
borrowed immutably:
ALIASABLE(*LV, imm) // M-Deref-Borrowed-Imm
TYPE(LV) = &Ty
### Checking mutability of mutable pointer types
`&mut T` can be frozen, so it is acceptable to borrow it as either imm or mut:
ALIASABLE(*LV, MQ) // M-Deref-Borrowed-Mut
TYPE(LV) = &mut Ty
## Checking lifetime
These rules aim to ensure that no data is borrowed for a scope that exceeds

View File

@ -460,6 +460,11 @@ impl<'a> GatherLoanCtxt<'a> {
return; // reported an error, no sense in reporting more.
}
// Check that we don't allow mutable borrows of aliasable data.
if check_aliasability(self.bccx, borrow_span, cmt, req_mutbl).is_err() {
return; // reported an error, no sense in reporting more.
}
// Compute the restrictions that are required to enforce the
// loan is safe.
let restr = restrictions::compute_restrictions(
@ -586,15 +591,53 @@ impl<'a> GatherLoanCtxt<'a> {
}
}
}
fn check_aliasability(bccx: &BorrowckCtxt,
borrow_span: Span,
cmt: mc::cmt,
req_mutbl: LoanMutability) -> Result<(),()> {
//! Implements the A-* rules in doc.rs.
match req_mutbl {
ImmutableMutability => {
// both imm and mut data can be lent as imm;
// for mutable data, this is a freeze
Ok(())
}
MutableMutability => {
// Check for those cases where we cannot control
// the aliasing and make sure that we are not
// being asked to.
match cmt.freely_aliasable() {
None => {
Ok(())
}
Some(mc::AliasableStaticMut) => {
// This is nasty, but we ignore the
// aliasing rules if the data is based in
// a `static mut`, since those are always
// unsafe. At your own peril and all that.
Ok(())
}
Some(cause) => {
bccx.report_aliasability_violation(
borrow_span,
BorrowViolation,
cause);
Err(())
}
}
}
}
}
}
pub fn restriction_set(&self, req_mutbl: LoanMutability)
-> RestrictionSet {
match req_mutbl {
ImmutableMutability => RESTR_EMPTY | RESTR_MUTATE | RESTR_CLAIM,
MutableMutability => {
RESTR_EMPTY | RESTR_MUTATE | RESTR_CLAIM | RESTR_FREEZE
}
ImmutableMutability => RESTR_MUTATE | RESTR_CLAIM,
MutableMutability => RESTR_MUTATE | RESTR_CLAIM | RESTR_FREEZE,
}
}

View File

@ -53,16 +53,6 @@ impl<'a> RestrictionsContext<'a> {
fn restrict(&self,
cmt: mc::cmt,
restrictions: RestrictionSet) -> RestrictionResult {
// Check for those cases where we cannot control the aliasing
// and make sure that we are not being asked to.
match cmt.freely_aliasable() {
None => {}
Some(cause) => {
self.check_aliasing_permitted(cause, restrictions);
}
}
match cmt.cat {
mc::cat_rvalue(..) => {
// Effectively, rvalues are stored into a
@ -179,28 +169,4 @@ impl<'a> RestrictionsContext<'a> {
}
}
}
fn check_aliasing_permitted(&self,
cause: mc::AliasableReason,
restrictions: RestrictionSet) {
//! This method is invoked when the current `cmt` is something
//! where aliasing cannot be controlled. It reports an error if
//! the restrictions required that it not be aliased; currently
//! this only occurs when re-borrowing an `&mut` pointer.
//!
//! NB: To be 100% consistent, we should report an error if
//! RESTR_FREEZE is found, because we cannot prevent freezing,
//! nor would we want to. However, we do not report such an
//! error, because this restriction only occurs when the user
//! is creating an `&mut` pointer to immutable or read-only
//! data, and there is already another piece of code that
//! checks for this condition.
if restrictions.intersects(RESTR_ALIAS) {
self.bccx.report_aliasability_violation(
self.span,
BorrowViolation,
cause);
}
}
}

View File

@ -316,7 +316,6 @@ pub fn opt_loan_path(cmt: mc::cmt) -> Option<@LoanPath> {
// - `RESTR_MUTATE`: The lvalue may not be modified.
// - `RESTR_CLAIM`: `&mut` borrows of the lvalue are forbidden.
// - `RESTR_FREEZE`: `&` borrows of the lvalue are forbidden.
// - `RESTR_ALIAS`: All borrows of the lvalue are forbidden.
//
// In addition, no value which is restricted may be moved. Therefore,
// restrictions are meaningful even if the RestrictionSet is empty,
@ -336,7 +335,6 @@ pub static RESTR_EMPTY: RestrictionSet = RestrictionSet {bits: 0b0000};
pub static RESTR_MUTATE: RestrictionSet = RestrictionSet {bits: 0b0001};
pub static RESTR_CLAIM: RestrictionSet = RestrictionSet {bits: 0b0010};
pub static RESTR_FREEZE: RestrictionSet = RestrictionSet {bits: 0b0100};
pub static RESTR_ALIAS: RestrictionSet = RestrictionSet {bits: 0b1000};
impl RestrictionSet {
pub fn intersects(&self, restr: RestrictionSet) -> bool {
@ -661,8 +659,8 @@ impl BorrowckCtxt {
kind: AliasableViolationKind,
cause: mc::AliasableReason) {
let prefix = match kind {
MutabilityViolation => "cannot assign to an `&mut`",
BorrowViolation => "cannot borrow an `&mut`"
MutabilityViolation => "cannot assign to data",
BorrowViolation => "cannot borrow data mutably"
};
match cause {
@ -671,17 +669,21 @@ impl BorrowckCtxt {
span,
format!("{} in an aliasable location", prefix));
}
mc::AliasableManaged => {
self.tcx.sess.span_err(span, format!("{} in a `@` pointer",
prefix))
}
mc::AliasableBorrowed(m) => {
mc::AliasableStatic |
mc::AliasableStaticMut => {
self.tcx.sess.span_err(
span,
format!("{} in a `&{}` pointer; \
try an `&mut` instead",
prefix,
self.mut_to_keyword(m)));
format!("{} in a static location", prefix));
}
mc::AliasableManaged => {
self.tcx.sess.span_err(
span,
format!("{} in a `@` pointer", prefix));
}
mc::AliasableBorrowed(_) => {
self.tcx.sess.span_err(
span,
format!("{} in a `&` reference", prefix));
}
}
}

View File

@ -1100,7 +1100,9 @@ pub fn field_mutbl(tcx: ty::ctxt,
pub enum AliasableReason {
AliasableManaged,
AliasableBorrowed(ast::Mutability),
AliasableOther
AliasableOther,
AliasableStatic,
AliasableStaticMut,
}
impl cmt_ {
@ -1130,10 +1132,6 @@ impl cmt_ {
}
}
pub fn is_freely_aliasable(&self) -> bool {
self.freely_aliasable().is_some()
}
pub fn freely_aliasable(&self) -> Option<AliasableReason> {
/*!
* Returns `Some(_)` if this lvalue represents a freely aliasable
@ -1145,20 +1143,36 @@ impl cmt_ {
// aliased and eventually recused.
match self.cat {
cat_deref(b, _, region_ptr(MutMutable, _)) |
cat_downcast(b) |
cat_stack_upvar(b) |
cat_deref(b, _, uniq_ptr) |
cat_interior(b, _) |
cat_discr(b, _) => {
// Aliasability depends on base cmt
b.freely_aliasable()
}
cat_copied_upvar(CopiedUpvar {onceness: ast::Once, ..}) |
cat_rvalue(..) |
cat_local(..) |
cat_arg(_) |
cat_deref(_, _, unsafe_ptr(..)) | // of course it is aliasable, but...
cat_deref(_, _, region_ptr(MutMutable, _)) => {
cat_deref(_, _, unsafe_ptr(..)) => { // yes, it's aliasable, but...
None
}
cat_copied_upvar(CopiedUpvar {onceness: ast::Many, ..}) |
cat_static_item(..) => {
cat_copied_upvar(CopiedUpvar {onceness: ast::Many, ..}) => {
Some(AliasableOther)
}
cat_static_item(..) => {
if self.mutbl.is_mutable() {
Some(AliasableStaticMut)
} else {
Some(AliasableStatic)
}
}
cat_deref(_, _, gc_ptr) => {
Some(AliasableManaged)
}
@ -1166,14 +1180,6 @@ impl cmt_ {
cat_deref(_, _, region_ptr(m @ MutImmutable, _)) => {
Some(AliasableBorrowed(m))
}
cat_downcast(..) |
cat_stack_upvar(..) |
cat_deref(_, _, uniq_ptr) |
cat_interior(..) |
cat_discr(..) => {
None
}
}
}
}

View File

@ -27,5 +27,10 @@ fn foo3(t0: &mut &mut int) {
**t1 = 22;
}
fn foo4(t0: & &mut int) {
let x: &mut int = &mut **t0; //~ ERROR cannot borrow
*x += 1;
}
fn main() {
}