From 1829fa5199bae5a192c771807c532badce14be37 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 5 Jun 2015 08:31:27 +0200 Subject: [PATCH] Hack for "unsafety hygiene" -- `push_unsafe!` and `pop_unsafe!`. Even after expansion, the generated expressions still track depth of such pushes (i.e. how often you have "pushed" without a corresponding "pop"), and we add a rule that in a context with a positive `push_unsafe!` depth, it is effectively an `unsafe` block context. (This way, we can inject code that uses `unsafe` features, but still contains within it a sub-expression that should inherit the outer safety checking setting, outside of the injected code.) This is a total hack; it not only needs a feature-gate, but probably should be feature-gated forever (if possible). ignore-pretty in test/run-pass/pushpop-unsafe-okay.rs --- src/librustc/middle/effect.rs | 37 ++++++-- src/librustc_typeck/check/mod.rs | 20 ++-- src/libsyntax/ast.rs | 2 + src/libsyntax/ext/base.rs | 6 ++ src/libsyntax/ext/expand.rs | 1 + src/libsyntax/ext/pushpop_safe.rs | 94 +++++++++++++++++++ src/libsyntax/feature_gate.rs | 11 +++ src/libsyntax/lib.rs | 1 + src/libsyntax/print/pprust.rs | 4 +- .../feature-gate-pushpop-unsafe.rs | 14 +++ .../compile-fail/pushpop-unsafe-rejects.rs | 71 ++++++++++++++ src/test/run-pass/pushpop-unsafe-okay.rs | 56 +++++++++++ 12 files changed, 301 insertions(+), 16 deletions(-) create mode 100644 src/libsyntax/ext/pushpop_safe.rs create mode 100644 src/test/compile-fail/feature-gate-pushpop-unsafe.rs create mode 100644 src/test/compile-fail/pushpop-unsafe-rejects.rs create mode 100644 src/test/run-pass/pushpop-unsafe-okay.rs diff --git a/src/librustc/middle/effect.rs b/src/librustc/middle/effect.rs index b2a064bf86c6a..a2e42245bbef8 100644 --- a/src/librustc/middle/effect.rs +++ b/src/librustc/middle/effect.rs @@ -10,7 +10,7 @@ //! Enforces the Rust effect system. Currently there is just one effect, //! `unsafe`. -use self::UnsafeContext::*; +use self::RootUnsafeContext::*; use middle::def; use middle::ty::{self, Ty}; @@ -21,8 +21,20 @@ use syntax::codemap::Span; use syntax::visit; use syntax::visit::Visitor; +#[derive(Copy, Clone)] +struct UnsafeContext { + push_unsafe_count: usize, + root: RootUnsafeContext, +} + +impl UnsafeContext { + fn new(root: RootUnsafeContext) -> UnsafeContext { + UnsafeContext { root: root, push_unsafe_count: 0 } + } +} + #[derive(Copy, Clone, PartialEq)] -enum UnsafeContext { +enum RootUnsafeContext { SafeContext, UnsafeFn, UnsafeBlock(ast::NodeId), @@ -44,7 +56,8 @@ struct EffectCheckVisitor<'a, 'tcx: 'a> { impl<'a, 'tcx> EffectCheckVisitor<'a, 'tcx> { fn require_unsafe(&mut self, span: Span, description: &str) { - match self.unsafe_context { + if self.unsafe_context.push_unsafe_count > 0 { return; } + match self.unsafe_context.root { SafeContext => { // Report an error. span_err!(self.tcx.sess, span, E0133, @@ -75,9 +88,9 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> { let old_unsafe_context = self.unsafe_context; if is_unsafe_fn { - self.unsafe_context = UnsafeFn + self.unsafe_context = UnsafeContext::new(UnsafeFn) } else if is_item_fn { - self.unsafe_context = SafeContext + self.unsafe_context = UnsafeContext::new(SafeContext) } visit::walk_fn(self, fn_kind, fn_decl, block, span); @@ -105,10 +118,18 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> { // external blocks (e.g. `unsafe { println("") }`, // expands to `unsafe { ... unsafe { ... } }` where // the inner one is compiler generated). - if self.unsafe_context == SafeContext || source == ast::CompilerGenerated { - self.unsafe_context = UnsafeBlock(block.id) + if self.unsafe_context.root == SafeContext || source == ast::CompilerGenerated { + self.unsafe_context.root = UnsafeBlock(block.id) } } + ast::PushUnsafeBlock(..) => { + self.unsafe_context.push_unsafe_count = + self.unsafe_context.push_unsafe_count.saturating_add(1); + } + ast::PopUnsafeBlock(..) => { + self.unsafe_context.push_unsafe_count = + self.unsafe_context.push_unsafe_count.saturating_sub(1); + } } visit::walk_block(self, block); @@ -162,7 +183,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for EffectCheckVisitor<'a, 'tcx> { pub fn check_crate(tcx: &ty::ctxt) { let mut visitor = EffectCheckVisitor { tcx: tcx, - unsafe_context: SafeContext, + unsafe_context: UnsafeContext::new(SafeContext), }; visit::walk_crate(&mut visitor, tcx.map.krate()); diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index fdeed657e071b..d6afd72d3b4db 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -231,12 +231,13 @@ impl<'tcx> Expectation<'tcx> { pub struct UnsafetyState { pub def: ast::NodeId, pub unsafety: ast::Unsafety, + pub unsafe_push_count: u32, from_fn: bool } impl UnsafetyState { pub fn function(unsafety: ast::Unsafety, def: ast::NodeId) -> UnsafetyState { - UnsafetyState { def: def, unsafety: unsafety, from_fn: true } + UnsafetyState { def: def, unsafety: unsafety, unsafe_push_count: 0, from_fn: true } } pub fn recurse(&mut self, blk: &ast::Block) -> UnsafetyState { @@ -248,13 +249,20 @@ impl UnsafetyState { ast::Unsafety::Unsafe if self.from_fn => *self, unsafety => { - let (unsafety, def) = match blk.rules { - ast::UnsafeBlock(..) => (ast::Unsafety::Unsafe, blk.id), - ast::DefaultBlock => (unsafety, self.def), + let (unsafety, def, count) = match blk.rules { + ast::PushUnsafeBlock(..) => + (unsafety, blk.id, self.unsafe_push_count.saturating_add(1)), + ast::PopUnsafeBlock(..) => + (unsafety, blk.id, self.unsafe_push_count.saturating_sub(1)), + ast::UnsafeBlock(..) => + (ast::Unsafety::Unsafe, blk.id, self.unsafe_push_count), + ast::DefaultBlock => + (unsafety, self.def, self.unsafe_push_count), }; UnsafetyState{ def: def, - unsafety: unsafety, - from_fn: false } + unsafety: unsafety, + unsafe_push_count: count, + from_fn: false } } } } diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index a0059d33bed84..fba9db401dbb1 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -810,6 +810,8 @@ pub type SpannedIdent = Spanned; pub enum BlockCheckMode { DefaultBlock, UnsafeBlock(UnsafeSource), + PushUnsafeBlock(UnsafeSource), + PopUnsafeBlock(UnsafeSource), } #[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)] diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 499562edc0cc9..409ae86db35d4 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -591,6 +591,12 @@ fn initial_syntax_expander_table<'feat>(ecfg: &expand::ExpansionConfig<'feat>) syntax_expanders.insert(intern("cfg"), builtin_normal_expander( ext::cfg::expand_cfg)); + syntax_expanders.insert(intern("push_unsafe"), + builtin_normal_expander( + ext::pushpop_safe::expand_push_unsafe)); + syntax_expanders.insert(intern("pop_unsafe"), + builtin_normal_expander( + ext::pushpop_safe::expand_pop_unsafe)); syntax_expanders.insert(intern("trace_macros"), builtin_normal_expander( ext::trace_macros::expand_trace_macros)); diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 53befc092da88..b540e0adeea87 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -1504,6 +1504,7 @@ impl<'feat> ExpansionConfig<'feat> { fn enable_trace_macros = allow_trace_macros, fn enable_allow_internal_unstable = allow_internal_unstable, fn enable_custom_derive = allow_custom_derive, + fn enable_pushpop_unsafe = allow_pushpop_unsafe, } } diff --git a/src/libsyntax/ext/pushpop_safe.rs b/src/libsyntax/ext/pushpop_safe.rs new file mode 100644 index 0000000000000..fee445cd31af3 --- /dev/null +++ b/src/libsyntax/ext/pushpop_safe.rs @@ -0,0 +1,94 @@ +// Copyright 2015 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. + +/* + * The compiler code necessary to support the `push_unsafe!` and + * `pop_unsafe!` macros. + * + * This is a hack to allow a kind of "safety hygiene", where a macro + * can generate code with an interior expression that inherits the + * safety of some outer context. + * + * For example, in: + * + * ```rust + * fn foo() { push_unsafe!( { EXPR_1; pop_unsafe!( EXPR_2 ) } ) } + * ``` + * + * the `EXPR_1` is considered to be in an `unsafe` context, + * but `EXPR_2` is considered to be in a "safe" (i.e. checked) context. + * + * For comparison, in: + * + * ```rust + * fn foo() { unsafe { push_unsafe!( { EXPR_1; pop_unsafe!( EXPR_2 ) } ) } } + * ``` + * + * both `EXPR_1` and `EXPR_2` are considered to be in `unsafe` + * contexts. + * + */ + +use ast; +use codemap::Span; +use ext::base::*; +use ext::base; +use ext::build::AstBuilder; +use feature_gate; +use ptr::P; + +enum PushPop { Push, Pop } + +pub fn expand_push_unsafe<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) + -> Box { + feature_gate::check_for_pushpop_syntax( + cx.ecfg.features, &cx.parse_sess.span_diagnostic, sp); + expand_pushpop_unsafe(cx, sp, tts, PushPop::Push) +} + +pub fn expand_pop_unsafe<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) + -> Box { + feature_gate::check_for_pushpop_syntax( + cx.ecfg.features, &cx.parse_sess.span_diagnostic, sp); + expand_pushpop_unsafe(cx, sp, tts, PushPop::Pop) +} + +fn expand_pushpop_unsafe<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree], + pp: PushPop) -> Box { + let mut exprs = match get_exprs_from_tts(cx, sp, tts) { + Some(exprs) => exprs.into_iter(), + None => return DummyResult::expr(sp), + }; + let expr = match (exprs.next(), exprs.next()) { + (Some(expr), None) => expr, + _ => { + let msg = match pp { + PushPop::Push => "push_unsafe! takes 1 arguments", + PushPop::Pop => "pop_unsafe! takes 1 arguments", + }; + cx.span_err(sp, msg); + return DummyResult::expr(sp); + } + }; + + let source = ast::UnsafeSource::CompilerGenerated; + let check_mode = match pp { + PushPop::Push => ast::BlockCheckMode::PushUnsafeBlock(source), + PushPop::Pop => ast::BlockCheckMode::PopUnsafeBlock(source), + }; + + MacEager::expr(cx.expr_block(P(ast::Block { + stmts: vec![], + expr: Some(expr), + id: ast::DUMMY_NODE_ID, + rules: check_mode, + span: sp + }))) +} diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index ab8cf9ae6b64f..69d120cff601c 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -80,6 +80,7 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Status)] = &[ ("visible_private_types", "1.0.0", Active), ("slicing_syntax", "1.0.0", Accepted), ("box_syntax", "1.0.0", Active), + ("pushpop_unsafe", "1.2.0", Active), ("on_unimplemented", "1.0.0", Active), ("simd_ffi", "1.0.0", Active), ("allocator", "1.0.0", Active), @@ -325,6 +326,7 @@ pub struct Features { pub allow_trace_macros: bool, pub allow_internal_unstable: bool, pub allow_custom_derive: bool, + pub allow_pushpop_unsafe: bool, pub simd_ffi: bool, pub unmarked_api: bool, pub negate_unsigned: bool, @@ -348,6 +350,7 @@ impl Features { allow_trace_macros: false, allow_internal_unstable: false, allow_custom_derive: false, + allow_pushpop_unsafe: false, simd_ffi: false, unmarked_api: false, negate_unsigned: false, @@ -358,6 +361,13 @@ impl Features { } } +pub fn check_for_pushpop_syntax(f: Option<&Features>, diag: &SpanHandler, span: Span) { + if let Some(&Features { allow_pushpop_unsafe: true, .. }) = f { + return; + } + emit_feature_err(diag, "pushpop_unsafe", span, EXPLAIN_PUSHPOP_UNSAFE); +} + struct Context<'a> { features: Vec<&'static str>, span_handler: &'a SpanHandler, @@ -787,6 +797,7 @@ fn check_crate_inner(cm: &CodeMap, span_handler: &SpanHandler, allow_trace_macros: cx.has_feature("trace_macros"), allow_internal_unstable: cx.has_feature("allow_internal_unstable"), allow_custom_derive: cx.has_feature("custom_derive"), + allow_pushpop_unsafe: cx.has_feature("pushpop_unsafe"), simd_ffi: cx.has_feature("simd_ffi"), unmarked_api: cx.has_feature("unmarked_api"), negate_unsigned: cx.has_feature("negate_unsigned"), diff --git a/src/libsyntax/lib.rs b/src/libsyntax/lib.rs index d93af5da13c1e..5424c0b214a4e 100644 --- a/src/libsyntax/lib.rs +++ b/src/libsyntax/lib.rs @@ -120,6 +120,7 @@ pub mod ext { pub mod log_syntax; pub mod mtwt; pub mod quote; + pub mod pushpop_safe; pub mod source_util; pub mod trace_macros; diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 6693eed6aced5..448857389da61 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -1434,8 +1434,8 @@ impl<'a> State<'a> { attrs: &[ast::Attribute], close_box: bool) -> io::Result<()> { match blk.rules { - ast::UnsafeBlock(..) => try!(self.word_space("unsafe")), - ast::DefaultBlock => () + ast::UnsafeBlock(..) | ast::PushUnsafeBlock(..) => try!(self.word_space("unsafe")), + ast::DefaultBlock | ast::PopUnsafeBlock(..) => () } try!(self.maybe_print_comment(blk.span.lo)); try!(self.ann.pre(self, NodeBlock(blk))); diff --git a/src/test/compile-fail/feature-gate-pushpop-unsafe.rs b/src/test/compile-fail/feature-gate-pushpop-unsafe.rs new file mode 100644 index 0000000000000..e317b4c7d4d2a --- /dev/null +++ b/src/test/compile-fail/feature-gate-pushpop-unsafe.rs @@ -0,0 +1,14 @@ +// Copyright 2015 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. + +fn main() { + let c = push_unsafe!('c'); //~ ERROR push/pop_unsafe macros are experimental + let c = pop_unsafe!('c'); //~ ERROR push/pop_unsafe macros are experimental +} diff --git a/src/test/compile-fail/pushpop-unsafe-rejects.rs b/src/test/compile-fail/pushpop-unsafe-rejects.rs new file mode 100644 index 0000000000000..b009a670da1e1 --- /dev/null +++ b/src/test/compile-fail/pushpop-unsafe-rejects.rs @@ -0,0 +1,71 @@ +// Copyright 2012 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. + +// Basic sanity check for `push_unsafe!(EXPR)` and +// `pop_unsafe!(EXPR)`: we can call unsafe code when there are a +// positive number of pushes in the stack, or if we are within a +// normal `unsafe` block, but otherwise cannot. + +static mut X: i32 = 0; + +unsafe fn f() { X += 1; return; } +fn g() { unsafe { X += 1_000; } return; } + +fn main() { + push_unsafe!( { + f(); pop_unsafe!({ + f() //~ ERROR: call to unsafe function + }) + } ); + + push_unsafe!({ + f(); + pop_unsafe!({ + g(); + f(); //~ ERROR: call to unsafe function + }) + } ); + + push_unsafe!({ + g(); pop_unsafe!({ + unsafe { + f(); + } + f(); //~ ERROR: call to unsafe function + }) + }); + + + // Note: For implementation simplicity I have chosen to just have + // the stack do "saturated pop", but perhaps we would prefer to + // have cases like these two here be errors: + + pop_unsafe!{ g() }; + + push_unsafe!({ + pop_unsafe!(pop_unsafe!{ g() }) + }); + + + // Okay, back to examples that do error, even in the presence of + // "saturated pop" + + push_unsafe!({ + g(); + pop_unsafe!(pop_unsafe!({ + f() //~ ERROR: call to unsafe function + })) + }); + + pop_unsafe!({ + f(); //~ ERROR: call to unsafe function + }) + +} diff --git a/src/test/run-pass/pushpop-unsafe-okay.rs b/src/test/run-pass/pushpop-unsafe-okay.rs new file mode 100644 index 0000000000000..fc402d4136888 --- /dev/null +++ b/src/test/run-pass/pushpop-unsafe-okay.rs @@ -0,0 +1,56 @@ +// Copyright 2015 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. + +// Basic sanity check for `push_unsafe!(EXPR)` and +// `pop_unsafe!(EXPR)`: we can call unsafe code when there are a +// positive number of pushes in the stack, or if we are within a +// normal `unsafe` block, but otherwise cannot. + +// ignore-pretty because the `push_unsafe!` and `pop_unsafe!` macros +// are not integrated with the pretty-printer. + +#![feature(pushpop_unsafe)] + +static mut X: i32 = 0; + +unsafe fn f() { X += 1; return; } +fn g() { unsafe { X += 1_000; } return; } + +fn check_reset_x(x: i32) -> bool { + #![allow(unused_parens)] // dont you judge my style choices! + unsafe { + let ret = (x == X); + X = 0; + ret + } +} + +fn main() { + // double-check test infrastructure + assert!(check_reset_x(0)); + unsafe { f(); } + assert!(check_reset_x(1)); + assert!(check_reset_x(0)); + { g(); } + assert!(check_reset_x(1000)); + assert!(check_reset_x(0)); + unsafe { f(); g(); g(); } + assert!(check_reset_x(2001)); + + push_unsafe!( { f(); pop_unsafe!( g() ) } ); + assert!(check_reset_x(1_001)); + push_unsafe!( { g(); pop_unsafe!( unsafe { f(); f(); } ) } ); + assert!(check_reset_x(1_002)); + + unsafe { push_unsafe!( { f(); pop_unsafe!( { f(); f(); } ) } ); } + assert!(check_reset_x(3)); + push_unsafe!( { f(); push_unsafe!( { pop_unsafe!( { f(); f(); f(); } ) } ); } ); + assert!(check_reset_x(4)); +}