Invalid dereferences for all non-local mutations

Author: dianqk

compiler/rustc_mir_transform/src/gvn.rs

@@ -1728,12 +1728,18 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
         self.tcx
     }
 
-    fn visit_place(&mut self, place: &mut Place<'tcx>, _: PlaceContext, location: Location) {
+    fn visit_place(&mut self, place: &mut Place<'tcx>, context: PlaceContext, location: Location) {
         self.simplify_place_projection(place, location);
+        if context.is_mutating_use() && !place.projection.is_empty() {
+            // Non-local mutation maybe invalidate deref.
+            self.invalidate_derefs();
+        }
+        self.super_place(place, context, location);
     }
 
     fn visit_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) {
         self.simplify_operand(operand, location);
+        self.super_operand(operand, location);
     }
 
     fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, location: Location) {
@@ -1751,22 +1757,18 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
                 self.assign(local, value);
                 Some(value)
             } else {
-                // Non-local assignments maybe invalidate deref.
-                self.invalidate_derefs();
                 value
             };
-            let Some(value) = value else { return };
-
-            if let Some(const_) = self.try_as_constant(value) {
-                *rvalue = Rvalue::Use(Operand::Constant(Box::new(const_)));
-            } else if let Some(local) = self.try_as_local(value, location)
-                && *rvalue != Rvalue::Use(Operand::Move(local.into()))
-            {
-                *rvalue = Rvalue::Use(Operand::Copy(local.into()));
-                self.reused_locals.insert(local);
+            if let Some(value) = value {
+                if let Some(const_) = self.try_as_constant(value) {
+                    *rvalue = Rvalue::Use(Operand::Constant(Box::new(const_)));
+                } else if let Some(local) = self.try_as_local(value, location)
+                    && *rvalue != Rvalue::Use(Operand::Move(local.into()))
+                {
+                    *rvalue = Rvalue::Use(Operand::Copy(local.into()));
+                    self.reused_locals.insert(local);
+                }
             }
-
-            return;
         }
         self.super_statement(stmt, location);
     }

tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.32bit.diff

@@ -10,8 +10,9 @@
           StorageLive(_1);
           StorageLive(_2);
 -         _2 = ();
+-         _1 = Union32 { value: move _2 };
 +         _2 = const ();
-          _1 = Union32 { value: move _2 };
++         _1 = Union32 { value: const () };
           StorageDead(_2);
           _0 = move _1 as u32 (Transmute);
           StorageDead(_1);

tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.64bit.diff

@@ -10,8 +10,9 @@
           StorageLive(_1);
           StorageLive(_2);
 -         _2 = ();
+-         _1 = Union32 { value: move _2 };
 +         _2 = const ();
-          _1 = Union32 { value: move _2 };
++         _1 = Union32 { value: const () };
           StorageDead(_2);
           _0 = move _1 as u32 (Transmute);
           StorageDead(_1);

tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-abort.diff

@@ -5,25 +5,21 @@
       let mut _0: ();
       let _1: main::Un;
       let mut _2: u32;
-      let mut _3: u32;
       scope 1 {
           debug un => _1;
           scope 3 (inlined std::mem::drop::<u32>) {
-              debug _x => _3;
+              debug _x => _2;
           }
       }
       scope 2 (inlined val) {
       }
 
       bb0: {
           StorageLive(_1);
-          StorageLive(_2);
-          nop;
           _1 = Un { us: const 1_u32 };
+          StorageLive(_2);
+          _2 = copy (_1.0: u32);
           StorageDead(_2);
-          StorageLive(_3);
-          _3 = copy (_1.0: u32);
-          StorageDead(_3);
           StorageDead(_1);
           return;
       }

tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-unwind.diff

@@ -5,25 +5,21 @@
       let mut _0: ();
       let _1: main::Un;
       let mut _2: u32;
-      let mut _3: u32;
       scope 1 {
           debug un => _1;
           scope 3 (inlined std::mem::drop::<u32>) {
-              debug _x => _3;
+              debug _x => _2;
           }
       }
       scope 2 (inlined val) {
       }
 
       bb0: {
           StorageLive(_1);
-          StorageLive(_2);
-          nop;
           _1 = Un { us: const 1_u32 };
+          StorageLive(_2);
+          _2 = copy (_1.0: u32);
           StorageDead(_2);
-          StorageLive(_3);
-          _3 = copy (_1.0: u32);
-          StorageDead(_3);
           StorageDead(_1);
           return;
       }

tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff

@@ -20,7 +20,7 @@
           StorageLive(_2);
           StorageLive(_3);
           _3 = &(*_1);
-          _2 = get(move _3) -> [return: bb1, unwind unreachable];
+          _2 = get::<Option<i32>>(move _3) -> [return: bb1, unwind unreachable];
       }
 
       bb1: {

tests/mir-opt/simplify_aggregate_to_copy_miscompile.rs

@@ -9,15 +9,16 @@
 #![crate_type = "lib"]
 //@ test-mir-pass: GVN
 #![allow(internal_features)]
-#![feature(rustc_attrs, core_intrinsics)]
+#![feature(core_intrinsics, custom_mir, rustc_attrs)]
+
+use std::intrinsics::mir::*;
 
 // EMIT_MIR simplify_aggregate_to_copy_miscompile.foo.GVN.diff
-#[no_mangle]
 fn foo(v: &mut Option<i32>) -> Option<i32> {
     // CHECK-LABEL: fn foo(
     // CHECK-SAME: [[v:_.*]]: &mut Option<i32>
     // CHECK: [[v_alias_1:_.*]] = &(*_1)
-    // CHECK-NEXT: [[v_alias_2:_.*]] = get(move [[v_alias_1]])
+    // CHECK-NEXT: [[v_alias_2:_.*]] = get::<Option<i32>>(move [[v_alias_1]])
     // CHECK: (*[[v]]) = const Option::<i32>::None;
     // CHECK-NOT: _0 = copy (*[[v_alias_2]])
     // CHECK: _0 = Option::<i32>::Some
@@ -30,9 +31,31 @@ fn foo(v: &mut Option<i32>) -> Option<i32> {
     }
 }
 
-#[no_mangle]
+pub enum Value {
+    V0(i32),
+    V1(i32),
+}
+
+// EMIT_MIR simplify_aggregate_to_copy_miscompile.set_discriminant.GVN.diff
+#[custom_mir(dialect = "runtime", phase = "initial")]
+fn set_discriminant(v: &mut Value) -> Value {
+    // CHECK-LABEL: fn set_discriminant(
+    mir! {
+        let v_: &Value;
+        {
+            Call(v_ = get(v), ReturnTo(ret), UnwindUnreachable())
+        }
+        ret = {
+            let col: i32 = Field(Variant(*v_, 0), 0);
+            SetDiscriminant(*v, 1);
+            RET = Value::V0(col);
+            Return()
+        }
+    }
+}
+
 #[inline(never)]
 #[rustc_nounwind]
-fn get(v: &Option<i32>) -> &Option<i32> {
+fn get<T>(v: &T) -> &T {
     v
 }

tests/mir-opt/simplify_aggregate_to_copy_miscompile.set_discriminant.GVN.diff

@@ -0,0 +1,20 @@
+- // MIR for `set_discriminant` before GVN
++ // MIR for `set_discriminant` after GVN
+  
+  fn set_discriminant(_1: &mut Value) -> Value {
+      let mut _0: Value;
+      let mut _2: &Value;
+      let mut _3: i32;
+  
+      bb0: {
+          _2 = get::<Value>(copy _1) -> [return: bb1, unwind unreachable];
+      }
+  
+      bb1: {
+          _3 = copy (((*_2) as variant#0).0: i32);
+          discriminant((*_1)) = 1;
+          _0 = Value::V0(copy _3);
+          return;
+      }
+  }
+