From ddfa2463e205a1bcae51aeb2698f09b4b8288e3d Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 24 Nov 2022 19:09:27 +0000 Subject: [PATCH] Evaluate place expression in `PlaceMention`. --- compiler/rustc_borrowck/src/def_use.rs | 8 +- compiler/rustc_borrowck/src/invalidation.rs | 2 +- compiler/rustc_borrowck/src/lib.rs | 2 +- .../rustc_const_eval/src/interpret/step.rs | 10 ++- compiler/rustc_middle/src/mir/syntax.rs | 3 +- .../clippy/tests/ui/option_if_let_else.fixed | 4 +- .../clippy/tests/ui/option_if_let_else.rs | 4 +- .../clippy/tests/ui/option_if_let_else.stderr | 8 +- .../dangling_pointer_deref_underscore.rs | 11 +++ .../dangling_pointer_deref_underscore.stderr | 15 ++++ tests/ui/borrowck/let_underscore_temporary.rs | 30 ++++++- .../borrowck/let_underscore_temporary.stderr | 79 +++++++++++++++++++ .../span/send-is-not-static-std-sync-2.stderr | 2 - 13 files changed, 159 insertions(+), 19 deletions(-) create mode 100644 src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_underscore.rs create mode 100644 src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_underscore.stderr create mode 100644 tests/ui/borrowck/let_underscore_temporary.stderr diff --git a/compiler/rustc_borrowck/src/def_use.rs b/compiler/rustc_borrowck/src/def_use.rs index 9e9f0b4b4ad..6259722b694 100644 --- 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 { 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 { 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 index 498d254da65..06986f848bf 100644 --- a/compiler/rustc_borrowck/src/invalidation.rs +++ b/compiler/rustc_borrowck/src/invalidation.rs @@ -79,7 +79,7 @@ fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { } // 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 index 6a5a7e08d38..5bf3e7632ac 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -665,7 +665,7 @@ fn visit_statement_before_primary_effect( } // 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 index 9a366364e76..f9c15c50513 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -113,8 +113,14 @@ pub fn statement(&mut self, stmt: &mir::Statement<'tcx>) -> InterpResult<'tcx> { 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 index c38a347809f..303c120f795 100644 --- 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>), diff --git a/src/tools/clippy/tests/ui/option_if_let_else.fixed b/src/tools/clippy/tests/ui/option_if_let_else.fixed index 0456005dce4..35ce89c7986 100644 --- 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) { 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) { 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 index 23b148752cb..c8683e5aae2 100644 --- 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) { *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) { *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 index a5dbf6e1f22..f5e4affb672 100644 --- 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 new file mode 100644 index 00000000000..3b2aba67a68 --- /dev/null +++ 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 new file mode 100644 index 00000000000..e047c3287b5 --- /dev/null +++ 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 index 37b5c5d9d7a..835cd20798f 100644 --- 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) { let _ = if let Some(s) = *string { s.len() } else { 0 }; @@ -8,6 +8,7 @@ fn let_underscore(string: &Option<&str>, mut num: Option) { 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) { s } else { &mut 0 + //~^ ERROR temporary value dropped while borrowed + }; +} + +fn let_ascribe(string: &Option<&str>, mut num: Option) { + 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 new file mode 100644 index 00000000000..74f3598c4d0 --- /dev/null +++ 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 index b0267fa6f43..c825cc8d668 100644 --- 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();