Forbid borrow of static items with unsafe interior

6 files changed, 112 insertions, 34 deletions

diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs
index 841123906cc..7dfaaa6900c 100644
--- a/src/librustc/middle/borrowck/check_loans.rs
+++ b/src/librustc/middle/borrowck/check_loans.rs
@@ -518,11 +518,11 @@ impl<'a> CheckLoanCtxt<'a> {
                                             expr: &ast::Expr,
                                             cmt: mc::cmt)
                                             -> bool {
-            match cmt.freely_aliasable() {
+            match cmt.freely_aliasable(this.tcx()) {
                 None => {
                     return true;
                 }
-                Some(mc::AliasableStaticMut) => {
+                Some(mc::AliasableStaticMut(..)) => {
                     return true;
                 }
                 Some(cause) => {
diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs
index b92a23a53ee..459c908cae2 100644
--- a/src/librustc/middle/borrowck/gather_loans/mod.rs
+++ b/src/librustc/middle/borrowck/gather_loans/mod.rs
@@ -175,9 +175,9 @@ pub fn gather_loans_in_static_initializer(bccx: &mut BorrowckCtxt, expr: &ast::E
         move_data: MoveData::new()
     };
 
+    // FIXME #13005 This should also walk the
+    // expression.
     match expr.node {
-        // Just visit the expression if the
-        // item is taking an address.
         ast::ExprAddrOf(..) => {
             glcx.visit_expr(expr, ());
         }
@@ -686,34 +686,45 @@ impl<'a> GatherLoanCtxt<'a> {
                               -> Result<(),()> {
             //! Implements the A-* rules in doc.rs.
 
-            match req_kind {
-                ty::ImmBorrow => {
+            match (cmt.freely_aliasable(bccx.tcx), req_kind) {
+                (None, _) => {
+                    /* Uniquely accessible path -- OK for `&` and `&mut` */
                     Ok(())
                 }
-
-                ty::UniqueImmBorrow | ty::MutBorrow => {
-                    // 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::AliasableStatic(safety)), ty::ImmBorrow) => {
+                    // Borrow of an immutable static item:
+                    match safety {
+                        mc::InteriorUnsafe => {
+                            // If the static item contains an Unsafe<T>, it has interior mutability.
+                            // In such cases, we cannot permit it to be borrowed, because the
+                            // static item resides in immutable memory and mutating it would
+                            // cause segfaults.
+                            bccx.tcx.sess.span_err(borrow_span,
+                                                   format!("borrow of immutable static items with \
+                                                            unsafe interior is not allowed"));
+                            Err(())
                         }
-                        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.
+                        mc::InteriorSafe => {
+                            // Immutable static can be borrowed, no problem.
                             Ok(())
                         }
-                        Some(alias_cause) => {
-                            bccx.report_aliasability_violation(
+                    }
+                }
+                (Some(mc::AliasableStaticMut(..)), _) => {
+                    // Even touching a static mut is considered unsafe. We assume the
+                    // user knows what they're doing in these cases.
+                    Ok(())
+                }
+                (Some(alias_cause), ty::UniqueImmBorrow) |
+                (Some(alias_cause), ty::MutBorrow) => {
+                    bccx.report_aliasability_violation(
                                 borrow_span,
                                 BorrowViolation(loan_cause),
                                 alias_cause);
-                            Err(())
-                        }
-                    }
+                    Err(())
+                }
+                (_, _) => {
+                    Ok(())
                 }
             }
         }
diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs
index 9657e935873..dd2f02bf241 100644
--- a/src/librustc/middle/borrowck/mod.rs
+++ b/src/librustc/middle/borrowck/mod.rs
@@ -130,9 +130,10 @@ fn borrowck_item(this: &mut BorrowckCtxt, item: &ast::Item) {
         ast::ItemStatic(_, _, ex) => {
             gather_loans::gather_loans_in_static_initializer(this, ex);
         }
-        _ => {}
+        _ => {
+            visit::walk_item(this, item, ());
+        }
     }
-    visit::walk_item(this, item, ());
 }
 
 fn borrowck_fn(this: &mut BorrowckCtxt,
@@ -733,8 +734,8 @@ impl<'a> BorrowckCtxt<'a> {
                     span,
                     format!("{} in an aliasable location", prefix));
             }
-            mc::AliasableStatic |
-            mc::AliasableStaticMut => {
+            mc::AliasableStatic(..) |
+            mc::AliasableStaticMut(..) => {
                 self.tcx.sess.span_err(
                     span,
                     format!("{} in a static location", prefix));
diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs
index 7c95815af54..49dbe668403 100644
--- a/src/librustc/middle/mem_categorization.rs
+++ b/src/librustc/middle/mem_categorization.rs
@@ -1222,12 +1222,17 @@ pub fn field_mutbl(tcx: &ty::ctxt,
     return None;
 }
 
+pub enum InteriorSafety {
+    InteriorUnsafe,
+    InteriorSafe
+}
+
 pub enum AliasableReason {
     AliasableManaged,
     AliasableBorrowed,
     AliasableOther,
-    AliasableStatic,
-    AliasableStaticMut,
+    AliasableStatic(InteriorSafety),
+    AliasableStaticMut(InteriorSafety),
 }
 
 impl cmt_ {
@@ -1257,7 +1262,7 @@ impl cmt_ {
         }
     }
 
-    pub fn freely_aliasable(&self) -> Option<AliasableReason> {
+    pub fn freely_aliasable(&self, ctxt: &ty::ctxt) -> Option<AliasableReason> {
         /*!
          * Returns `Some(_)` if this lvalue represents a freely aliasable
          * pointer type.
@@ -1275,7 +1280,7 @@ impl cmt_ {
             cat_interior(b, _) |
             cat_discr(b, _) => {
                 // Aliasability depends on base cmt
-                b.freely_aliasable()
+                b.freely_aliasable(ctxt)
             }
 
             cat_copied_upvar(CopiedUpvar {onceness: ast::Once, ..}) |
@@ -1292,10 +1297,16 @@ impl cmt_ {
             }
 
             cat_static_item(..) => {
+                let int_safe = if ty::type_interior_is_unsafe(ctxt, self.ty) {
+                    InteriorUnsafe
+                } else {
+                    InteriorSafe
+                };
+
                 if self.mutbl.is_mutable() {
-                    Some(AliasableStaticMut)
+                    Some(AliasableStaticMut(int_safe))
                 } else {
-                    Some(AliasableStatic)
+                    Some(AliasableStatic(int_safe))
                 }
             }
 
diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs
index cf974b2f1fa..653b4372695 100644
--- a/src/librustc/middle/ty.rs
+++ b/src/librustc/middle/ty.rs
@@ -1996,6 +1996,10 @@ impl TypeContents {
         !self.intersects(TC::Nonpod)
     }
 
+    pub fn interior_unsafe(&self) -> bool {
+        self.intersects(TC::InteriorUnsafe)
+    }
+
     pub fn moves_by_default(&self, _: &ctxt) -> bool {
         self.intersects(TC::Moves)
     }
@@ -2092,6 +2096,10 @@ pub fn type_is_freezable(cx: &ctxt, t: ty::t) -> bool {
     type_contents(cx, t).is_freezable(cx)
 }
 
+pub fn type_interior_is_unsafe(cx: &ctxt, t: ty::t) -> bool {
+    type_contents(cx, t).interior_unsafe()
+}
+
 pub fn type_contents(cx: &ctxt, ty: t) -> TypeContents {
     let ty_id = type_id(ty);
 
diff --git a/src/test/compile-fail/borrowck-forbid-static-unsafe-interior.rs b/src/test/compile-fail/borrowck-forbid-static-unsafe-interior.rs
new file mode 100644
index 00000000000..c790a040a91
--- /dev/null
+++ b/src/test/compile-fail/borrowck-forbid-static-unsafe-interior.rs
@@ -0,0 +1,47 @@
+// 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.
+
+// Verify that it is not possible to take the address of
+// static items with usnafe interior.
+
+use std::kinds::marker;
+use std::ty::Unsafe;
+
+struct MyUnsafe<T> {
+    value: Unsafe<T>
+}
+
+impl<T> MyUnsafe<T> {
+    fn forbidden(&self) {}
+}
+
+enum UnsafeEnum<T> {
+    VariantSafe,
+    VariantUnsafe(Unsafe<T>)
+}
+
+static STATIC1: UnsafeEnum<int> = VariantSafe;
+
+static STATIC2: Unsafe<int> = Unsafe{value: 1, marker1: marker::InvariantType};
+static STATIC3: MyUnsafe<int> = MyUnsafe{value: STATIC2};
+
+static STATIC4: &'static Unsafe<int> = &'static STATIC2;
+//~^ ERROR borrow of immutable static items with unsafe interior is not allowed
+
+
+fn main() {
+    let a = &STATIC1;
+    //~^ ERROR borrow of immutable static items with unsafe interior is not allowed
+
+    STATIC3.forbidden()
+    //~^ ERROR borrow of immutable static items with unsafe interior is not allowed
+}
+
+