]> git.lizzy.rs Git - rust.git/blobdiff - src/librustc_mir/transform/check_unsafety.rs
Various minor/cosmetic improvements to code
[rust.git] / src / librustc_mir / transform / check_unsafety.rs
index 3404772f8255f4c31932fe62d1df50fa738f9d2f..6af29b74c1c4fab1e535a8b8ee492e47202b66ad 100644 (file)
@@ -23,6 +23,9 @@
 
 use syntax::ast;
 use syntax::symbol::Symbol;
+use syntax::feature_gate::{emit_feature_err, GateIssue};
+
+use std::ops::Bound;
 
 use util;
 
@@ -34,6 +37,7 @@ pub struct UnsafetyChecker<'a, 'tcx: 'a> {
     source_info: SourceInfo,
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     param_env: ty::ParamEnv<'tcx>,
+    /// mark an `unsafe` block as used, so we don't lint it
     used_unsafe: FxHashSet<ast::NodeId>,
     inherited_blocks: Vec<(ast::NodeId, bool)>,
 }
@@ -93,7 +97,7 @@ fn visit_terminator(&mut self,
                 if let hir::Unsafety::Unsafe = sig.unsafety() {
                     self.require_unsafe("call to unsafe function",
                         "consult the function's documentation for information on how to avoid \
-                         undefined behavior")
+                         undefined behavior", UnsafetyViolationKind::GatedConstFnCall)
                 }
             }
         }
@@ -121,7 +125,8 @@ fn visit_statement(&mut self,
 
             StatementKind::InlineAsm { .. } => {
                 self.require_unsafe("use of inline assembly",
-                    "inline assembly is entirely unchecked and can cause undefined behavior")
+                    "inline assembly is entirely unchecked and can cause undefined behavior",
+                    UnsafetyViolationKind::General)
             },
         }
         self.super_statement(block, statement, location);
@@ -134,8 +139,18 @@ fn visit_rvalue(&mut self,
         if let &Rvalue::Aggregate(box ref aggregate, _) = rvalue {
             match aggregate {
                 &AggregateKind::Array(..) |
-                &AggregateKind::Tuple |
-                &AggregateKind::Adt(..) => {}
+                &AggregateKind::Tuple => {}
+                &AggregateKind::Adt(ref def, ..) => {
+                    match self.tcx.layout_scalar_valid_range(def.did) {
+                        (Bound::Unbounded, Bound::Unbounded) => {},
+                        _ => self.require_unsafe(
+                            "initializing type with `rustc_layout_scalar_valid_range` attr",
+                            "initializing a layout restricted type's field with a value outside \
+                            the valid range is undefined behavior",
+                            UnsafetyViolationKind::GeneralAndConstFn,
+                        ),
+                    }
+                }
                 &AggregateKind::Closure(def_id, _) |
                 &AggregateKind::Generator(def_id, _, _) => {
                     let UnsafetyCheckResult {
@@ -152,28 +167,43 @@ fn visit_place(&mut self,
                     place: &Place<'tcx>,
                     context: PlaceContext<'tcx>,
                     location: Location) {
-        if context.is_borrow() {
-            if util::is_disaligned(self.tcx, self.mir, self.param_env, place) {
-                let source_info = self.source_info;
-                let lint_root =
-                    self.source_scope_local_data[source_info.scope].lint_root;
-                self.register_violations(&[UnsafetyViolation {
-                    source_info,
-                    description: Symbol::intern("borrow of packed field").as_interned_str(),
-                    details:
-                        Symbol::intern("fields of packed structs might be misaligned: \
-                                        dereferencing a misaligned pointer or even just creating a \
-                                        misaligned reference is undefined behavior")
-                            .as_interned_str(),
-                    kind: UnsafetyViolationKind::BorrowPacked(lint_root)
-                }], &[]);
-            }
-        }
-
         match place {
             &Place::Projection(box Projection {
                 ref base, ref elem
             }) => {
+                if context.is_borrow() {
+                    if util::is_disaligned(self.tcx, self.mir, self.param_env, place) {
+                        let source_info = self.source_info;
+                        let lint_root =
+                            self.source_scope_local_data[source_info.scope].lint_root;
+                        self.register_violations(&[UnsafetyViolation {
+                            source_info,
+                            description: Symbol::intern("borrow of packed field").as_interned_str(),
+                            details:
+                                Symbol::intern("fields of packed structs might be misaligned: \
+                                                dereferencing a misaligned pointer or even just \
+                                                creating a misaligned reference is undefined \
+                                                behavior")
+                                    .as_interned_str(),
+                            kind: UnsafetyViolationKind::BorrowPacked(lint_root)
+                        }], &[]);
+                    }
+                }
+                let is_borrow_of_interior_mut = context.is_borrow() && !base
+                    .ty(self.mir, self.tcx)
+                    .to_ty(self.tcx)
+                    .is_freeze(self.tcx, self.param_env, self.source_info.span);
+                // prevent
+                // * `&mut x.field`
+                // * `x.field = y;`
+                // * `&x.field` if `field`'s type has interior mutability
+                // because either of these would allow modifying the layout constrained field and
+                // insert values that violate the layout constraints.
+                if context.is_mutating_use() || is_borrow_of_interior_mut {
+                    self.check_mut_borrowing_layout_constrained_field(
+                        place, context.is_mutating_use(),
+                    );
+                }
                 let old_source_info = self.source_info;
                 if let &Place::Local(local) = base {
                     if self.mir.local_decls[local].internal {
@@ -189,7 +219,7 @@ fn visit_place(&mut self,
                         self.require_unsafe("dereference of raw pointer",
                             "raw pointers may be NULL, dangling or unaligned; they can violate \
                              aliasing rules and cause data races: all of these are undefined \
-                             behavior")
+                             behavior", UnsafetyViolationKind::General)
                     }
                     ty::Adt(adt, _) => {
                         if adt.is_union() {
@@ -212,14 +242,15 @@ fn visit_place(&mut self,
                                         "assignment to non-`Copy` union field",
                                         "the previous content of the field will be dropped, which \
                                          causes undefined behavior if the field was not properly \
-                                         initialized")
+                                         initialized", UnsafetyViolationKind::General)
                                 } else {
                                     // write to non-move union, safe
                                 }
                             } else {
                                 self.require_unsafe("access to union field",
                                     "the field may not be properly initialized: using \
-                                     uninitialized data will cause undefined behavior")
+                                     uninitialized data will cause undefined behavior",
+                                     UnsafetyViolationKind::General)
                             }
                         }
                     }
@@ -237,7 +268,8 @@ fn visit_place(&mut self,
                 if self.tcx.is_static(def_id) == Some(hir::Mutability::MutMutable) {
                     self.require_unsafe("use of mutable static",
                         "mutable statics can be mutated by multiple threads: aliasing violations \
-                         or data races will cause undefined behavior");
+                         or data races will cause undefined behavior",
+                         UnsafetyViolationKind::General);
                 } else if self.tcx.is_foreign_item(def_id) {
                     let source_info = self.source_info;
                     let lint_root =
@@ -260,45 +292,96 @@ fn visit_place(&mut self,
 }
 
 impl<'a, 'tcx> UnsafetyChecker<'a, 'tcx> {
-    fn require_unsafe(&mut self,
-                      description: &'static str,
-                      details: &'static str)
-    {
+    fn require_unsafe(
+        &mut self,
+        description: &'static str,
+        details: &'static str,
+        kind: UnsafetyViolationKind,
+    ) {
         let source_info = self.source_info;
         self.register_violations(&[UnsafetyViolation {
             source_info,
             description: Symbol::intern(description).as_interned_str(),
             details: Symbol::intern(details).as_interned_str(),
-            kind: UnsafetyViolationKind::General,
+            kind,
         }], &[]);
     }
 
     fn register_violations(&mut self,
                            violations: &[UnsafetyViolation],
                            unsafe_blocks: &[(ast::NodeId, bool)]) {
-        if self.min_const_fn {
-            for violation in violations {
-                let mut violation = violation.clone();
-                violation.kind = UnsafetyViolationKind::MinConstFn;
-                if !self.violations.contains(&violation) {
-                    self.violations.push(violation)
-                }
-            }
-        }
-        let within_unsafe = match self.source_scope_local_data[self.source_info.scope].safety {
-            Safety::Safe => {
+        let safety = self.source_scope_local_data[self.source_info.scope].safety;
+        let within_unsafe = match (safety, self.min_const_fn) {
+            // Erring on the safe side, pun intended
+            (Safety::BuiltinUnsafe, true) |
+            // mir building encodes const fn bodies as safe, even for `const unsafe fn`
+            (Safety::FnUnsafe, true) => bug!("const unsafe fn body treated as inherently unsafe"),
+            // `unsafe` blocks are required in safe code
+            (Safety::Safe, _) => {
                 for violation in violations {
-                    if !self.violations.contains(violation) {
-                        self.violations.push(violation.clone())
+                    let mut violation = violation.clone();
+                    match violation.kind {
+                        UnsafetyViolationKind::GeneralAndConstFn |
+                        UnsafetyViolationKind::General => {},
+                        UnsafetyViolationKind::BorrowPacked(_) |
+                        UnsafetyViolationKind::ExternStatic(_) => if self.min_const_fn {
+                            // const fns don't need to be backwards compatible and can
+                            // emit these violations as a hard error instead of a backwards
+                            // compat lint
+                            violation.kind = UnsafetyViolationKind::General;
+                        },
+                        UnsafetyViolationKind::GatedConstFnCall => {
+                            // safe code can't call unsafe const fns, this `UnsafetyViolationKind`
+                            // is only relevant for `Safety::ExplicitUnsafe` in `unsafe const fn`s
+                            violation.kind = UnsafetyViolationKind::General;
+                        }
+                    }
+                    if !self.violations.contains(&violation) {
+                        self.violations.push(violation)
                     }
                 }
                 false
             }
-            Safety::BuiltinUnsafe | Safety::FnUnsafe => true,
-            Safety::ExplicitUnsafe(node_id) => {
+            // regular `unsafe` function bodies allow unsafe without additional unsafe blocks
+            (Safety::BuiltinUnsafe, false) | (Safety::FnUnsafe, false) => true,
+            (Safety::ExplicitUnsafe(node_id), _) => {
+                // mark unsafe block as used if there are any unsafe operations inside
                 if !violations.is_empty() {
                     self.used_unsafe.insert(node_id);
                 }
+                // only some unsafety is allowed in const fn
+                if self.min_const_fn {
+                    let min_const_unsafe_fn = self.tcx.features().min_const_unsafe_fn;
+                    for violation in violations {
+                        match violation.kind {
+                            UnsafetyViolationKind::GatedConstFnCall if min_const_unsafe_fn => {
+                                // these function calls to unsafe functions are allowed
+                                // if `#![feature(min_const_unsafe_fn)]` is active
+                            },
+                            UnsafetyViolationKind::GatedConstFnCall => {
+                                // without the feature gate, we report errors
+                                if !self.violations.contains(&violation) {
+                                    self.violations.push(violation.clone())
+                                }
+                            }
+                            // these unsafe things are stable in const fn
+                            UnsafetyViolationKind::GeneralAndConstFn => {},
+                            // these things are forbidden in const fns
+                            UnsafetyViolationKind::General |
+                            UnsafetyViolationKind::BorrowPacked(_) |
+                            UnsafetyViolationKind::ExternStatic(_) => {
+                                let mut violation = violation.clone();
+                                // const fns don't need to be backwards compatible and can
+                                // emit these violations as a hard error instead of a backwards
+                                // compat lint
+                                violation.kind = UnsafetyViolationKind::General;
+                                if !self.violations.contains(&violation) {
+                                    self.violations.push(violation)
+                                }
+                            },
+                        }
+                    }
+                }
                 true
             }
         };
@@ -306,6 +389,53 @@ fn register_violations(&mut self,
             (node_id, is_used && !within_unsafe)
         }));
     }
+    fn check_mut_borrowing_layout_constrained_field(
+        &mut self,
+        mut place: &Place<'tcx>,
+        is_mut_use: bool,
+    ) {
+        while let &Place::Projection(box Projection {
+            ref base, ref elem
+        }) = place {
+            match *elem {
+                ProjectionElem::Field(..) => {
+                    let ty = base.ty(&self.mir.local_decls, self.tcx).to_ty(self.tcx);
+                    match ty.sty {
+                        ty::Adt(def, _) => match self.tcx.layout_scalar_valid_range(def.did) {
+                            (Bound::Unbounded, Bound::Unbounded) => {},
+                            _ => {
+                                let (description, details) = if is_mut_use {
+                                    (
+                                        "mutation of layout constrained field",
+                                        "mutating layout constrained fields cannot statically be \
+                                        checked for valid values",
+                                    )
+                                } else {
+                                    (
+                                        "borrow of layout constrained field with interior \
+                                        mutability",
+                                        "references to fields of layout constrained fields \
+                                        lose the constraints. Coupled with interior mutability, \
+                                        the field can be changed to invalid values",
+                                    )
+                                };
+                                let source_info = self.source_info;
+                                self.register_violations(&[UnsafetyViolation {
+                                    source_info,
+                                    description: Symbol::intern(description).as_interned_str(),
+                                    details: Symbol::intern(details).as_interned_str(),
+                                    kind: UnsafetyViolationKind::GeneralAndConstFn,
+                                }], &[]);
+                            }
+                        },
+                        _ => {}
+                    }
+                }
+                _ => {}
+            }
+            place = base;
+        }
+    }
 }
 
 pub(crate) fn provide(providers: &mut Providers) {
@@ -343,8 +473,8 @@ fn check_unused_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                                  unsafe_blocks: &'a mut Vec<(ast::NodeId, bool)>)
 {
     let body_id =
-        tcx.hir.as_local_node_id(def_id).and_then(|node_id| {
-            tcx.hir.maybe_body_owned_by(node_id)
+        tcx.hir().as_local_node_id(def_id).and_then(|node_id| {
+            tcx.hir().maybe_body_owned_by(node_id)
         });
 
     let body_id = match body_id {
@@ -354,7 +484,7 @@ fn check_unused_unsafe<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
             return
         }
     };
-    let body = tcx.hir.body(body_id);
+    let body = tcx.hir().body(body_id);
     debug!("check_unused_unsafe({:?}, body={:?}, used_unsafe={:?})",
            def_id, body, used_unsafe);
 
@@ -367,7 +497,7 @@ fn unsafety_check_result<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId)
 {
     debug!("unsafety_violations({:?})", def_id);
 
-    // NB: this borrow is valid because all the consumers of
+    // N.B., this borrow is valid because all the consumers of
     // `mir_built` force this.
     let mir = &tcx.mir_built(def_id).borrow();
 
@@ -384,7 +514,7 @@ fn unsafety_check_result<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId)
 
     let param_env = tcx.param_env(def_id);
     let mut checker = UnsafetyChecker::new(
-        tcx.is_const_fn(def_id) && tcx.is_min_const_fn(def_id),
+        tcx.is_min_const_fn(def_id),
         mir, source_scope_local_data, tcx, param_env);
     checker.visit_mir(mir);
 
@@ -396,7 +526,7 @@ fn unsafety_check_result<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId)
 }
 
 fn unsafe_derive_on_repr_packed<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
-    let lint_node_id = match tcx.hir.as_local_node_id(def_id) {
+    let lint_node_id = match tcx.hir().as_local_node_id(def_id) {
         Some(node_id) => node_id,
         None => bug!("checking unsafety for non-local def id {:?}", def_id)
     };
@@ -420,14 +550,14 @@ fn unsafe_derive_on_repr_packed<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: D
 fn is_enclosed(tcx: TyCtxt,
                used_unsafe: &FxHashSet<ast::NodeId>,
                id: ast::NodeId) -> Option<(String, ast::NodeId)> {
-    let parent_id = tcx.hir.get_parent_node(id);
+    let parent_id = tcx.hir().get_parent_node(id);
     if parent_id != id {
         if used_unsafe.contains(&parent_id) {
             Some(("block".to_string(), parent_id))
         } else if let Some(Node::Item(&hir::Item {
             node: hir::ItemKind::Fn(_, header, _, _),
             ..
-        })) = tcx.hir.find(parent_id) {
+        })) = tcx.hir().find(parent_id) {
             match header.unsafety {
                 hir::Unsafety::Unsafe => Some(("fn".to_string(), parent_id)),
                 hir::Unsafety::Normal => None,
@@ -441,12 +571,12 @@ fn is_enclosed(tcx: TyCtxt,
 }
 
 fn report_unused_unsafe(tcx: TyCtxt, used_unsafe: &FxHashSet<ast::NodeId>, id: ast::NodeId) {
-    let span = tcx.sess.source_map().def_span(tcx.hir.span(id));
+    let span = tcx.sess.source_map().def_span(tcx.hir().span(id));
     let msg = "unnecessary `unsafe` block";
     let mut db = tcx.struct_span_lint_node(UNUSED_UNSAFE, id, span, msg);
     db.span_label(span, msg);
     if let Some((kind, id)) = is_enclosed(tcx, used_unsafe, id) {
-        db.span_label(tcx.sess.source_map().def_span(tcx.hir.span(id)),
+        db.span_label(tcx.sess.source_map().def_span(tcx.hir().span(id)),
                       format!("because it's nested under this `unsafe` {}", kind));
     }
     db.emit();
@@ -486,6 +616,22 @@ pub fn check_unsafety<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
     } in violations.iter() {
         // Report an error.
         match kind {
+            UnsafetyViolationKind::General if tcx.is_min_const_fn(def_id) => {
+                let mut err = tcx.sess.struct_span_err(
+                    source_info.span,
+                    &format!("{} is unsafe and unsafe operations \
+                            are not allowed in const fn", description));
+                err.span_label(source_info.span, &description.as_str()[..])
+                    .note(&details.as_str()[..]);
+                if tcx.fn_sig(def_id).unsafety() == hir::Unsafety::Unsafe {
+                    err.note(
+                        "unsafe action within a `const unsafe fn` still require an `unsafe` \
+                        block in contrast to regular `unsafe fn`."
+                    );
+                }
+                err.emit();
+            }
+            UnsafetyViolationKind::GeneralAndConstFn |
             UnsafetyViolationKind::General => {
                 struct_span_err!(
                     tcx.sess, source_info.span, E0133,
@@ -494,14 +640,15 @@ pub fn check_unsafety<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
                     .note(&details.as_str()[..])
                     .emit();
             }
-            UnsafetyViolationKind::MinConstFn => {
-                tcx.sess.struct_span_err(
+            UnsafetyViolationKind::GatedConstFnCall => {
+                emit_feature_err(
+                    &tcx.sess.parse_sess,
+                    "min_const_unsafe_fn",
                     source_info.span,
-                    &format!("{} is unsafe and unsafe operations \
-                            are not allowed in const fn", description))
-                    .span_label(source_info.span, &description.as_str()[..])
-                    .note(&details.as_str()[..])
-                    .emit();
+                    GateIssue::Language,
+                    "calls to `const unsafe fn` in const fns are unstable",
+                );
+
             }
             UnsafetyViolationKind::ExternStatic(lint_node_id) => {
                 tcx.lint_node_note(SAFE_EXTERN_STATICS,