Rollup merge of #145030 - cjgillot:gvn-no-flatten-index, r=saethlin

GVN:  Do not flatten derefs with ProjectionElem::Index.

r? `@saethlin`

This should fix the bug you found with #131650

Author: Zalathar

compiler/rustc_mir_transform/src/gvn.rs

@@ -756,7 +756,13 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                 && let Some(v) = self.simplify_place_value(&mut pointee, location)
             {
                 value = v;
-                place_ref = pointee.project_deeper(&place.projection[index..], self.tcx).as_ref();
+                // `pointee` holds a `Place`, so `ProjectionElem::Index` holds a `Local`.
+                // That local is SSA, but we otherwise have no guarantee on that local's value at
+                // the current location compared to its value where `pointee` was borrowed.
+                if pointee.projection.iter().all(|elem| !matches!(elem, ProjectionElem::Index(_))) {
+                    place_ref =
+                        pointee.project_deeper(&place.projection[index..], self.tcx).as_ref();
+                }
             }
             if let Some(local) = self.try_as_local(value, location) {
                 // Both `local` and `Place { local: place.local, projection: projection[..index] }`
@@ -774,7 +780,12 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             && let Some(v) = self.simplify_place_value(&mut pointee, location)
         {
             value = v;
-            place_ref = pointee.project_deeper(&[], self.tcx).as_ref();
+            // `pointee` holds a `Place`, so `ProjectionElem::Index` holds a `Local`.
+            // That local is SSA, but we otherwise have no guarantee on that local's value at
+            // the current location compared to its value where `pointee` was borrowed.
+            if pointee.projection.iter().all(|elem| !matches!(elem, ProjectionElem::Index(_))) {
+                place_ref = pointee.project_deeper(&[], self.tcx).as_ref();
+            }
         }
         if let Some(new_local) = self.try_as_local(value, location) {
             place_ref = PlaceRef { local: new_local, projection: &[] };

tests/mir-opt/gvn.dereference_indexing.GVN.panic-abort.diff

@@ -0,0 +1,59 @@
+- // MIR for `dereference_indexing` before GVN
++ // MIR for `dereference_indexing` after GVN
+  
+  fn dereference_indexing(_1: [u8; 2], _2: usize) -> () {
+      debug array => _1;
+      debug index => _2;
+      let mut _0: ();
+      let _3: &u8;
+      let _4: usize;
+      let mut _5: usize;
+      let _6: usize;
+      let mut _7: bool;
+      let _8: ();
+      let mut _9: u8;
+      scope 1 {
+          debug a => _3;
+      }
+      scope 2 {
+          debug i => _4;
+      }
+  
+      bb0: {
+          StorageLive(_3);
+-         StorageLive(_4);
++         nop;
+          StorageLive(_5);
+          _5 = copy _2;
+-         _4 = Add(move _5, const 1_usize);
++         _4 = Add(copy _2, const 1_usize);
+          StorageDead(_5);
+          StorageLive(_6);
+          _6 = copy _4;
+-         _7 = Lt(copy _6, const 2_usize);
+-         assert(move _7, "index out of bounds: the length is {} but the index is {}", const 2_usize, copy _6) -> [success: bb1, unwind unreachable];
++         _7 = Lt(copy _4, const 2_usize);
++         assert(move _7, "index out of bounds: the length is {} but the index is {}", const 2_usize, copy _4) -> [success: bb1, unwind unreachable];
+      }
+  
+      bb1: {
+-         _3 = &_1[_6];
+-         StorageDead(_4);
++         _3 = &_1[_4];
++         nop;
+          StorageLive(_8);
+          StorageLive(_9);
+          _9 = copy (*_3);
+          _8 = opaque::<u8>(move _9) -> [return: bb2, unwind unreachable];
+      }
+  
+      bb2: {
+          StorageDead(_9);
+          StorageDead(_8);
+          _0 = const ();
+          StorageDead(_6);
+          StorageDead(_3);
+          return;
+      }
+  }
+  

tests/mir-opt/gvn.dereference_indexing.GVN.panic-unwind.diff

@@ -0,0 +1,59 @@
+- // MIR for `dereference_indexing` before GVN
++ // MIR for `dereference_indexing` after GVN
+  
+  fn dereference_indexing(_1: [u8; 2], _2: usize) -> () {
+      debug array => _1;
+      debug index => _2;
+      let mut _0: ();
+      let _3: &u8;
+      let _4: usize;
+      let mut _5: usize;
+      let _6: usize;
+      let mut _7: bool;
+      let _8: ();
+      let mut _9: u8;
+      scope 1 {
+          debug a => _3;
+      }
+      scope 2 {
+          debug i => _4;
+      }
+  
+      bb0: {
+          StorageLive(_3);
+-         StorageLive(_4);
++         nop;
+          StorageLive(_5);
+          _5 = copy _2;
+-         _4 = Add(move _5, const 1_usize);
++         _4 = Add(copy _2, const 1_usize);
+          StorageDead(_5);
+          StorageLive(_6);
+          _6 = copy _4;
+-         _7 = Lt(copy _6, const 2_usize);
+-         assert(move _7, "index out of bounds: the length is {} but the index is {}", const 2_usize, copy _6) -> [success: bb1, unwind continue];
++         _7 = Lt(copy _4, const 2_usize);
++         assert(move _7, "index out of bounds: the length is {} but the index is {}", const 2_usize, copy _4) -> [success: bb1, unwind continue];
+      }
+  
+      bb1: {
+-         _3 = &_1[_6];
+-         StorageDead(_4);
++         _3 = &_1[_4];
++         nop;
+          StorageLive(_8);
+          StorageLive(_9);
+          _9 = copy (*_3);
+          _8 = opaque::<u8>(move _9) -> [return: bb2, unwind continue];
+      }
+  
+      bb2: {
+          StorageDead(_9);
+          StorageDead(_8);
+          _0 = const ();
+          StorageDead(_6);
+          StorageDead(_3);
+          return;
+      }
+  }
+  

tests/mir-opt/gvn.rs

@@ -1052,6 +1052,26 @@ fn remove_casts_must_change_both_sides(mut_a: &*mut u8, mut_b: *mut u8) -> bool
     }
 }
 
+/// Verify that we do not references to non-existing locals when dereferencing projections.
+fn dereference_indexing(array: [u8; 2], index: usize) {
+    // CHECK-LABEL: fn dereference_indexing(
+    // CHECK: debug a => [[a:_.*]];
+    // CHECK: debug i => [[i:_.*]];
+
+    let a = {
+        // CHECK: [[i]] = Add(copy _2, const 1_usize);
+        let i = index + 1;
+        // CHECK: [[a]] = &_1[[[i]]];
+        &array[i]
+    };
+
+    // CHECK-NOT: [{{.*}}]
+    // CHECK: [[tmp:_.*]] = copy (*[[a]]);
+    // CHECK: opaque::<u8>(move [[tmp]])
+    opaque(*a);
+}
+
+// CHECK-LABEL: fn main(
 fn main() {
     subexpression_elimination(2, 4, 5);
     wrap_unwrap(5);
@@ -1079,6 +1099,7 @@ fn main() {
     slice_const_length(&[1]);
     meta_of_ref_to_slice(&42);
     slice_from_raw_parts_as_ptr(&123, 456);
+    dereference_indexing([129, 14], 5);
 }
 
 #[inline(never)]
@@ -1138,3 +1159,4 @@ enum Never {}
 // EMIT_MIR gvn.cast_pointer_then_transmute.GVN.diff
 // EMIT_MIR gvn.transmute_then_cast_pointer.GVN.diff
 // EMIT_MIR gvn.remove_casts_must_change_both_sides.GVN.diff
+// EMIT_MIR gvn.dereference_indexing.GVN.diff