Auto merge of rust-lang#112070 - lcnr:disjoint-closure-capture-ub, r=…

…oli-obk change `BorrowKind::Unique` to be a mutating `PlaceContext` fixes rust-lang#112056 I believe that `BorrowKind::Unique` is a footgun in general, so I added a FIXME and opened rust-lang#112072. This is a bit too involved for this PR though.
matthiaskrgr · May 31, 2023 · e6e4f7e · e6e4f7e
2 parents 9610dfe + 25f8f4c
commit e6e4f7e
Show file tree

Hide file tree

Showing 14 changed files with 90 additions and 19 deletions.
diff --git a/compiler/rustc_borrowck/src/def_use.rs b/compiler/rustc_borrowck/src/def_use.rs
@@ -50,7 +50,6 @@ pub fn categorize(context: PlaceContext) -> Option<DefUse> {
         PlaceContext::MutatingUse(MutatingUseContext::Borrow) |
         PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow) |
         PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow) |
-        PlaceContext::NonMutatingUse(NonMutatingUseContext::UniqueBorrow) |
 
         // `PlaceMention` and `AscribeUserType` both evaluate the place, which must not
         // contain dangling references.

diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs
@@ -766,8 +766,8 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
             PlaceContext::MutatingUse(_) => ty::Invariant,
             PlaceContext::NonUse(StorageDead | StorageLive | VarDebugInfo) => ty::Invariant,
             PlaceContext::NonMutatingUse(
-                Inspect | Copy | Move | PlaceMention | SharedBorrow | ShallowBorrow | UniqueBorrow
-                | AddressOf | Projection,
+                Inspect | Copy | Move | PlaceMention | SharedBorrow | ShallowBorrow | AddressOf
+                | Projection,
             ) => ty::Covariant,
             PlaceContext::NonUse(AscribeUserTy(variance)) => variance,
         }

diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs
@@ -234,7 +234,6 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
             | PlaceContext::NonMutatingUse(
                 NonMutatingUseContext::Inspect
                 | NonMutatingUseContext::SharedBorrow
-                | NonMutatingUseContext::UniqueBorrow
                 | NonMutatingUseContext::ShallowBorrow
                 | NonMutatingUseContext::AddressOf
                 | NonMutatingUseContext::Projection,

diff --git a/compiler/rustc_const_eval/src/transform/check_consts/check.rs b/compiler/rustc_const_eval/src/transform/check_consts/check.rs
@@ -412,9 +412,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
                         BorrowKind::Shallow => {
                             PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow)
                         }
-                        BorrowKind::Unique => {
-                            PlaceContext::NonMutatingUse(NonMutatingUseContext::UniqueBorrow)
-                        }
+                        BorrowKind::Unique => PlaceContext::MutatingUse(MutatingUseContext::Borrow),
                         BorrowKind::Mut { .. } => {
                             PlaceContext::MutatingUse(MutatingUseContext::Borrow)
                         }

diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs
@@ -220,6 +220,11 @@ pub enum BorrowKind {
     /// immutable, but not aliasable. This solves the problem. For
     /// simplicity, we don't give users the way to express this
     /// borrow, it's just used when translating closures.
+    ///
+    // FIXME(#112072): This is wrong. Unique borrows are mutable borrows except
+    // that they do not require their pointee to be marked as a mutable.
+    // They should still be treated as mutable borrows in every other way,
+    // e.g. for variance or overlap checking.
     Unique,
 
     /// Data is mutable and not aliasable.

diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs
@@ -650,8 +650,8 @@ macro_rules! make_mir_visitor {
                             BorrowKind::Shallow => PlaceContext::NonMutatingUse(
                                 NonMutatingUseContext::ShallowBorrow
                             ),
-                            BorrowKind::Unique => PlaceContext::NonMutatingUse(
-                                NonMutatingUseContext::UniqueBorrow
+                            BorrowKind::Unique => PlaceContext::MutatingUse(
+                                MutatingUseContext::Borrow
                             ),
                             BorrowKind::Mut { .. } =>
                                 PlaceContext::MutatingUse(MutatingUseContext::Borrow),
@@ -1265,8 +1265,6 @@ pub enum NonMutatingUseContext {
     SharedBorrow,
     /// Shallow borrow.
     ShallowBorrow,
-    /// Unique borrow.
-    UniqueBorrow,
     /// AddressOf for *const pointer.
     AddressOf,
     /// PlaceMention statement.
@@ -1345,9 +1343,7 @@ impl PlaceContext {
         matches!(
             self,
             PlaceContext::NonMutatingUse(
-                NonMutatingUseContext::SharedBorrow
-                    | NonMutatingUseContext::ShallowBorrow
-                    | NonMutatingUseContext::UniqueBorrow
+                NonMutatingUseContext::SharedBorrow | NonMutatingUseContext::ShallowBorrow
             ) | PlaceContext::MutatingUse(MutatingUseContext::Borrow)
         )
     }

diff --git a/compiler/rustc_mir_dataflow/src/impls/liveness.rs b/compiler/rustc_mir_dataflow/src/impls/liveness.rs
@@ -199,8 +199,7 @@ impl DefUse {
                 | NonMutatingUseContext::Move
                 | NonMutatingUseContext::PlaceMention
                 | NonMutatingUseContext::ShallowBorrow
-                | NonMutatingUseContext::SharedBorrow
-                | NonMutatingUseContext::UniqueBorrow,
+                | NonMutatingUseContext::SharedBorrow,
             ) => Some(DefUse::Use),
 
             PlaceContext::MutatingUse(MutatingUseContext::Projection)

diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs
@@ -772,7 +772,6 @@ impl<'tcx> Visitor<'tcx> for CanConstProp {
             // mutation.
             | NonMutatingUse(NonMutatingUseContext::SharedBorrow)
             | NonMutatingUse(NonMutatingUseContext::ShallowBorrow)
-            | NonMutatingUse(NonMutatingUseContext::UniqueBorrow)
             | NonMutatingUse(NonMutatingUseContext::AddressOf)
             | MutatingUse(MutatingUseContext::Borrow)
             | MutatingUse(MutatingUseContext::AddressOf) => {

diff --git a/compiler/rustc_mir_transform/src/copy_prop.rs b/compiler/rustc_mir_transform/src/copy_prop.rs
@@ -130,7 +130,6 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
             PlaceContext::NonMutatingUse(
                 NonMutatingUseContext::SharedBorrow
                 | NonMutatingUseContext::ShallowBorrow
-                | NonMutatingUseContext::UniqueBorrow
                 | NonMutatingUseContext::AddressOf,
             ) => true,
             // For debuginfo, merging locals is ok.

diff --git a/compiler/rustc_mir_transform/src/ssa.rs b/compiler/rustc_mir_transform/src/ssa.rs
@@ -216,7 +216,6 @@ impl<'tcx> Visitor<'tcx> for SsaVisitor<'_> {
             PlaceContext::NonMutatingUse(
                 NonMutatingUseContext::SharedBorrow
                 | NonMutatingUseContext::ShallowBorrow
-                | NonMutatingUseContext::UniqueBorrow
                 | NonMutatingUseContext::AddressOf,
             )
             | PlaceContext::MutatingUse(_) => {

diff --git a/tests/ui/closures/2229_closure_analysis/unique-borrows-are-invariant-1.rs b/tests/ui/closures/2229_closure_analysis/unique-borrows-are-invariant-1.rs
@@ -0,0 +1,18 @@
+// edition:2021
+
+// regression test for #112056
+
+fn extend_lifetime<'a, 'b>(x: &mut (&'a str,), y: &'b str) {
+    let mut closure = |input| x.0 = input;
+    //~^ ERROR: lifetime may not live long enough
+    closure(y);
+}
+
+fn main() {
+    let mut tuple = ("static",);
+    {
+        let x = String::from("temporary");
+        extend_lifetime(&mut tuple, &x);
+    }
+    println!("{}", tuple.0);
+}
diff --git a/tests/ui/closures/2229_closure_analysis/unique-borrows-are-invariant-1.stderr b/tests/ui/closures/2229_closure_analysis/unique-borrows-are-invariant-1.stderr
@@ -0,0 +1,14 @@
+error: lifetime may not live long enough
+  --> $DIR/unique-borrows-are-invariant-1.rs:6:31
+   |
+LL | fn extend_lifetime<'a, 'b>(x: &mut (&'a str,), y: &'b str) {
+   |                    --  -- lifetime `'b` defined here
+   |                    |
+   |                    lifetime `'a` defined here
+LL |     let mut closure = |input| x.0 = input;
+   |                               ^^^^^^^^^^^ assignment requires that `'b` must outlive `'a`
+   |
+   = help: consider adding the following bound: `'b: 'a`
+
+error: aborting due to previous error
+
diff --git a/tests/ui/closures/2229_closure_analysis/unique-borrows-are-invariant-2.rs b/tests/ui/closures/2229_closure_analysis/unique-borrows-are-invariant-2.rs
@@ -0,0 +1,31 @@
+// edition:2021
+
+// regression test for #112056
+
+struct Spooky<'b> {
+    owned: Option<&'static u32>,
+    borrowed: &'b &'static u32,
+}
+
+impl<'b> Spooky<'b> {
+    fn create_self_reference<'a>(&'a mut self) {
+        let mut closure = || {
+            if let Some(owned) = &self.owned {
+                let borrow: &'a &'static u32 = owned;
+                self.borrowed = borrow;
+                //~^ ERROR: lifetime may not live long enough
+            }
+        };
+        closure();
+    }
+}
+
+fn main() {
+    let mut spooky: Spooky<'static> = Spooky {
+        owned: Some(&1),
+        borrowed: &&1,
+    };
+    spooky.create_self_reference();
+    spooky.owned = None;
+    println!("{}", **spooky.borrowed);
+}
diff --git a/tests/ui/closures/2229_closure_analysis/unique-borrows-are-invariant-2.stderr b/tests/ui/closures/2229_closure_analysis/unique-borrows-are-invariant-2.stderr
@@ -0,0 +1,15 @@
+error: lifetime may not live long enough
+  --> $DIR/unique-borrows-are-invariant-2.rs:15:17
+   |
+LL | impl<'b> Spooky<'b> {
+   |      -- lifetime `'b` defined here
+LL |     fn create_self_reference<'a>(&'a mut self) {
+   |                              -- lifetime `'a` defined here
+...
+LL |                 self.borrowed = borrow;
+   |                 ^^^^^^^^^^^^^^^^^^^^^^ assignment requires that `'a` must outlive `'b`
+   |
+   = help: consider adding the following bound: `'a: 'b`
+
+error: aborting due to previous error
+