From ee2f001a42a10b74e5a9fbb5bda175d350ebc3ff Mon Sep 17 00:00:00 2001 From: Flavio Percoco Date: Wed, 26 Feb 2014 19:22:41 +0100 Subject: [PATCH] Forbid certain types for static items - 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) - does not contain a struct literal or call to an enum variant / struct constructor where - the type of the struct/enum has a dtor --- src/librustc/driver/driver.rs | 3 + src/librustc/lib.rs | 1 + src/librustc/middle/check_static.rs | 138 ++++++++++++++++++ src/librustc/middle/ty.rs | 8 + .../check-static-values-constraints.rs | 128 ++++++++++++++++ src/test/compile-fail/issue-10487.rs | 2 +- 6 files changed, 279 insertions(+), 1 deletion(-) create mode 100644 src/librustc/middle/check_static.rs create mode 100644 src/test/compile-fail/check-static-values-constraints.rs diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index 5f9910340c486..4c552acc93692 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -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)); diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index 9806c39345ba7..97718849e631f 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -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; diff --git a/src/librustc/middle/check_static.rs b/src/librustc/middle/check_static.rs new file mode 100644 index 0000000000000..86078c4023a13 --- /dev/null +++ b/src/librustc/middle/check_static.rs @@ -0,0 +1,138 @@ +// 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 or the MIT license +// , 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 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"); + } + _ => { + 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; + } + } + _ => {} + } + visit::walk_expr(self, e, is_const); + } + } + } +} diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index a52b1c62923c5..8ea7f75d2b44e 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -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) } @@ -2042,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) } diff --git a/src/test/compile-fail/check-static-values-constraints.rs b/src/test/compile-fail/check-static-values-constraints.rs new file mode 100644 index 0000000000000..1248f56d4b88f --- /dev/null +++ b/src/test/compile-fail/check-static-values-constraints.rs @@ -0,0 +1,128 @@ +// 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 or the MIT license +// , 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. + +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 + +pub fn main() { + let y = { static x: ~int = ~3; x }; + //~^ ERROR static items are not allowed to have owned pointers +} diff --git a/src/test/compile-fail/issue-10487.rs b/src/test/compile-fail/issue-10487.rs index 3fc6106f7480b..01fb2ea942737 100644 --- a/src/test/compile-fail/issue-10487.rs +++ b/src/test/compile-fail/issue-10487.rs @@ -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() {}