Auto merge of #103208 - cjgillot:match-fake-read, r=oli-obk,RalfJung

Allow partially moved values in match This PR attempts to unify the behaviour between `let _ = PLACE`, `let _: TY = PLACE;` and `match PLACE { _ => {} }`. The logical conclusion is that the `match` version should not check for uninitialised places nor check that borrows are still live. The `match PLACE {}` case is handled by keeping a `FakeRead` in the unreachable fallback case to verify that `PLACE` has a legal value. Schematically, `match PLACE { arms }` in surface rust becomes in MIR: ```rust PlaceMention(PLACE) match PLACE { // Decision tree for the explicit arms arms, // An extra fallback arm _ => { FakeRead(ForMatchedPlace, PLACE); unreachable } } ``` `match *borrow { _ => {} }` continues to check that `*borrow` is live, but does not read the value. `match *borrow {}` both checks that `*borrow` is live, and fake-reads the value. Continuation of ~#102256 ~#104844 Fixes #99180 #53114
rust-lang · Oct 27, 2023 · 59bb950 · 59bb950
2 parents 10143e7 479fb4b
commit 59bb950
Show file tree

Hide file tree

Showing 64 changed files with 394 additions and 506 deletions.
diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs
@@ -157,7 157,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
  /// [ 0. Pre-match ]
  /// |
  /// [ 1. Evaluate Scrutinee (expression being matched on) ]
- /// [ (fake read of scrutinee) ]
  /// [ (PlaceMention of scrutinee) ]
  /// |
  /// [ 2. Decision tree -- check discriminants ] <-------- 
  /// | |
@@ -184,7 184,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
  ///
  /// We generate MIR in the following steps:
  ///
- /// 1. Evaluate the scrutinee and add the fake read of it ([Builder::lower_scrutinee]).
  /// 1. Evaluate the scrutinee and add the PlaceMention of it ([Builder::lower_scrutinee]).
  /// 2. Create the decision tree ([Builder::lower_match_tree]).
  /// 3. Determine the fake borrows that are needed from the places that were
  /// matched against and create the required temporaries for them
@@ -223,6 223,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
  let fake_borrow_temps = self.lower_match_tree(
  block,
  scrutinee_span,
  &scrutinee_place,
  match_start_span,
  match_has_guard,
  &mut candidates,
@@ -238,34 239,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
  )
  }
 
- /// Evaluate the scrutinee and add the fake read of it.
  /// Evaluate the scrutinee and add the PlaceMention for it.
  fn lower_scrutinee(
  &mut self,
  mut block: BasicBlock,
  scrutinee: &Expr<'tcx>,
  scrutinee_span: Span,
  ) -> BlockAnd<PlaceBuilder<'tcx>> {
  let scrutinee_place_builder = unpack!(block = self.as_place_builder(block, scrutinee));
- // Matching on a `scrutinee_place` with an uninhabited type doesn't
- // generate any memory reads by itself, and so if the place "expression"
- // contains unsafe operations like raw pointer dereferences or union
- // field projections, we wouldn't know to require an `unsafe` block
- // around a `match` equivalent to `std::intrinsics::unreachable()`.
- // See issue #47412 for this hole being discovered in the wild.
- //
- // HACK(eddyb) Work around the above issue by adding a dummy inspection
- // of `scrutinee_place`, specifically by applying `ReadForMatch`.
- //
- // NOTE: ReadForMatch also checks that the scrutinee is initialized.
- // This is currently needed to not allow matching on an uninitialized,
- // uninhabited value. If we get never patterns, those will check that
- // the place is initialized, and so this read would only be used to
- // check safety.
- let cause_matched_place = FakeReadCause::ForMatchedPlace(None);
- let source_info = self.source_info(scrutinee_span);
-
  if let Some(scrutinee_place) = scrutinee_place_builder.try_to_place(self) {
- self.cfg.push_fake_read(block, source_info, cause_matched_place, scrutinee_place);
  let source_info = self.source_info(scrutinee_span);
  self.cfg.push_place_mention(block, source_info, scrutinee_place);
  }
 
  block.and(scrutinee_place_builder)
@@ -304,6 288,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
  &mut self,
  block: BasicBlock,
  scrutinee_span: Span,
  scrutinee_place_builder: &PlaceBuilder<'tcx>,
  match_start_span: Span,
  match_has_guard: bool,
  candidates: &mut [&mut Candidate<'pat, 'tcx>],
@@ -331,6 316,33 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
  // otherwise block. Match checking will ensure this is actually
  // unreachable.
  let source_info = self.source_info(scrutinee_span);
 
  // Matching on a `scrutinee_place` with an uninhabited type doesn't
  // generate any memory reads by itself, and so if the place "expression"
  // contains unsafe operations like raw pointer dereferences or union
  // field projections, we wouldn't know to require an `unsafe` block
  // around a `match` equivalent to `std::intrinsics::unreachable()`.
  // See issue #47412 for this hole being discovered in the wild.
  //
  // HACK(eddyb) Work around the above issue by adding a dummy inspection
  // of `scrutinee_place`, specifically by applying `ReadForMatch`.
  //
  // NOTE: ReadForMatch also checks that the scrutinee is initialized.
  // This is currently needed to not allow matching on an uninitialized,
  // uninhabited value. If we get never patterns, those will check that
  // the place is initialized, and so this read would only be used to
  // check safety.
  let cause_matched_place = FakeReadCause::ForMatchedPlace(None);
 
  if let Some(scrutinee_place) = scrutinee_place_builder.try_to_place(self) {
  self.cfg.push_fake_read(
  otherwise_block,
  source_info,
  cause_matched_place,
  scrutinee_place,
  );
  }
 
  self.cfg.terminate(otherwise_block, source_info, TerminatorKind::Unreachable);
  }
 
@@ -599,13 611,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
  }
 
  _ => {
- let place_builder = unpack!(block = self.as_place_builder(block, initializer));
-
- if let Some(place) = place_builder.try_to_place(self) {
- let source_info = self.source_info(initializer.span);
- self.cfg.push_place_mention(block, source_info, place);
- }
-
  let place_builder =
  unpack!(block = self.lower_scrutinee(block, initializer, initializer.span));
  self.place_into_pattern(block, &irrefutable_pat, place_builder, true)
  }
  }
@@ -622,6 629,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
  let fake_borrow_temps = self.lower_match_tree(
  block,
  irrefutable_pat.span,
  &initializer,
  irrefutable_pat.span,
  false,
  &mut [&mut candidate],
@@ -1841,6 1849,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
  let fake_borrow_temps = self.lower_match_tree(
  block,
  pat.span,
  &expr_place_builder,
  pat.span,
  false,
  &mut [&mut guard_candidate, &mut otherwise_candidate],
@@ -2342,6 2351,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
  let fake_borrow_temps = this.lower_match_tree(
  block,
  initializer_span,
  &scrutinee,
  pattern.span,
  false,
  &mut [&mut candidate, &mut wildcard],

diff --git a/src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_match_never.rs b/src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_match_never.rs
@@ -0,0 1,17 @@
 // Make sure we find these even with many checks disabled.
 //@compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation
 
 #![allow(unreachable_code)]
 #![feature(never_type)]
 
 fn main() {
  let p = {
  let b = Box::new(42);
  &*b as *const i32 as *const !
  };
  unsafe {
  match *p {} //~ ERROR: entering unreachable code
  }
  panic!("this should never print");
 }
 
diff --git a/src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_match_never.stderr b/src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref_match_never.stderr
@@ -0,0 1,15 @@
 error: Undefined Behavior: entering unreachable code
  --> $DIR/dangling_pointer_deref_match_never.rs:LL:CC
  |
 LL | match *p {}
  | ^^ entering unreachable code
  |
  = 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_match_never.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/src/tools/miri/tests/fail/never_match_never.rs b/src/tools/miri/tests/fail/never_match_never.rs
@@ -0,0 1,10 @@
 // This should fail even without validation
 //@compile-flags: -Zmiri-disable-validation
 
 #![feature(never_type)]
 #![allow(unreachable_code)]
 
 fn main() {
  let ptr: *const (i32, !) = &0i32 as *const i32 as *const _;
  unsafe { match (*ptr).1 {} } //~ ERROR: entering unreachable code
 }
diff --git a/src/tools/miri/tests/fail/never_match_never.stderr b/src/tools/miri/tests/fail/never_match_never.stderr
@@ -0,0 1,15 @@
 error: Undefined Behavior: entering unreachable code
  --> $DIR/never_match_never.rs:LL:CC
  |
 LL | unsafe { match (*ptr).1 {} }
  | ^^^^^^^^ entering unreachable code
  |
  = 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/never_match_never.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/src/tools/miri/tests/pass/dangling_pointer_deref_match_underscore.rs b/src/tools/miri/tests/pass/dangling_pointer_deref_match_underscore.rs
@@ -0,0 1,14 @@
 // A `_` binding in a match is a nop, so we do not detect that the pointer is dangling.
 //@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
  };
  unsafe {
  match *p {
  _ => {}
  }
  }
 }
diff --git a/src/tools/miri/tests/pass/union-uninhabited-match-underscore.rs b/src/tools/miri/tests/pass/union-uninhabited-match-underscore.rs
@@ -0,0 1,17 @@
 fn main() {
  #[derive(Copy, Clone)]
  enum Void {}
  union Uninit<T: Copy> {
  value: T,
  uninit: (),
  }
  unsafe {
  let x: Uninit<Void> = Uninit { uninit: () };
  match x.value {
  // rustc warns about un unreachable pattern,
  // but is wrong in unsafe code.
  #[allow(unreachable_patterns)]
  _ => println!("hi from the void!"),
  }
  }
 }
diff --git a/src/tools/miri/tests/pass/union-uninhabited-match-underscore.stdout b/src/tools/miri/tests/pass/union-uninhabited-match-underscore.stdout
@@ -0,0 1 @@
 hi from the void!
diff --git a/tests/mir-opt/building/async_await.b-{closure#0}.coroutine_resume.0.mir b/tests/mir-opt/building/async_await.b-{closure#0}.coroutine_resume.0.mir
@@ -127,6 127,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
 
  bb3: {
  StorageDead(_5);
  PlaceMention(_4);
  nop;
  (((*(_1.0: &mut {async fn body@$DIR/async_await.rs:15:18: 18:2})) as variant#3).0: {async fn body@$DIR/async_await.rs:12:14: 12:16}) = move _4;
  goto -> bb4;
@@ -162,6 163,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
  bb7: {
  StorageDead(_13);
  StorageDead(_10);
  PlaceMention(_9);
  _16 = discriminant(_9);
  switchInt(move _16) -> [0: bb10, 1: bb8, otherwise: bb9];
  }
@@ -223,6 225,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
 
  bb15: {
  StorageDead(_22);
  PlaceMention(_21);
  nop;
  (((*(_1.0: &mut {async fn body@$DIR/async_await.rs:15:18: 18:2})) as variant#4).0: {async fn body@$DIR/async_await.rs:12:14: 12:16}) = move _21;
  goto -> bb16;
@@ -258,6 261,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
  bb19: {
  StorageDead(_29);
  StorageDead(_26);
  PlaceMention(_25);
  _32 = discriminant(_25);
  switchInt(move _32) -> [0: bb21, 1: bb20, otherwise: bb9];
  }

diff --git a/tests/mir-opt/building/issue_101867.main.built.after.mir b/tests/mir-opt/building/issue_101867.main.built.after.mir
@@ -25,7 25,7 @@ fn main() -> () {
  FakeRead(ForLet(None), _1);
  AscribeUserType(_1, o, UserTypeProjection { base: UserType(1), projs: [] });
  StorageLive(_5);
- FakeRead(ForMatchedPlace(None), _1);
  PlaceMention(_1);
  _6 = discriminant(_1);
  switchInt(move _6) -> [1: bb4, otherwise: bb3];
  }

diff --git a/tests/mir-opt/building/issue_49232.main.built.after.mir b/tests/mir-opt/building/issue_49232.main.built.after.mir
@@ -24,7 24,7 @@ fn main() -> () {
  StorageLive(_2);
  StorageLive(_3);
  _3 = const true;
- FakeRead(ForMatchedPlace(None), _3);
  PlaceMention(_3);
  switchInt(_3) -> [0: bb3, otherwise: bb4];
  }
 

diff --git a/tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir b/tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir
@@ -23,7 23,7 @@ fn test_complex() -> () {
  }
 
  bb1: {
- FakeRead(ForMatchedPlace(None), _2);
  PlaceMention(_2);
  _3 = discriminant(_2);
  switchInt(move _3) -> [0: bb2, otherwise: bb3];
  }
@@ -151,7 151,7 @@ fn test_complex() -> () {
  }
 
  bb25: {
- FakeRead(ForMatchedPlace(None), _12);
  PlaceMention(_12);
  _13 = discriminant(_12);
  switchInt(move _13) -> [1: bb27, otherwise: bb26];
  }

diff --git a/tests/mir-opt/building/match_false_edges.full_tested_match.built.after.mir b/tests/mir-opt/building/match_false_edges.full_tested_match.built.after.mir
@@ -26,7 26,7 @@ fn full_tested_match() -> () {
  StorageLive(_1);
  StorageLive(_2);
  _2 = Option::<i32>::Some(const 42_i32);
- FakeRead(ForMatchedPlace(None), _2);
  PlaceMention(_2);
  _3 = discriminant(_2);
  switchInt(move _3) -> [0: bb1, 1: bb2, otherwise: bb4];
  }
@@ -45,6 45,7 @@ fn full_tested_match() -> () {
  }
 
  bb4: {
  FakeRead(ForMatchedPlace(None), _2);
  unreachable;
  }
 

diff --git a/tests/mir-opt/building/match_false_edges.full_tested_match2.built.after.mir b/tests/mir-opt/building/match_false_edges.full_tested_match2.built.after.mir
@@ -26,7 26,7 @@ fn full_tested_match2() -> () {
  StorageLive(_1);
  StorageLive(_2);
  _2 = Option::<i32>::Some(const 42_i32);
- FakeRead(ForMatchedPlace(None), _2);
  PlaceMention(_2);
  _3 = discriminant(_2);
  switchInt(move _3) -> [0: bb1, 1: bb2, otherwise: bb4];
  }
@@ -51,6 51,7 @@ fn full_tested_match2() -> () {
  }
 
  bb4: {
  FakeRead(ForMatchedPlace(None), _2);
  unreachable;
  }
 

diff --git a/tests/mir-opt/building/match_false_edges.main.built.after.mir b/tests/mir-opt/building/match_false_edges.main.built.after.mir
@@ -37,7 37,7 @@ fn main() -> () {
  StorageLive(_1);
  StorageLive(_2);
  _2 = Option::<i32>::Some(const 1_i32);
- FakeRead(ForMatchedPlace(None), _2);
  PlaceMention(_2);
  _4 = discriminant(_2);
  switchInt(move _4) -> [1: bb2, otherwise: bb1];
  }

diff --git a/tests/mir-opt/building/simple_match.match_bool.built.after.mir b/tests/mir-opt/building/simple_match.match_bool.built.after.mir
@@ -5,7 5,7 @@ fn match_bool(_1: bool) -> usize {
  let mut _0: usize;
 
  bb0: {
- FakeRead(ForMatchedPlace(None), _1);
  PlaceMention(_1);
  switchInt(_1) -> [0: bb2, otherwise: bb1];
  }
 

diff --git a/tests/mir-opt/const_prop/transmute.unreachable_box.ConstProp.32bit.diff b/tests/mir-opt/const_prop/transmute.unreachable_box.ConstProp.32bit.diff
@@ -4,6 4,7 @@
  fn unreachable_box() -> ! {
  let mut _0: !;
  let _1: std::boxed::Box<Never>;
  let mut _2: *const Never;
  scope 1 {
  debug x => _1;
  }
@@ -13,7 14,9 @@
  bb0: {
  StorageLive(_1);
 - _1 = const 1_usize as std::boxed::Box<Never> (Transmute);
 - _2 = (((_1.0: std::ptr::Unique<Never>).0: std::ptr::NonNull<Never>).0: *const Never);
   _1 = const Box::<Never>(Unique::<Never> {{ pointer: NonNull::<Never> {{ pointer: {0x1 as *const Never} }}, _marker: PhantomData::<Never> }}, std::alloc::Global);
   _2 = const {0x1 as *const Never};
  unreachable;
  }
  }

diff --git a/tests/mir-opt/const_prop/transmute.unreachable_box.ConstProp.64bit.diff b/tests/mir-opt/const_prop/transmute.unreachable_box.ConstProp.64bit.diff
@@ -4,6 4,7 @@
  fn unreachable_box() -> ! {
  let mut _0: !;
  let _1: std::boxed::Box<Never>;
  let mut _2: *const Never;
  scope 1 {
  debug x => _1;
  }
@@ -13,7 14,9 @@
  bb0: {
  StorageLive(_1);
 - _1 = const 1_usize as std::boxed::Box<Never> (Transmute);
 - _2 = (((_1.0: std::ptr::Unique<Never>).0: std::ptr::NonNull<Never>).0: *const Never);
   _1 = const Box::<Never>(Unique::<Never> {{ pointer: NonNull::<Never> {{ pointer: {0x1 as *const Never} }}, _marker: PhantomData::<Never> }}, std::alloc::Global);
   _2 = const {0x1 as *const Never};
  unreachable;
  }
  }