Skip to content

Commit

Permalink
rustc: Allow safe access of shareable static muts
Browse files Browse the repository at this point in the history
Taking a shareable loan of a `static mut` is safe if the type contained in the
location is ascribes to `Share`. This commit removes the need to have an
`unsafe` block in these situations.

Unsafe blocks are still required to write to or take mutable loans out of
statics. An example of code that no longer requires unsafe code is:

    use std::sync::atomics;

    static mut CNT: atomics::AtomicUint = atomics::INIT_ATOMIC_UINT;

    let cnt = CNT.fetch_add(1, atomics::SeqCst);

As a consequence of this change, this code is not permitted:

    static mut FOO: uint = 30;

    println!("{}", FOO);

The type `uint` is `Share`, and the static only has a shareable loan taken out
on it, so the access does not require an unsafe block. Writes to the static are
still unsafe, however, as they can invoke undefined behavior if not properly
synchronized.

Closes rust-lang#13232
  • Loading branch information
alexcrichton committed Aug 1, 2014
1 parent b495933 commit 4c62486
Show file tree
Hide file tree
Showing 5 changed files with 214 additions and 4 deletions.
111 changes: 109 additions & 2 deletions src/librustc/middle/effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ use middle::def;
use middle::ty;
use middle::typeck::MethodCall;
use util::ppaux;
use util::nodemap::NodeSet;
use euv = middle::expr_use_visitor;
use mc = middle::mem_categorization;

use syntax::ast;
use syntax::ast_util::PostExpansionMethod;
Expand All @@ -40,10 +43,18 @@ fn type_is_unsafe_function(ty: ty::t) -> bool {
struct EffectCheckVisitor<'a> {
tcx: &'a ty::ctxt,

mutably_accessed_statics: &'a mut NodeSet,

/// Whether we're in an unsafe context.
unsafe_context: UnsafeContext,
}

struct FunctionVisitor<'a, 'b>(euv::ExprUseVisitor<'a, 'b, ty::ctxt>);

struct StaticMutChecker<'a> {
mutably_accessed_statics: NodeSet,
}

impl<'a> EffectCheckVisitor<'a> {
fn require_unsafe(&mut self, span: Span, description: &str) {
match self.unsafe_context {
Expand Down Expand Up @@ -142,6 +153,10 @@ impl<'a> Visitor<()> for EffectCheckVisitor<'a> {
}

fn visit_expr(&mut self, expr: &ast::Expr, _:()) {
if self.mutably_accessed_statics.remove(&expr.id) {
self.require_unsafe(expr.span, "mutable use of static")
}

match expr.node {
ast::ExprMethodCall(_, _, _) => {
let method_call = MethodCall::expr(expr.id);
Expand Down Expand Up @@ -185,7 +200,12 @@ impl<'a> Visitor<()> for EffectCheckVisitor<'a> {
ast::ExprPath(..) => {
match ty::resolve_expr(self.tcx, expr) {
def::DefStatic(_, true) => {
self.require_unsafe(expr.span, "use of mutable static")
let ty = ty::node_id_to_type(self.tcx, expr.id);
let contents = ty::type_contents(self.tcx, ty);
if !contents.is_sharable(self.tcx) {
self.require_unsafe(expr.span,
"use of non-Share static mut")
}
}
_ => {}
}
Expand All @@ -197,11 +217,98 @@ impl<'a> Visitor<()> for EffectCheckVisitor<'a> {
}
}

impl<'a, 'b> Visitor<()> for FunctionVisitor<'a, 'b> {
fn visit_fn(&mut self, fk: &visit::FnKind, fd: &ast::FnDecl,
b: &ast::Block, s: Span, _: ast::NodeId, _: ()) {
{
let FunctionVisitor(ref mut inner) = *self;
inner.walk_fn(fd, b);
}
visit::walk_fn(self, fk, fd, b, s, ());
}
}

impl<'a> StaticMutChecker<'a> {
fn is_static_mut(&self, mut cur: &mc::cmt) -> bool {
loop {
match cur.cat {
mc::cat_static_item => {
return match cur.mutbl {
mc::McImmutable => return false,
_ => true
}
}
mc::cat_deref(ref cmt, _, _) |
mc::cat_discr(ref cmt, _) |
mc::cat_downcast(ref cmt) |
mc::cat_interior(ref cmt, _) => cur = cmt,

mc::cat_rvalue(..) |
mc::cat_copied_upvar(..) |
mc::cat_upvar(..) |
mc::cat_local(..) |
mc::cat_arg(..) => return false
}
}
}
}

impl<'a> euv::Delegate for StaticMutChecker<'a> {
fn borrow(&mut self,
borrow_id: ast::NodeId,
_borrow_span: Span,
cmt: mc::cmt,
_loan_region: ty::Region,
bk: ty::BorrowKind,
_loan_cause: euv::LoanCause) {
if !self.is_static_mut(&cmt) {
return
}
match bk {
ty::ImmBorrow => {}
ty::UniqueImmBorrow | ty::MutBorrow => {
self.mutably_accessed_statics.insert(borrow_id);
}
}
}

fn mutate(&mut self,
assignment_id: ast::NodeId,
_assignment_span: Span,
assignee_cmt: mc::cmt,
_mode: euv::MutateMode) {
if !self.is_static_mut(&assignee_cmt) {
return
}
self.mutably_accessed_statics.insert(assignment_id);
}

fn consume(&mut self,
_consume_id: ast::NodeId,
_consume_span: Span,
_cmt: mc::cmt,
_mode: euv::ConsumeMode) {}
fn consume_pat(&mut self,
_consume_pat: &ast::Pat,
_cmt: mc::cmt,
_mode: euv::ConsumeMode) {}
fn decl_without_init(&mut self, _id: ast::NodeId, _span: Span) {}
}

pub fn check_crate(tcx: &ty::ctxt, krate: &ast::Crate) {
let mut delegate = StaticMutChecker {
mutably_accessed_statics: NodeSet::new(),
};
{
let visitor = euv::ExprUseVisitor::new(&mut delegate, tcx);
visit::walk_crate(&mut FunctionVisitor(visitor), krate, ());
}

let mut visitor = EffectCheckVisitor {
tcx: tcx,
unsafe_context: SafeContext,
mutably_accessed_statics: &mut delegate.mutably_accessed_statics,
};

visit::walk_crate(&mut visitor, krate, ());
assert!(visitor.mutably_accessed_statics.len() == 0);
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,4 @@ extern {
fn main() {
a += 3; //~ ERROR: requires unsafe
a = 4; //~ ERROR: requires unsafe
let _b = a; //~ ERROR: requires unsafe
}
66 changes: 66 additions & 0 deletions src/test/compile-fail/static-mut-needs-unsafe.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright 2014 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.

use std::kinds::marker;

struct NonSharable {
field: uint,
noshare: marker::NoShare
}

struct Sharable {
field: uint
}

impl Sharable {
fn foo(&self) {}
fn foo_mut(&mut self) {}
}

static mut NON_SHARABLE: NonSharable = NonSharable {
field: 1,
noshare: marker::NoShare,
};

static mut SHARABLE: Sharable = Sharable { field: 0 };

pub fn fn_mut(_: &mut Sharable) {}

pub fn main() {
SHARABLE.foo();

SHARABLE.foo_mut();
//~^ ERROR: mutable use of static requires unsafe function or block

SHARABLE.field = 2;
//~^ ERROR: mutable use of static requires unsafe function or block

fn_mut(&mut SHARABLE);
//~^ ERROR mutable use of static requires unsafe function or block

NON_SHARABLE.field = 2;
//~^ ERROR: use of non-Share static mut requires unsafe function or block
//~^^ ERROR: mutable use of static requires unsafe function or block

SHARABLE = Sharable {field: 1};
//~^ ERROR: mutable use of static requires unsafe function or block

let _: &mut Sharable = &mut SHARABLE;
//~^ ERROR mutable use of static requires unsafe function or block

let _ = &NON_SHARABLE.field;
//~^ ERROR: use of non-Share static mut requires unsafe function or block

let mut slc = ['a', 'c'];
slc[NON_SHARABLE.field] = 'b';
//~^ ERROR: use of non-Share static mut requires unsafe function or block

slc[SHARABLE.field] = 'b';
}
1 change: 0 additions & 1 deletion src/test/compile-fail/static-mut-requires-unsafe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,4 @@ static mut a: int = 3;
fn main() {
a += 3; //~ ERROR: requires unsafe
a = 4; //~ ERROR: requires unsafe
let _b = a; //~ ERROR: requires unsafe
}
39 changes: 39 additions & 0 deletions src/test/run-pass/static-mut-no-need-unsafe.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2014 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.

struct Sharable {
field: uint
}

impl Sharable {
fn foo(&self) {}
fn foo_mut(&mut self) {}
}

static mut FOO: Sharable = Sharable { field: 1 };

fn borrow_static(_: &Sharable) {}

pub fn main() {

FOO.foo();

borrow_static(&FOO);

let _ = &FOO;

unsafe { let _: &mut Sharable = &mut FOO; }

let mut slc = ['a', 'c'];
slc[FOO.field] = 'b';

let _ = &((((FOO))));
}

0 comments on commit 4c62486

Please sign in to comment.