Evaluate place expression in PlaceMention.

cjgillot · cjgillot · commit ddfa2463e205 · 2023-04-21T21:34:59.000Z
diff --git a/compiler/rustc_borrowck/src/def_use.rs b/compiler/rustc_borrowck/src/def_use.rs
@@ -52,12 +52,16 @@ pub fn categorize(context: PlaceContext) -> Option<DefUse> {
         PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow) |
         PlaceContext::NonMutatingUse(NonMutatingUseContext::UniqueBorrow) |
 
+        // `PlaceMention` and `AscribeUserType` both evaluate the place, which must not
+        // contain dangling references.
+        PlaceContext::NonUse(NonUseContext::PlaceMention) |
+        PlaceContext::NonUse(NonUseContext::AscribeUserTy) |
+
         PlaceContext::MutatingUse(MutatingUseContext::AddressOf) |
         PlaceContext::NonMutatingUse(NonMutatingUseContext::AddressOf) |
         PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect) |
         PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) |
         PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) |
-        PlaceContext::NonUse(NonUseContext::AscribeUserTy) |
         PlaceContext::MutatingUse(MutatingUseContext::Retag) =>
             Some(DefUse::Use),
 
@@ -72,8 +76,6 @@ pub fn categorize(context: PlaceContext) -> Option<DefUse> {
         PlaceContext::MutatingUse(MutatingUseContext::Drop) =>
             Some(DefUse::Drop),
 
-        // This statement exists to help unsafeck. It does not require the place to be live.
-        PlaceContext::NonUse(NonUseContext::PlaceMention) => None,
         // Debug info is neither def nor use.
         PlaceContext::NonUse(NonUseContext::VarDebugInfo) => None,
 
diff --git a/compiler/rustc_borrowck/src/invalidation.rs b/compiler/rustc_borrowck/src/invalidation.rs
@@ -79,7 +79,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {
             }
             // Only relevant for mir typeck
             StatementKind::AscribeUserType(..)
-            // Only relevant for unsafeck
+            // Only relevant for liveness and unsafeck
             | StatementKind::PlaceMention(..)
             // Doesn't have any language semantics
             | StatementKind::Coverage(..)
diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs
@@ -665,7 +665,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
             }
             // Only relevant for mir typeck
             StatementKind::AscribeUserType(..)
-            // Only relevant for unsafeck
+            // Only relevant for liveness and unsafeck
             | StatementKind::PlaceMention(..)
             // Doesn't have any language semantics
             | StatementKind::Coverage(..)
diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs
@@ -113,8 +113,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
             Intrinsic(box intrinsic) => self.emulate_nondiverging_intrinsic(intrinsic)?,
 
-            // Statements we do not track.
-            PlaceMention(..) | AscribeUserType(..) => {}
+            // Evaluate the place expression, without reading from it.
+            PlaceMention(box place) => {
+                let _ = self.eval_place(*place)?;
+            }
+
+            // This exists purely to guide borrowck lifetime inference, and does not have
+            // an operational effect.
+            AscribeUserType(..) => {}
 
             // Currently, Miri discards Coverage statements. Coverage statements are only injected
             // via an optional compile time MIR pass and have no side effects. Since Coverage
diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs
@@ -331,7 +331,8 @@ pub enum StatementKind<'tcx> {
     /// This is especially useful for `let _ = PLACE;` bindings that desugar to a single
     /// `PlaceMention(PLACE)`.
     ///
-    /// When executed at runtime this is a nop.
+    /// When executed at runtime, this computes the given place, but then discards
+    /// it without doing a load. It is UB if the place is not pointing to live memory.
     ///
     /// Disallowed after drop elaboration.
     PlaceMention(Box<Place<'tcx>>),
diff --git a/src/tools/clippy/tests/ui/option_if_let_else.fixed b/src/tools/clippy/tests/ui/option_if_let_else.fixed
@@ -25,7 +25,7 @@ fn else_if_option(string: Option<&str>) -> Option<(bool, &str)> {
 fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {
     let _ = string.map_or(0, |s| s.len());
     let _ = num.as_ref().map_or(&0, |s| s);
-    let _ = num.as_mut().map_or(&mut 0, |s| {
+    let _ = num.as_mut().map_or(&0, |s| {
         *s += 1;
         s
     });
@@ -34,7 +34,7 @@ fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {
         s += 1;
         s
     });
-    let _ = num.as_mut().map_or(&mut 0, |s| {
+    let _ = num.as_mut().map_or(&0, |s| {
         *s += 1;
         s
     });
diff --git a/src/tools/clippy/tests/ui/option_if_let_else.rs b/src/tools/clippy/tests/ui/option_if_let_else.rs
@@ -33,7 +33,7 @@ fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {
         *s += 1;
         s
     } else {
-        &mut 0
+        &0
     };
     let _ = if let Some(ref s) = num { s } else { &0 };
     let _ = if let Some(mut s) = num {
@@ -46,7 +46,7 @@ fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {
         *s += 1;
         s
     } else {
-        &mut 0
+        &0
     };
 }
 
diff --git a/src/tools/clippy/tests/ui/option_if_let_else.stderr b/src/tools/clippy/tests/ui/option_if_let_else.stderr
@@ -30,13 +30,13 @@ LL |       let _ = if let Some(s) = &mut num {
 LL | |         *s += 1;
 LL | |         s
 LL | |     } else {
-LL | |         &mut 0
+LL | |         &0
 LL | |     };
    | |_____^
    |
 help: try
    |
-LL ~     let _ = num.as_mut().map_or(&mut 0, |s| {
+LL ~     let _ = num.as_mut().map_or(&0, |s| {
 LL +         *s += 1;
 LL +         s
 LL ~     });
@@ -76,13 +76,13 @@ LL |       let _ = if let Some(ref mut s) = num {
 LL | |         *s += 1;
 LL | |         s
 LL | |     } else {
-LL | |         &mut 0
+LL | |         &0
 LL | |     };
    | |_____^
    |
 help: try
    |
-LL ~     let _ = num.as_mut().map_or(&mut 0, |s| {
+LL ~     let _ = num.as_mut().map_or(&0, |s| {
 LL +         *s += 1;
 LL +         s
 LL ~     });
diff --git a/src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_underscore.rs b/src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_underscore.rs
@@ -0,0 +1,11 @@
+// Make sure we find these even with many checks disabled.
+//@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation
+
+fn main() {
+    let p = {
+        let b = Box::new(42);
+        &*b as *const i32
+    };
+    let _ = unsafe { *p }; //~ ERROR: dereferenced after this allocation got freed
+    panic!("this should never print");
+}
diff --git a/src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_underscore.stderr b/src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_underscore.stderr
@@ -0,0 +1,15 @@
+error: Undefined Behavior: pointer to ALLOC was dereferenced after this allocation got freed
+  --> $DIR/dangling_pointer_deref_underscore.rs:LL:CC
+   |
+LL |     let _ = unsafe { *p };
+   |                      ^^ pointer to ALLOC was dereferenced after this allocation got freed
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `main` at $DIR/dangling_pointer_deref_underscore.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to previous error
+
diff --git a/tests/ui/borrowck/let_underscore_temporary.rs b/tests/ui/borrowck/let_underscore_temporary.rs
@@ -1,4 +1,4 @@
-// check-pass
+// check-fail
 
 fn let_underscore(string: &Option<&str>, mut num: Option<i32>) {
     let _ = if let Some(s) = *string { s.len() } else { 0 };
@@ -8,6 +8,7 @@ fn let_underscore(string: &Option<&str>, mut num: Option<i32>) {
         s
     } else {
         &mut 0
+        //~^ ERROR temporary value dropped while borrowed
     };
     let _ = if let Some(ref s) = num { s } else { &0 };
     let _ = if let Some(mut s) = num {
@@ -21,6 +22,33 @@ fn let_underscore(string: &Option<&str>, mut num: Option<i32>) {
         s
     } else {
         &mut 0
+        //~^ ERROR temporary value dropped while borrowed
+    };
+}
+
+fn let_ascribe(string: &Option<&str>, mut num: Option<i32>) {
+    let _: _ = if let Some(s) = *string { s.len() } else { 0 };
+    let _: _ = if let Some(s) = &num { s } else { &0 };
+    let _: _ = if let Some(s) = &mut num {
+        *s += 1;
+        s
+    } else {
+        &mut 0
+        //~^ ERROR temporary value dropped while borrowed
+    };
+    let _: _ = if let Some(ref s) = num { s } else { &0 };
+    let _: _ = if let Some(mut s) = num {
+        s += 1;
+        s
+    } else {
+        0
+    };
+    let _: _ = if let Some(ref mut s) = num {
+        *s += 1;
+        s
+    } else {
+        &mut 0
+        //~^ ERROR temporary value dropped while borrowed
     };
 }
 
diff --git a/tests/ui/borrowck/let_underscore_temporary.stderr b/tests/ui/borrowck/let_underscore_temporary.stderr
@@ -0,0 +1,79 @@
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/let_underscore_temporary.rs:10:14
+   |
+LL |       let _ = if let Some(s) = &mut num {
+   |  _____________-
+LL | |         *s += 1;
+LL | |         s
+LL | |     } else {
+LL | |         &mut 0
+   | |              ^ creates a temporary value which is freed while still in use
+LL | |
+LL | |     };
+   | |     -
+   | |     |
+   | |_____temporary value is freed at the end of this statement
+   |       borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
+
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/let_underscore_temporary.rs:24:14
+   |
+LL |       let _ = if let Some(ref mut s) = num {
+   |  _____________-
+LL | |         *s += 1;
+LL | |         s
+LL | |     } else {
+LL | |         &mut 0
+   | |              ^ creates a temporary value which is freed while still in use
+LL | |
+LL | |     };
+   | |     -
+   | |     |
+   | |_____temporary value is freed at the end of this statement
+   |       borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
+
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/let_underscore_temporary.rs:36:14
+   |
+LL |       let _: _ = if let Some(s) = &mut num {
+   |  ________________-
+LL | |         *s += 1;
+LL | |         s
+LL | |     } else {
+LL | |         &mut 0
+   | |              ^ creates a temporary value which is freed while still in use
+LL | |
+LL | |     };
+   | |     -
+   | |     |
+   | |_____temporary value is freed at the end of this statement
+   |       borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
+
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/let_underscore_temporary.rs:50:14
+   |
+LL |       let _: _ = if let Some(ref mut s) = num {
+   |  ________________-
+LL | |         *s += 1;
+LL | |         s
+LL | |     } else {
+LL | |         &mut 0
+   | |              ^ creates a temporary value which is freed while still in use
+LL | |
+LL | |     };
+   | |     -
+   | |     |
+   | |_____temporary value is freed at the end of this statement
+   |       borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
+
+error: aborting due to 4 previous errors
+
+For more information about this error, try `rustc --explain E0716`.
diff --git a/tests/ui/span/send-is-not-static-std-sync-2.stderr b/tests/ui/span/send-is-not-static-std-sync-2.stderr
@@ -25,8 +25,6 @@ LL |     };
 error[E0597]: `x` does not live long enough
   --> $DIR/send-is-not-static-std-sync-2.rs:31:25
    |
-LL |     let (_tx, rx) = {
-   |          --- borrow later used here
 LL |         let x = 1;
    |             - binding `x` declared here
 LL |         let (tx, rx) = mpsc::channel();

Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> {`
`79`	`79`	`}`
`80`	`80`	`// Only relevant for mir typeck`
`81`	`81`	`StatementKind::AscribeUserType(..)`
`82`		`- // Only relevant for unsafeck`
	`82`	`+ // Only relevant for liveness and unsafeck`
`83`	`83`	`\| StatementKind::PlaceMention(..)`
`84`	`84`	`// Doesn't have any language semantics`
`85`	`85`	`\| StatementKind::Coverage(..)`
Original file line number	Diff line number	Diff line change
`@@ -665,7 +665,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx`
`665`	`665`	`}`
`666`	`666`	`// Only relevant for mir typeck`
`667`	`667`	`StatementKind::AscribeUserType(..)`
`668`		`- // Only relevant for unsafeck`
	`668`	`+ // Only relevant for liveness and unsafeck`
`669`	`669`	`\| StatementKind::PlaceMention(..)`
`670`	`670`	`// Doesn't have any language semantics`
`671`	`671`	`\| StatementKind::Coverage(..)`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ fn else_if_option(string: Option<&str>) -> Option<(bool, &str)> {`
`25`	`25`	`fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {`
`26`	`26`	`let _ = string.map_or(0, \|s\| s.len());`
`27`	`27`	`let _ = num.as_ref().map_or(&0, \|s\| s);`
`28`		`- let _ = num.as_mut().map_or(&mut 0, \|s\| {`
	`28`	`+ let _ = num.as_mut().map_or(&0, \|s\| {`
`29`	`29`	`*s += 1;`
`30`	`30`	`s`
`31`	`31`	`});`
`@@ -34,7 +34,7 @@ fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {`
`34`	`34`	`s += 1;`
`35`	`35`	`s`
`36`	`36`	`});`
`37`		`- let _ = num.as_mut().map_or(&mut 0, \|s\| {`
	`37`	`+ let _ = num.as_mut().map_or(&0, \|s\| {`
`38`	`38`	`*s += 1;`
`39`	`39`	`s`
`40`	`40`	`});`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {`
`33`	`33`	`*s += 1;`
`34`	`34`	`s`
`35`	`35`	`} else {`
`36`		`- &mut 0`
	`36`	`+ &0`
`37`	`37`	`};`
`38`	`38`	`let _ = if let Some(ref s) = num { s } else { &0 };`
`39`	`39`	`let _ = if let Some(mut s) = num {`
`@@ -46,7 +46,7 @@ fn unop_bad(string: &Option<&str>, mut num: Option<i32>) {`
`46`	`46`	`*s += 1;`
`47`	`47`	`s`
`48`	`48`	`} else {`
`49`		`- &mut 0`
	`49`	`+ &0`
`50`	`50`	`};`
`51`	`51`	`}`
`52`	`52`