auto merge of #11979 : FlaPer87/rust/static, r=nikomatsakis

This pull request partially addresses the 2 issues listed before. As part of the work required for this PR, `NonCopyable` was completely removed.

This PR also replaces the content of `type_is_pod` with `TypeContents::is_pod`, although `type_is_content` is currently not being used anywhere. I kept it for consistency with the other functions that exist in this module.

cc #10834
cc #10577

Proposed static restrictions
=====================

Taken from [this](https://github.com/mozilla/rust/pull/11979#issuecomment-35768249) comment.

I expect some code that, at a high-level, works like this:

- For each *mutable* static item, check that the **type**:
    - cannot own any value whose type has a dtor
    - cannot own any values whose type is an owned pointer
- For each *immutable* static item, check that the **value**:
      - does not contain any ~ or box expressions (including ~[1, 2, 3] sort of things, for now)
      - does not contain a struct literal or call to an enum variant / struct constructor where
          - the type of the struct/enum is freeze
          - the type of the struct/enum has a dtor
This commit is contained in:
bors 2014-02-27 18:51:53 -08:00
commit 700fd35fb9
18 changed files with 459 additions and 94 deletions

View File

@ -301,6 +301,9 @@ pub fn phase_3_run_analysis_passes(sess: Session,
// passes are timed inside typeck
let (method_map, vtable_map) = typeck::check_crate(ty_cx, trait_map, krate);
time(time_passes, "check static items", (), |_|
middle::check_static::check_crate(ty_cx, krate));
// These next two const passes can probably be merged
time(time_passes, "const marking", (), |_|
middle::const_eval::process_crate(krate, ty_cx));

View File

@ -69,6 +69,7 @@ pub mod middle {
pub mod check_loop;
pub mod check_match;
pub mod check_const;
pub mod check_static;
pub mod lint;
pub mod borrowck;
pub mod dataflow;

View File

@ -104,7 +104,7 @@ fn check_is_legal_to_move_from(bccx: &BorrowckCtxt,
mc::cat_deref(_, _, mc::BorrowedPtr(..)) |
mc::cat_deref(_, _, mc::GcPtr) |
mc::cat_deref(_, _, mc::UnsafePtr(..)) |
mc::cat_upvar(..) |
mc::cat_upvar(..) | mc::cat_static_item |
mc::cat_copied_upvar(mc::CopiedUpvar { onceness: ast::Many, .. }) => {
bccx.span_err(
cmt0.span,
@ -120,19 +120,6 @@ fn check_is_legal_to_move_from(bccx: &BorrowckCtxt,
true
}
// It seems strange to allow a move out of a static item,
// but what happens in practice is that you have a
// reference to a constant with a type that should be
// moved, like `None::<~int>`. The type of this constant
// is technically `Option<~int>`, which moves, but we know
// that the content of static items will never actually
// contain allocated pointers, so we can just memcpy it.
// Since static items can never have allocated memory,
// this is ok. For now anyhow.
mc::cat_static_item => {
true
}
mc::cat_rvalue(..) |
mc::cat_local(..) |
mc::cat_arg(..) => {

View File

@ -0,0 +1,159 @@
// 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.
// Verifies that the types and values of static items
// are safe. The rules enforced by this module are:
//
// - For each *mutable* static item, it checks that its **type**:
// - doesn't have a destructor
// - doesn't own an owned pointer
//
// - For each *immutable* static item, it checks that its **value**:
// - doesn't own owned, managed pointers
// - doesn't contain a struct literal or a call to an enum variant / struct constructor where
// - the type of the struct/enum is not freeze
// - the type of the struct/enum has a dtor
use middle::ty;
use syntax::ast;
use syntax::codemap::Span;
use syntax::visit::Visitor;
use syntax::visit;
use syntax::print::pprust;
fn safe_type_for_static_mut(cx: ty::ctxt, e: &ast::Expr) -> Option<~str> {
let node_ty = ty::node_id_to_type(cx, e.id);
let tcontents = ty::type_contents(cx, node_ty);
debug!("safe_type_for_static_mut(dtor={}, managed={}, owned={})",
tcontents.has_dtor(), tcontents.owns_managed(), tcontents.owns_owned())
let suffix = if tcontents.has_dtor() {
"destructors"
} else if tcontents.owns_managed() {
"managed pointers"
} else if tcontents.owns_owned() {
"owned pointers"
} else {
return None;
};
Some(format!("mutable static items are not allowed to have {}", suffix))
}
struct CheckStaticVisitor {
tcx: ty::ctxt,
}
pub fn check_crate(tcx: ty::ctxt, krate: &ast::Crate) {
visit::walk_crate(&mut CheckStaticVisitor { tcx: tcx }, krate, false)
}
impl CheckStaticVisitor {
fn report_error(&self, span: Span, result: Option<~str>) -> bool {
match result {
None => { false }
Some(msg) => {
self.tcx.sess.span_err(span, msg);
true
}
}
}
}
impl Visitor<bool> for CheckStaticVisitor {
fn visit_item(&mut self, i: &ast::Item, _is_const: bool) {
debug!("visit_item(item={})", pprust::item_to_str(i));
match i.node {
ast::ItemStatic(_, mutability, expr) => {
match mutability {
ast::MutImmutable => {
self.visit_expr(expr, true);
}
ast::MutMutable => {
self.report_error(expr.span, safe_type_for_static_mut(self.tcx, expr));
}
}
}
_ => { visit::walk_item(self, i, false) }
}
}
/// This method is used to enforce the constraints on
/// immutable static items. It walks through the *value*
/// of the item walking down the expression and evaluating
/// every nested expression. if the expression is not part
/// of a static item, this method does nothing but walking
/// down through it.
fn visit_expr(&mut self, e: &ast::Expr, is_const: bool) {
debug!("visit_expr(expr={})", pprust::expr_to_str(e));
if !is_const {
return visit::walk_expr(self, e, is_const);
}
match e.node {
ast::ExprField(..) | ast::ExprVec(..) |
ast::ExprBlock(..) | ast::ExprTup(..) |
ast::ExprVstore(_, ast::ExprVstoreSlice) => {
visit::walk_expr(self, e, is_const);
}
ast::ExprUnary(ast::UnBox, _) => {
self.tcx.sess.span_err(e.span,
"static items are not allowed to have managed pointers");
}
ast::ExprBox(..) |
ast::ExprUnary(ast::UnUniq, _) |
ast::ExprVstore(_, ast::ExprVstoreUniq) => {
self.tcx.sess.span_err(e.span,
"static items are not allowed to have owned pointers");
}
ast::ExprProc(..) => {
self.report_error(e.span,
Some(~"immutable static items must be `Freeze`"));
return;
}
ast::ExprAddrOf(mutability, _) => {
match mutability {
ast::MutMutable => {
self.report_error(e.span,
Some(~"immutable static items must be `Freeze`"));
return;
}
_ => {}
}
}
_ => {
let node_ty = ty::node_id_to_type(self.tcx, e.id);
match ty::get(node_ty).sty {
ty::ty_struct(did, _) |
ty::ty_enum(did, _) => {
if ty::has_dtor(self.tcx, did) {
self.report_error(e.span,
Some(~"static items are not allowed to have destructors"));
return;
}
if Some(did) == self.tcx.lang_items.no_freeze_bound() {
self.report_error(e.span,
Some(~"immutable static items must be `Freeze`"));
return;
}
}
_ => {}
}
visit::walk_expr(self, e, is_const);
}
}
}
}

View File

@ -1950,6 +1950,10 @@ impl TypeContents {
self.intersects(TC::OwnsManaged)
}
pub fn owns_owned(&self) -> bool {
self.intersects(TC::OwnsOwned)
}
pub fn is_freezable(&self, _: ctxt) -> bool {
!self.intersects(TC::Nonfreezable)
}
@ -2012,6 +2016,10 @@ impl TypeContents {
pub fn inverse(&self) -> TypeContents {
TypeContents { bits: !self.bits }
}
pub fn has_dtor(&self) -> bool {
self.intersects(TC::OwnsDtor)
}
}
impl ops::BitOr<TypeContents,TypeContents> for TypeContents {
@ -2038,6 +2046,10 @@ impl fmt::Show for TypeContents {
}
}
pub fn type_has_dtor(cx: ctxt, t: ty::t) -> bool {
type_contents(cx, t).has_dtor()
}
pub fn type_is_static(cx: ctxt, t: ty::t) -> bool {
type_contents(cx, t).is_static(cx)
}
@ -2624,61 +2636,6 @@ pub fn type_is_machine(ty: t) -> bool {
}
}
// Whether a type is Plain Old Data -- meaning it does not contain pointers
// that the cycle collector might care about.
pub fn type_is_pod(cx: ctxt, ty: t) -> bool {
let mut result = true;
match get(ty).sty {
// Scalar types
ty_nil | ty_bot | ty_bool | ty_char | ty_int(_) | ty_float(_) | ty_uint(_) |
ty_ptr(_) | ty_bare_fn(_) => result = true,
// Boxed types
ty_box(_) | ty_uniq(_) | ty_closure(_) |
ty_str(vstore_uniq) |
ty_vec(_, vstore_uniq) |
ty_trait(_, _, _, _, _) | ty_rptr(_,_) => result = false,
// Structural types
ty_enum(did, ref substs) => {
let variants = enum_variants(cx, did);
for variant in (*variants).iter() {
// FIXME(pcwalton): This is an inefficient way to do this. Don't
// synthesize a tuple!
//
// Perform any type parameter substitutions.
let tup_ty = mk_tup(cx, variant.args.clone());
let tup_ty = subst(cx, substs, tup_ty);
if !type_is_pod(cx, tup_ty) { result = false; }
}
}
ty_tup(ref elts) => {
for elt in elts.iter() { if !type_is_pod(cx, *elt) { result = false; } }
}
ty_str(vstore_fixed(_)) => result = true,
ty_vec(ref mt, vstore_fixed(_)) | ty_unboxed_vec(ref mt) => {
result = type_is_pod(cx, mt.ty);
}
ty_param(_) => result = false,
ty_struct(did, ref substs) => {
let fields = lookup_struct_fields(cx, did);
result = fields.iter().all(|f| {
let fty = ty::lookup_item_type(cx, f.id);
let sty = subst(cx, substs, fty.ty);
type_is_pod(cx, sty)
});
}
ty_str(vstore_slice(..)) | ty_vec(_, vstore_slice(..)) => {
result = false;
}
ty_infer(..) | ty_self(..) | ty_err => {
cx.sess.bug("non concrete type in type_is_pod");
}
}
return result;
}
pub fn type_is_enum(ty: t) -> bool {
match get(ty).sty {
ty_enum(_, _) => return true,

View File

@ -725,6 +725,7 @@ impl CoherenceChecker {
let self_type = self.get_self_type_for_implementation(*impl_info);
match ty::get(self_type.ty).sty {
ty::ty_enum(type_def_id, _) |
ty::ty_struct(type_def_id, _) => {
let mut destructor_for_type = tcx.destructor_for_type
.borrow_mut();

View File

@ -0,0 +1,29 @@
// 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.
// Ensure that moves out of static items is forbidden
use std::kinds::marker;
struct Foo {
foo: int,
nopod: marker::NoPod
}
static BAR: Foo = Foo{foo: 5, nopod: marker::NoPod};
fn test(f: Foo) {
let _f = Foo{foo: 4, ..f};
}
fn main() {
test(BAR); //~ ERROR cannot move out of static item
}

View File

@ -0,0 +1,154 @@
// 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.
#[feature(managed_boxes)];
// Verifies all possible restrictions for static items values.
use std::kinds::marker;
struct WithDtor;
impl Drop for WithDtor {
fn drop(&mut self) {}
}
// This enum will be used to test the following rules:
// 1. Variants are safe for static
// 2. Expr calls are allowed as long as they arguments are safe
// 3. Expr calls with unsafe arguments for static items are rejected
enum SafeEnum {
Variant1,
Variant2(int),
Variant3(WithDtor),
Variant4(~str)
}
// These should be ok
static STATIC1: SafeEnum = Variant1;
static STATIC2: SafeEnum = Variant2(0);
// This one should fail
static STATIC3: SafeEnum = Variant3(WithDtor);
//~^ ERROR static items are not allowed to have destructors
// This enum will be used to test that variants
// are considered unsafe if their enum type implements
// a destructor.
enum UnsafeEnum {
Variant5,
Variant6(int)
}
impl Drop for UnsafeEnum {
fn drop(&mut self) {}
}
static STATIC4: UnsafeEnum = Variant5;
//~^ ERROR static items are not allowed to have destructors
static STATIC5: UnsafeEnum = Variant6(0);
//~^ ERROR static items are not allowed to have destructors
struct SafeStruct {
field1: SafeEnum,
field2: SafeEnum,
}
// Struct fields are safe, hence this static should be safe
static STATIC6: SafeStruct = SafeStruct{field1: Variant1, field2: Variant2(0)};
// field2 has an unsafe value, hence this should fail
static STATIC7: SafeStruct = SafeStruct{field1: Variant1, field2: Variant3(WithDtor)};
//~^ ERROR static items are not allowed to have destructors
// Test variadic constructor for structs. The base struct should be examined
// as well as every field persent in the constructor.
// This example shouldn't fail because all the fields are safe.
static STATIC8: SafeStruct = SafeStruct{field1: Variant1,
..SafeStruct{field1: Variant1, field2: Variant1}};
// This example should fail because field1 in the base struct is not safe
static STATIC9: SafeStruct = SafeStruct{field1: Variant1,
..SafeStruct{field1: Variant3(WithDtor), field2: Variant1}};
//~^ ERROR static items are not allowed to have destructors
struct UnsafeStruct;
impl Drop for UnsafeStruct {
fn drop(&mut self) {}
}
// Types with destructors are not allowed for statics
static STATIC10: UnsafeStruct = UnsafeStruct;
//~^ ERROR static items are not allowed to have destructor
static STATIC11: ~str = ~"Owned pointers are not allowed either";
//~^ ERROR static items are not allowed to have owned pointers
// The following examples test that mutable structs are just forbidden
// to have types with destructors
// These should fail
static mut STATIC12: UnsafeStruct = UnsafeStruct;
//~^ ERROR mutable static items are not allowed to have destructors
static mut STATIC13: SafeStruct = SafeStruct{field1: Variant1, field2: Variant3(WithDtor)};
//~^ ERROR mutable static items are not allowed to have destructors
static mut STATIC14: SafeStruct = SafeStruct{field1: Variant1, field2: Variant4(~"str")};
//~^ ERROR mutable static items are not allowed to have destructors
static STATIC15: &'static [~str] = &'static [~"str", ~"str"];
//~^ ERROR static items are not allowed to have owned pointers
//~^^ ERROR static items are not allowed to have owned pointers
static STATIC16: (~str, ~str) = (~"str", ~"str");
//~^ ERROR static items are not allowed to have owned pointers
//~^^ ERROR static items are not allowed to have owned pointers
static mut STATIC17: SafeEnum = Variant1;
//~^ ERROR mutable static items are not allowed to have destructors
static STATIC18: @SafeStruct = @SafeStruct{field1: Variant1, field2: Variant2(0)};
//~^ ERROR static items are not allowed to have managed pointers
static STATIC19: ~int = box 3;
//~^ ERROR static items are not allowed to have owned pointers
struct StructNoFreeze<'a> {
nf: &'a int
}
enum EnumNoFreeze<'a> {
FreezableVariant,
NonFreezableVariant(StructNoFreeze<'a>)
}
static STATIC20: StructNoFreeze<'static> = StructNoFreeze{nf: &'static mut 4};
//~^ ERROR immutable static items must be `Freeze`
static STATIC21: EnumNoFreeze<'static> = FreezableVariant;
static STATIC22: EnumNoFreeze<'static> = NonFreezableVariant(StructNoFreeze{nf: &'static mut 4});
//~^ ERROR immutable static items must be `Freeze`
struct NFMarker {
nf: marker::NoFreeze
}
static STATIC23: NFMarker = NFMarker{nf: marker::NoFreeze};
//~^ ERROR immutable static items must be `Freeze`
pub fn main() {
let y = { static x: ~int = ~3; x };
//~^ ERROR static items are not allowed to have owned pointers
}

View File

@ -10,6 +10,6 @@
#[feature(managed_boxes)];
static x: ~[int] = ~[123, 456]; //~ ERROR: cannot allocate vectors in constant expressions
static x: ~[int] = ~[123, 456]; //~ ERROR: static items are not allowed to have owned pointers
fn main() {}

View File

@ -0,0 +1,19 @@
// 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.
#[feature(managed_boxes)];
use std::cell::RefCell;
// Regresion test for issue 7364
static managed: @RefCell<int> = @RefCell::new(0);
//~^ ERROR static items are not allowed to have managed pointers
fn main() { }

View File

@ -0,0 +1,23 @@
// 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.
// Regresion test for issue 9243
struct Test {
mem: int,
}
pub static g_test: Test = Test {mem: 0}; //~ ERROR static items are not allowed to have destructors
impl Drop for Test {
fn drop(&mut self) {}
}
fn main() {}

View File

@ -0,0 +1,29 @@
// 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.
// Verifies that static items can't be moved
use std::kinds::marker;
struct Foo {
foo: int,
nopod: marker::NoPod
}
static BAR: Foo = Foo{foo: 5, nopod: marker::NoPod};
fn test(f: Foo) {
let _f = Foo{foo: 4, ..f};
}
fn main() {
test(BAR); //~ ERROR cannot move out of static item
}

View File

@ -8,6 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
static mut a: ~int = ~3; //~ ERROR: cannot do allocations in constant expressions
static mut a: ~int = ~3; //~ ERROR: mutable static items are not allowed to have owned pointers
fn main() {}

View File

@ -16,13 +16,13 @@ use std::sync::atomics::*;
use std::ptr;
fn main() {
let x = INIT_ATOMIC_FLAG;
let x = INIT_ATOMIC_FLAG; //~ ERROR cannot move out of static item
let x = *&x; //~ ERROR: cannot move out of dereference
let x = INIT_ATOMIC_BOOL;
let x = INIT_ATOMIC_BOOL; //~ ERROR cannot move out of static item
let x = *&x; //~ ERROR: cannot move out of dereference
let x = INIT_ATOMIC_INT;
let x = INIT_ATOMIC_INT; //~ ERROR cannot move out of static item
let x = *&x; //~ ERROR: cannot move out of dereference
let x = INIT_ATOMIC_UINT;
let x = INIT_ATOMIC_UINT; //~ ERROR cannot move out of static item
let x = *&x; //~ ERROR: cannot move out of dereference
let x: AtomicPtr<uint> = AtomicPtr::new(ptr::mut_null());
let x = *&x; //~ ERROR: cannot move out of dereference

View File

@ -14,8 +14,8 @@ fn foo() -> int {
struct Bar<'a> { f: 'a || -> int }
static b : Bar<'static> = Bar { f: foo };
static mut b : Bar<'static> = Bar { f: foo };
pub fn main() {
assert_eq!((b.f)(), 0xca7f000d);
unsafe { assert_eq!((b.f)(), 0xca7f000d); }
}

View File

@ -18,12 +18,14 @@
fn f() { }
static bare_fns: &'static [fn()] = &[f, f];
struct S<'a>('a ||);
static closures: &'static [S<'static>] = &[S(f), S(f)];
static mut closures: &'static [S<'static>] = &[S(f), S(f)];
pub fn main() {
for &bare_fn in bare_fns.iter() { bare_fn() }
for closure in closures.iter() {
let S(ref closure) = *closure;
(*closure)()
unsafe {
for &bare_fn in bare_fns.iter() { bare_fn() }
for closure in closures.iter() {
let S(ref closure) = *closure;
(*closure)()
}
}
}

View File

@ -23,22 +23,23 @@ use std::io::process::{Process, ProcessOutput};
#[test]
fn test_destroy_once() {
#[cfg(not(target_os="android"))]
static PROG: &'static str = "echo";
#[cfg(target_os="android")]
static PROG: &'static str = "ls"; // android don't have echo binary
static mut PROG: &'static str = "echo";
let mut p = Process::new(PROG, []).unwrap();
#[cfg(target_os="android")]
static mut PROG: &'static str = "ls"; // android don't have echo binary
let mut p = unsafe {Process::new(PROG, []).unwrap()};
p.signal_exit().unwrap(); // this shouldn't crash (and nor should the destructor)
}
#[test]
fn test_destroy_twice() {
#[cfg(not(target_os="android"))]
static PROG: &'static str = "echo";
static mut PROG: &'static str = "echo";
#[cfg(target_os="android")]
static PROG: &'static str = "ls"; // android don't have echo binary
static mut PROG: &'static str = "ls"; // android don't have echo binary
let mut p = match Process::new(PROG, []) {
let mut p = match unsafe{Process::new(PROG, [])} {
Ok(p) => p,
Err(e) => fail!("wut: {}", e),
};
@ -49,13 +50,13 @@ fn test_destroy_twice() {
fn test_destroy_actually_kills(force: bool) {
#[cfg(unix,not(target_os="android"))]
static BLOCK_COMMAND: &'static str = "cat";
static mut BLOCK_COMMAND: &'static str = "cat";
#[cfg(unix,target_os="android")]
static BLOCK_COMMAND: &'static str = "/system/bin/cat";
static mut BLOCK_COMMAND: &'static str = "/system/bin/cat";
#[cfg(windows)]
static BLOCK_COMMAND: &'static str = "cmd";
static mut BLOCK_COMMAND: &'static str = "cmd";
#[cfg(unix,not(target_os="android"))]
fn process_exists(pid: libc::pid_t) -> bool {
@ -91,7 +92,7 @@ fn test_destroy_actually_kills(force: bool) {
}
// this process will stay alive indefinitely trying to read from stdin
let mut p = Process::new(BLOCK_COMMAND, []).unwrap();
let mut p = unsafe {Process::new(BLOCK_COMMAND, []).unwrap()};
assert!(process_exists(p.id()));

View File

@ -15,6 +15,6 @@
extern crate issue6919_3;
pub fn main() {
issue6919_3::D.k;
let _ = issue6919_3::D.k;
}