Implement Mutation- and BorrowOfLayoutConstrainedField in thir-unsafeck

Author: FabianWolff

compiler/rustc_mir_build/src/build/expr/mod.rs

@@ -65,6 +65,6 @@ mod as_operand;
 pub mod as_place;
 mod as_rvalue;
 mod as_temp;
-mod category;
+pub mod category;
 mod into;
 mod stmt;

compiler/rustc_mir_build/src/build/mod.rs

@@ -1131,3 +1131,5 @@ mod expr;
 mod matches;
 mod misc;
 mod scope;
+
+pub(crate) use expr::category::Category as ExprCategory;

compiler/rustc_mir_build/src/check_unsafety.rs

@@ -1,9 +1,11 @@
+use crate::build::ExprCategory;
 use crate::thir::visit::{self, Visitor};
 
 use rustc_errors::struct_span_err;
 use rustc_hir as hir;
+use rustc_middle::mir::BorrowKind;
 use rustc_middle::thir::*;
-use rustc_middle::ty::{self, TyCtxt};
+use rustc_middle::ty::{self, ParamEnv, TyCtxt};
 use rustc_session::lint::builtin::{UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE};
 use rustc_session::lint::Level;
 use rustc_span::def_id::{DefId, LocalDefId};
@@ -28,6 +30,8 @@ struct UnsafetyVisitor<'a, 'tcx> {
     is_const: bool,
     in_possible_lhs_union_assign: bool,
     in_union_destructure: bool,
+    param_env: ParamEnv<'tcx>,
+    inside_adt: bool,
 }
 
 impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
@@ -134,6 +138,50 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
     }
 }
 
+// Searches for accesses to layout constrained fields.
+struct LayoutConstrainedPlaceVisitor<'a, 'tcx> {
+    found: bool,
+    thir: &'a Thir<'tcx>,
+    tcx: TyCtxt<'tcx>,
+}
+
+impl<'a, 'tcx> LayoutConstrainedPlaceVisitor<'a, 'tcx> {
+    fn new(thir: &'a Thir<'tcx>, tcx: TyCtxt<'tcx>) -> Self {
+        Self { found: false, thir, tcx }
+    }
+}
+
+impl<'a, 'tcx> Visitor<'a, 'tcx> for LayoutConstrainedPlaceVisitor<'a, 'tcx> {
+    fn thir(&self) -> &'a Thir<'tcx> {
+        self.thir
+    }
+
+    fn visit_expr(&mut self, expr: &Expr<'tcx>) {
+        match expr.kind {
+            ExprKind::Field { lhs, .. } => {
+                if let ty::Adt(adt_def, _) = self.thir[lhs].ty.kind() {
+                    if (Bound::Unbounded, Bound::Unbounded)
+                        != self.tcx.layout_scalar_valid_range(adt_def.did)
+                    {
+                        self.found = true;
+                    }
+                }
+                visit::walk_expr(self, expr);
+            }
+
+            // Keep walking through the expression as long as we stay in the same
+            // place, i.e. the expression is a place expression and not a dereference
+            // (since dereferencing something leads us to a different place).
+            ExprKind::Deref { .. } => {}
+            ref kind if ExprCategory::of(kind).map_or(true, |cat| cat == ExprCategory::Place) => {
+                visit::walk_expr(self, expr);
+            }
+
+            _ => {}
+        }
+    }
+}
+
 impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
     fn thir(&self) -> &'a Thir<'tcx> {
         &self.thir
@@ -161,60 +209,82 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
     }
 
     fn visit_pat(&mut self, pat: &Pat<'tcx>) {
-        use PatKind::*;
-
         if self.in_union_destructure {
             match *pat.kind {
                 // binding to a variable allows getting stuff out of variable
-                Binding { .. }
+                PatKind::Binding { .. }
                 // match is conditional on having this value
-                | Constant { .. }
-                | Variant { .. }
-                | Leaf { .. }
-                | Deref { .. }
-                | Range { .. }
-                | Slice { .. }
-                | Array { .. } => {
+                | PatKind::Constant { .. }
+                | PatKind::Variant { .. }
+                | PatKind::Leaf { .. }
+                | PatKind::Deref { .. }
+                | PatKind::Range { .. }
+                | PatKind::Slice { .. }
+                | PatKind::Array { .. } => {
                     self.requires_unsafe(pat.span, AccessToUnionField);
-                    return; // don't walk pattern
+                    return; // we can return here since this already requires unsafe
                 }
                 // wildcard doesn't take anything
-                Wild |
+                PatKind::Wild |
                 // these just wrap other patterns
-                Or { .. } |
-                AscribeUserType { .. } => {}
+                PatKind::Or { .. } |
+                PatKind::AscribeUserType { .. } => {}
             }
         };
 
-        if let ty::Adt(adt_def, _) = pat.ty.kind() {
-            // check for extracting values from union via destructuring
-            if adt_def.is_union() {
-                match *pat.kind {
-                    // assigning the whole union is okay
-                    // let x = Union { ... };
-                    // let y = x; // safe
-                    Binding { .. } |
-                    // binding to wildcard is okay since that never reads anything and stops double errors
-                    // with implict wildcard branches from `if let`s
-                    Wild |
-                    // doesn't have any effect on semantics
-                    AscribeUserType { .. } |
-                    // creating a union literal
-                    Constant { .. } => {},
-                    Leaf { .. } | Or { .. } => {
-                        // pattern matching with a union and not doing something like v = Union { bar: 5 }
-                        self.in_union_destructure = true;
+        match &*pat.kind {
+            PatKind::Leaf { .. } => {
+                if let ty::Adt(adt_def, ..) = pat.ty.kind() {
+                    if adt_def.is_union() {
+                        let old_in_union_destructure =
+                            std::mem::replace(&mut self.in_union_destructure, true);
+                        visit::walk_pat(self, pat);
+                        self.in_union_destructure = old_in_union_destructure;
+                    } else if (Bound::Unbounded, Bound::Unbounded)
+                        != self.tcx.layout_scalar_valid_range(adt_def.did)
+                    {
+                        let old_inside_adt = std::mem::replace(&mut self.inside_adt, true);
+                        visit::walk_pat(self, pat);
+                        self.inside_adt = old_inside_adt;
+                    } else {
                         visit::walk_pat(self, pat);
-                        self.in_union_destructure = false;
-                        return; // don't walk pattern
                     }
-                    Variant { .. } | Deref { .. } | Range { .. } | Slice { .. } | Array { .. } =>
-                        unreachable!("impossible union destructuring type"),
+                } else {
+                    visit::walk_pat(self, pat);
                 }
             }
+            PatKind::Binding { mode: BindingMode::ByRef(borrow_kind), ty, .. } => {
+                if self.inside_adt {
+                    if let ty::Ref(_, ty, _) = ty.kind() {
+                        match borrow_kind {
+                            BorrowKind::Shallow | BorrowKind::Shared | BorrowKind::Unique => {
+                                if !ty.is_freeze(self.tcx.at(pat.span), self.param_env) {
+                                    self.requires_unsafe(pat.span, BorrowOfLayoutConstrainedField);
+                                }
+                            }
+                            BorrowKind::Mut { .. } => {
+                                self.requires_unsafe(pat.span, MutationOfLayoutConstrainedField);
+                            }
+                        }
+                    } else {
+                        span_bug!(
+                            pat.span,
+                            "BindingMode::ByRef in pattern, but found non-reference type {}",
+                            ty
+                        );
+                    }
+                }
+                visit::walk_pat(self, pat);
+            }
+            PatKind::Deref { .. } => {
+                let old_inside_adt = std::mem::replace(&mut self.inside_adt, false);
+                visit::walk_pat(self, pat);
+                self.inside_adt = old_inside_adt;
+            }
+            _ => {
+                visit::walk_pat(self, pat);
+            }
         }
-
-        visit::walk_pat(self, pat);
     }
 
     fn visit_expr(&mut self, expr: &Expr<'tcx>) {
@@ -361,15 +431,46 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
                     }
                 }
             }
-            // don't have any special handling for AssignOp since it causes a read *and* write to lhs
-            ExprKind::Assign { lhs, rhs } => {
-                // assigning to a union is safe, check here so it doesn't get treated as a read later
-                self.in_possible_lhs_union_assign = true;
-                visit::walk_expr(self, &self.thir()[lhs]);
-                self.in_possible_lhs_union_assign = false;
-                visit::walk_expr(self, &self.thir()[rhs]);
-                return; // don't visit the whole expression
+            ExprKind::Assign { lhs, rhs } | ExprKind::AssignOp { lhs, rhs, .. } => {
+                // First, check whether we are mutating a layout constrained field
+                let mut visitor = LayoutConstrainedPlaceVisitor::new(self.thir, self.tcx);
+                visit::walk_expr(&mut visitor, &self.thir[lhs]);
+                if visitor.found {
+                    self.requires_unsafe(expr.span, MutationOfLayoutConstrainedField);
+                }
+
+                // Second, check for accesses to union fields
+                // don't have any special handling for AssignOp since it causes a read *and* write to lhs
+                if matches!(expr.kind, ExprKind::Assign { .. }) {
+                    // assigning to a union is safe, check here so it doesn't get treated as a read later
+                    self.in_possible_lhs_union_assign = true;
+                    visit::walk_expr(self, &self.thir()[lhs]);
+                    self.in_possible_lhs_union_assign = false;
+                    visit::walk_expr(self, &self.thir()[rhs]);
+                    return; // we have already visited everything by now
+                }
             }
+            ExprKind::Borrow { borrow_kind, arg } => match borrow_kind {
+                BorrowKind::Shallow | BorrowKind::Shared | BorrowKind::Unique => {
+                    if !self.thir[arg]
+                        .ty
+                        .is_freeze(self.tcx.at(self.thir[arg].span), self.param_env)
+                    {
+                        let mut visitor = LayoutConstrainedPlaceVisitor::new(self.thir, self.tcx);
+                        visit::walk_expr(&mut visitor, expr);
+                        if visitor.found {
+                            self.requires_unsafe(expr.span, BorrowOfLayoutConstrainedField);
+                        }
+                    }
+                }
+                BorrowKind::Mut { .. } => {
+                    let mut visitor = LayoutConstrainedPlaceVisitor::new(self.thir, self.tcx);
+                    visit::walk_expr(&mut visitor, expr);
+                    if visitor.found {
+                        self.requires_unsafe(expr.span, MutationOfLayoutConstrainedField);
+                    }
+                }
+            },
             _ => {}
         }
         visit::walk_expr(self, expr);
@@ -541,6 +642,8 @@ pub fn check_unsafety<'tcx>(tcx: TyCtxt<'tcx>, def: ty::WithOptConstParam<LocalD
         is_const,
         in_possible_lhs_union_assign: false,
         in_union_destructure: false,
+        param_env: tcx.param_env(def.did),
+        inside_adt: false,
     };
     visitor.visit_expr(&thir[expr]);
 }

src/test/ui/unsafe/ranged_ints2.stderr renamed to src/test/ui/unsafe/ranged_ints2.mirunsafeck.stderr

@@ -1,5 +1,5 @@
 error[E0133]: mutation of layout constrained field is unsafe and requires unsafe function or block
-  --> $DIR/ranged_ints2.rs:8:13
+  --> $DIR/ranged_ints2.rs:11:13
    |
 LL |     let y = &mut x.0;
    |             ^^^^^^^^ mutation of layout constrained field

src/test/ui/unsafe/ranged_ints2.rs

@@ -1,3 +1,6 @@
+// revisions: mirunsafeck thirunsafeck
+// [thirunsafeck]compile-flags: -Z thir-unsafeck
+
 #![feature(rustc_attrs)]
 
 #[rustc_layout_scalar_valid_range_start(1)]

src/test/ui/unsafe/ranged_ints2.thirunsafeck.stderr

@@ -0,0 +1,11 @@
+error[E0133]: mutation of layout constrained field is unsafe and requires unsafe function or block
+  --> $DIR/ranged_ints2.rs:11:13
+   |
+LL |     let y = &mut x.0;
+   |             ^^^^^^^^ mutation of layout constrained field
+   |
+   = note: mutating layout constrained fields cannot statically be checked for valid values
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0133`.

src/test/ui/unsafe/ranged_ints2_const.stderr renamed to src/test/ui/unsafe/ranged_ints2_const.mirunsafeck.stderr

@@ -1,5 +1,5 @@
 error[E0658]: mutable references are not allowed in constant functions
-  --> $DIR/ranged_ints2_const.rs:11:13
+  --> $DIR/ranged_ints2_const.rs:14:13
    |
 LL |     let y = &mut x.0;
    |             ^^^^^^^^
@@ -8,7 +8,7 @@ LL |     let y = &mut x.0;
    = help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
 
 error[E0658]: mutable references are not allowed in constant functions
-  --> $DIR/ranged_ints2_const.rs:18:22
+  --> $DIR/ranged_ints2_const.rs:21:22
    |
 LL |     let y = unsafe { &mut x.0 };
    |                      ^^^^^^^^
@@ -17,7 +17,7 @@ LL |     let y = unsafe { &mut x.0 };
    = help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
 
 error[E0658]: mutable references are not allowed in constant functions
-  --> $DIR/ranged_ints2_const.rs:24:22
+  --> $DIR/ranged_ints2_const.rs:27:22
    |
 LL |     unsafe { let y = &mut x.0; }
    |                      ^^^^^^^^
@@ -26,7 +26,7 @@ LL |     unsafe { let y = &mut x.0; }
    = help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
 
 error[E0133]: mutation of layout constrained field is unsafe and requires unsafe function or block
-  --> $DIR/ranged_ints2_const.rs:11:13
+  --> $DIR/ranged_ints2_const.rs:14:13
    |
 LL |     let y = &mut x.0;
    |             ^^^^^^^^ mutation of layout constrained field

src/test/ui/unsafe/ranged_ints2_const.rs

@@ -1,3 +1,6 @@
+// revisions: mirunsafeck thirunsafeck
+// [thirunsafeck]compile-flags: -Z thir-unsafeck
+
 #![feature(rustc_attrs)]
 
 #[rustc_layout_scalar_valid_range_start(1)]

src/test/ui/unsafe/ranged_ints2_const.thirunsafeck.stderr

@@ -0,0 +1,39 @@
+error[E0133]: mutation of layout constrained field is unsafe and requires unsafe function or block
+  --> $DIR/ranged_ints2_const.rs:14:13
+   |
+LL |     let y = &mut x.0;
+   |             ^^^^^^^^ mutation of layout constrained field
+   |
+   = note: mutating layout constrained fields cannot statically be checked for valid values
+
+error[E0658]: mutable references are not allowed in constant functions
+  --> $DIR/ranged_ints2_const.rs:14:13
+   |
+LL |     let y = &mut x.0;
+   |             ^^^^^^^^
+   |
+   = note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
+   = help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
+
+error[E0658]: mutable references are not allowed in constant functions
+  --> $DIR/ranged_ints2_const.rs:21:22
+   |
+LL |     let y = unsafe { &mut x.0 };
+   |                      ^^^^^^^^
+   |
+   = note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
+   = help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
+
+error[E0658]: mutable references are not allowed in constant functions
+  --> $DIR/ranged_ints2_const.rs:27:22
+   |
+LL |     unsafe { let y = &mut x.0; }
+   |                      ^^^^^^^^
+   |
+   = note: see issue #57349 <https://github.com/rust-lang/rust/issues/57349> for more information
+   = help: add `#![feature(const_mut_refs)]` to the crate attributes to enable
+
+error: aborting due to 4 previous errors
+
+Some errors have detailed explanations: E0133, E0658.
+For more information about an error, try `rustc --explain E0133`.

src/test/ui/unsafe/ranged_ints3.stderr renamed to src/test/ui/unsafe/ranged_ints3.mirunsafeck.stderr

@@ -1,5 +1,5 @@
 error[E0133]: borrow of layout constrained field with interior mutability is unsafe and requires unsafe function or block
-  --> $DIR/ranged_ints3.rs:10:13
+  --> $DIR/ranged_ints3.rs:13:13
    |
 LL |     let y = &x.0;
    |             ^^^^ borrow of layout constrained field with interior mutability