Check which places are dead

Fixes rust-lang#78192

Author: simonvandel

compiler/rustc_mir/src/transform/instcombine.rs

@@ -119,11 +119,6 @@ impl OptimizationFinder<'b, 'tcx> {
     }
 
     fn find_deref_of_address(&mut self, rvalue: &Rvalue<'tcx>, location: Location) -> Option<()> {
-        // FIXME(#78192): This optimization can result in unsoundness.
-        if !self.tcx.sess.opts.debugging_opts.unsound_mir_opts {
-            return None;
-        }
-
         // Look for the sequence
         //
         // _2 = &_1;
@@ -137,6 +132,8 @@ impl OptimizationFinder<'b, 'tcx> {
                 _ => None,
             }?;
 
+            let mut dead_locals_seen = vec![];
+
             let stmt_index = location.statement_index;
             // Look behind for statement that assigns the local from a address of operator.
             // 6 is chosen as a heuristic determined by seeing the number of times
@@ -160,6 +157,11 @@ impl OptimizationFinder<'b, 'tcx> {
                                 BorrowKind::Shared,
                                 place_taken_address_of,
                             ) => {
+                                // Make sure that the place has not been marked dead
+                                if dead_locals_seen.contains(&place_taken_address_of.local) {
+                                    return None;
+                                }
+
                                 self.optimizations
                                     .unneeded_deref
                                     .insert(location, *place_taken_address_of);
@@ -178,13 +180,19 @@ impl OptimizationFinder<'b, 'tcx> {
                     // Inline asm can do anything, so bail out of the optimization.
                     rustc_middle::mir::StatementKind::LlvmInlineAsm(_) => return None,
 
+                    // Remember `StorageDead`s, as the local being marked dead could be the
+                    // place RHS we are looking for, in which case we need to abort to avoid UB
+                    // using an uninitialized place
+                    rustc_middle::mir::StatementKind::StorageDead(dead) => {
+                        dead_locals_seen.push(*dead)
+                    }
+
                     // Check that `local_being_deref` is not being used in a mutating way which can cause misoptimization.
                     rustc_middle::mir::StatementKind::Assign(box (_, _))
                     | rustc_middle::mir::StatementKind::Coverage(_)
                     | rustc_middle::mir::StatementKind::Nop
                     | rustc_middle::mir::StatementKind::FakeRead(_, _)
                     | rustc_middle::mir::StatementKind::StorageLive(_)
-                    | rustc_middle::mir::StatementKind::StorageDead(_)
                     | rustc_middle::mir::StatementKind::Retag(_, _)
                     | rustc_middle::mir::StatementKind::AscribeUserType(_, _)
                     | rustc_middle::mir::StatementKind::SetDiscriminant { .. } => {

src/test/mir-opt/const_prop/ref_deref.main.ConstProp.diff

@@ -19,7 +19,7 @@
                                            // + span: $DIR/ref_deref.rs:5:6: 5:10
                                            // + literal: Const { ty: &i32, val: Unevaluated(WithOptConstParam { did: DefId(0:3 ~ ref_deref[317d]::main), const_param_did: None }, [], Some(promoted[0])) }
           _2 = _4;                         // scope 0 at $DIR/ref_deref.rs:5:6: 5:10
--         _1 = (*_2);                      // scope 0 at $DIR/ref_deref.rs:5:5: 5:10
+-         _1 = (*_4);                      // scope 0 at $DIR/ref_deref.rs:5:5: 5:10
 +         _1 = const 4_i32;                // scope 0 at $DIR/ref_deref.rs:5:5: 5:10
           StorageDead(_2);                 // scope 0 at $DIR/ref_deref.rs:5:10: 5:11
           StorageDead(_1);                 // scope 0 at $DIR/ref_deref.rs:5:10: 5:11

src/test/mir-opt/const_prop/ref_deref_project.main.ConstProp.diff

@@ -19,7 +19,7 @@
                                            // + span: $DIR/ref_deref_project.rs:5:6: 5:17
                                            // + literal: Const { ty: &(i32, i32), val: Unevaluated(WithOptConstParam { did: DefId(0:3 ~ ref_deref_project[317d]::main), const_param_did: None }, [], Some(promoted[0])) }
           _2 = &((*_4).1: i32);            // scope 0 at $DIR/ref_deref_project.rs:5:6: 5:17
-          _1 = (*_2);                      // scope 0 at $DIR/ref_deref_project.rs:5:5: 5:17
+          _1 = ((*_4).1: i32);             // scope 0 at $DIR/ref_deref_project.rs:5:5: 5:17
           StorageDead(_2);                 // scope 0 at $DIR/ref_deref_project.rs:5:17: 5:18
           StorageDead(_1);                 // scope 0 at $DIR/ref_deref_project.rs:5:17: 5:18
           _0 = const ();                   // scope 0 at $DIR/ref_deref_project.rs:4:11: 6:2

src/test/mir-opt/inst_combine_deref.rs

@@ -1,4 +1,4 @@
-// compile-flags: -O -Zunsound-mir-opts
+// compile-flags: -O
 // EMIT_MIR inst_combine_deref.simple_opt.InstCombine.diff
 fn simple_opt() -> u64 {
     let x = 5;

src/test/mir-opt/issue-78192.rs

@@ -0,0 +1,9 @@
+// EMIT_MIR issue_78192.f.InstCombine.diff
+pub fn f<T>(a: &T) -> *const T {
+    let b: &*const T = &(a as *const T);
+    *b
+}
+
+fn main() {
+    f(&2);
+}

src/test/mir-opt/issue_78192.f.InstCombine.diff

@@ -0,0 +1,29 @@
+- // MIR for `f` before InstCombine
++ // MIR for `f` after InstCombine
+  
+  fn f(_1: &T) -> *const T {
+      debug a => _1;                       // in scope 0 at $DIR/issue-78192.rs:2:13: 2:14
+      let mut _0: *const T;                // return place in scope 0 at $DIR/issue-78192.rs:2:23: 2:31
+      let _2: &*const T;                   // in scope 0 at $DIR/issue-78192.rs:3:9: 3:10
+      let _3: &*const T;                   // in scope 0 at $DIR/issue-78192.rs:3:24: 3:40
+      let _4: *const T;                    // in scope 0 at $DIR/issue-78192.rs:3:25: 3:40
+      scope 1 {
+          debug b => _2;                   // in scope 1 at $DIR/issue-78192.rs:3:9: 3:10
+      }
+  
+      bb0: {
+          StorageLive(_2);                 // scope 0 at $DIR/issue-78192.rs:3:9: 3:10
+          StorageLive(_3);                 // scope 0 at $DIR/issue-78192.rs:3:24: 3:40
+          StorageLive(_4);                 // scope 0 at $DIR/issue-78192.rs:3:25: 3:40
+          _4 = &raw const (*_1);           // scope 0 at $DIR/issue-78192.rs:3:26: 3:27
+          _3 = &_4;                        // scope 0 at $DIR/issue-78192.rs:3:24: 3:40
+-         _2 = &(*_3);                     // scope 0 at $DIR/issue-78192.rs:3:24: 3:40
++         _2 = _3;                         // scope 0 at $DIR/issue-78192.rs:3:24: 3:40
+          StorageDead(_3);                 // scope 0 at $DIR/issue-78192.rs:3:40: 3:41
+          _0 = (*_2);                      // scope 1 at $DIR/issue-78192.rs:4:5: 4:7
+          StorageDead(_4);                 // scope 0 at $DIR/issue-78192.rs:5:1: 5:2
+          StorageDead(_2);                 // scope 0 at $DIR/issue-78192.rs:5:1: 5:2
+          return;                          // scope 0 at $DIR/issue-78192.rs:5:2: 5:2
+      }
+  }
+