Rollup merge of #107971 - saethlin:mir-opt-ub, r=cjgillot

Clearly document intentional UB in mir-opt tests

All of the changed mir-opt test input files did not pass Miri. Now they do.

r? `@cjgillot` because there's a CopyProp test in here that I do not fully understand

Author: matthiaskrgr

tests/mir-opt/copy-prop/mutate_through_pointer.rs

@@ -1,3 +1,13 @@
+// This attempts to mutate `a` via a pointer derived from `addr_of!(a)`. That is UB
+// according to Miri. However, the decision to make this UB - and to allow
+// rustc to rely on that fact for the purpose of optimizations - has not been
+// finalized.
+//
+// As such, we include this test to ensure that copy prop does not rely on that
+// fact. Specifically, if `addr_of!(a)` could not be used to modify a, it would
+// be correct for CopyProp to replace all occurrences of `a` with `c` - but that
+// would cause `f(true)` to output `false` instead of `true`.
+
 #![feature(custom_mir, core_intrinsics)]
 #![allow(unused_assignments)]
 extern crate core;

tests/mir-opt/dataflow-const-prop/sibling_ptr.main.DataflowConstProp.diff

@@ -32,7 +32,7 @@
           _5 = _3;                         // scope 3 at $DIR/sibling_ptr.rs:+4:10: +4:11
           _4 = ptr::mut_ptr::<impl *mut u8>::add(move _5, const 1_usize) -> bb1; // scope 3 at $DIR/sibling_ptr.rs:+4:10: +4:18
                                            // mir::Constant
-                                           // + span: $DIR/sibling_ptr.rs:8:12: 8:15
+                                           // + span: $DIR/sibling_ptr.rs:15:12: 15:15
                                            // + literal: Const { ty: unsafe fn(*mut u8, usize) -> *mut u8 {ptr::mut_ptr::<impl *mut u8>::add}, val: Value(<ZST>) }
       }
 

tests/mir-opt/dataflow-const-prop/sibling_ptr.rs

@@ -1,3 +1,10 @@
+// This attempts to modify `x.1` via a pointer derived from `addr_of_mut!(x.0)`.
+// According to Miri, that is UB. However, T-opsem has not finalized that
+// decision and as such we cannot rely on it in optimizations. Consequently,
+// DataflowConstProp must treat the `addr_of_mut!(x.0)` as potentially being
+// used to modify `x.1` - if it did not, then it might incorrectly assume that it
+// can infer the value of `x.1` at the end of this function.
+
 // unit-test: DataflowConstProp
 
 // EMIT_MIR sibling_ptr.main.DataflowConstProp.diff
@@ -7,5 +14,5 @@ fn main() {
         let p = std::ptr::addr_of_mut!(x.0);
         *p.add(1) = 1;
     }
-    let x1 = x.1;  // should not be propagated
+    let x1 = x.1; // should not be propagated
 }

tests/mir-opt/sroa.escaping.ScalarReplacementOfAggregates.diff

@@ -3,42 +3,42 @@
 
   fn escaping() -> () {
       let mut _0: ();                      // return place in scope 0 at $DIR/sroa.rs:+0:19: +0:19
-      let _1: ();                          // in scope 0 at $DIR/sroa.rs:+2:5: +2:42
-      let mut _2: *const u32;              // in scope 0 at $DIR/sroa.rs:+2:7: +2:41
-      let _3: &u32;                        // in scope 0 at $DIR/sroa.rs:+2:7: +2:41
-      let _4: Escaping;                    // in scope 0 at $DIR/sroa.rs:+2:8: +2:39
-      let mut _5: u32;                     // in scope 0 at $DIR/sroa.rs:+2:34: +2:37
+      let _1: ();                          // in scope 0 at $DIR/sroa.rs:+1:5: +1:42
+      let mut _2: *const u32;              // in scope 0 at $DIR/sroa.rs:+1:7: +1:41
+      let _3: &u32;                        // in scope 0 at $DIR/sroa.rs:+1:7: +1:41
+      let _4: Escaping;                    // in scope 0 at $DIR/sroa.rs:+1:8: +1:39
+      let mut _5: u32;                     // in scope 0 at $DIR/sroa.rs:+1:34: +1:37
 
       bb0: {
-          StorageLive(_1);                 // scope 0 at $DIR/sroa.rs:+2:5: +2:42
-          StorageLive(_2);                 // scope 0 at $DIR/sroa.rs:+2:7: +2:41
-          StorageLive(_3);                 // scope 0 at $DIR/sroa.rs:+2:7: +2:41
-          StorageLive(_4);                 // scope 0 at $DIR/sroa.rs:+2:8: +2:39
-          StorageLive(_5);                 // scope 0 at $DIR/sroa.rs:+2:34: +2:37
-          _5 = g() -> bb1;                 // scope 0 at $DIR/sroa.rs:+2:34: +2:37
+          StorageLive(_1);                 // scope 0 at $DIR/sroa.rs:+1:5: +1:42
+          StorageLive(_2);                 // scope 0 at $DIR/sroa.rs:+1:7: +1:41
+          StorageLive(_3);                 // scope 0 at $DIR/sroa.rs:+1:7: +1:41
+          StorageLive(_4);                 // scope 0 at $DIR/sroa.rs:+1:8: +1:39
+          StorageLive(_5);                 // scope 0 at $DIR/sroa.rs:+1:34: +1:37
+          _5 = g() -> bb1;                 // scope 0 at $DIR/sroa.rs:+1:34: +1:37
                                            // mir::Constant
-                                           // + span: $DIR/sroa.rs:73:34: 73:35
+                                           // + span: $DIR/sroa.rs:78:34: 78:35
                                            // + literal: Const { ty: fn() -> u32 {g}, val: Value(<ZST>) }
       }
 
       bb1: {
-          _4 = Escaping { a: const 1_u32, b: const 2_u32, c: move _5 }; // scope 0 at $DIR/sroa.rs:+2:8: +2:39
-          StorageDead(_5);                 // scope 0 at $DIR/sroa.rs:+2:38: +2:39
-          _3 = &(_4.0: u32);               // scope 0 at $DIR/sroa.rs:+2:7: +2:41
-          _2 = &raw const (*_3);           // scope 0 at $DIR/sroa.rs:+2:7: +2:41
-          _1 = f(move _2) -> bb2;          // scope 0 at $DIR/sroa.rs:+2:5: +2:42
+          _4 = Escaping { a: const 1_u32, b: const 2_u32, c: move _5 }; // scope 0 at $DIR/sroa.rs:+1:8: +1:39
+          StorageDead(_5);                 // scope 0 at $DIR/sroa.rs:+1:38: +1:39
+          _3 = &(_4.0: u32);               // scope 0 at $DIR/sroa.rs:+1:7: +1:41
+          _2 = &raw const (*_3);           // scope 0 at $DIR/sroa.rs:+1:7: +1:41
+          _1 = f(move _2) -> bb2;          // scope 0 at $DIR/sroa.rs:+1:5: +1:42
                                            // mir::Constant
-                                           // + span: $DIR/sroa.rs:73:5: 73:6
+                                           // + span: $DIR/sroa.rs:78:5: 78:6
                                            // + literal: Const { ty: fn(*const u32) {f}, val: Value(<ZST>) }
       }
 
       bb2: {
-          StorageDead(_2);                 // scope 0 at $DIR/sroa.rs:+2:41: +2:42
-          StorageDead(_4);                 // scope 0 at $DIR/sroa.rs:+2:42: +2:43
-          StorageDead(_3);                 // scope 0 at $DIR/sroa.rs:+2:42: +2:43
-          StorageDead(_1);                 // scope 0 at $DIR/sroa.rs:+2:42: +2:43
-          _0 = const ();                   // scope 0 at $DIR/sroa.rs:+0:19: +3:2
-          return;                          // scope 0 at $DIR/sroa.rs:+3:2: +3:2
+          StorageDead(_2);                 // scope 0 at $DIR/sroa.rs:+1:41: +1:42
+          StorageDead(_4);                 // scope 0 at $DIR/sroa.rs:+1:42: +1:43
+          StorageDead(_3);                 // scope 0 at $DIR/sroa.rs:+1:42: +1:43
+          StorageDead(_1);                 // scope 0 at $DIR/sroa.rs:+1:42: +1:43
+          _0 = const ();                   // scope 0 at $DIR/sroa.rs:+0:19: +2:2
+          return;                          // scope 0 at $DIR/sroa.rs:+2:2: +2:2
       }
   }
 

tests/mir-opt/sroa.rs

@@ -68,8 +68,13 @@ fn f(a: *const u32) {
     println!("{}", unsafe { *a.add(2) });
 }
 
+// `f` uses the `&e.a` to access `e.c`. This is UB according to Miri today; however,
+// T-opsem has not finalized that decision and as such rustc should not rely on
+// it. If SROA were to rely on it, it would be (almost) correct to turn `e` into
+// three distinct locals - one for each field - and pass a reference to only one
+// of them to `f`. However, this would lead to a miscompilation because `b` and `c`
+// might no longer appear right after `a` in memory.
 pub fn escaping() {
-    // Verify this struct is not flattened.
     f(&Escaping { a: 1, b: 2, c: g() }.a);
 }