Auto merge of #45996 - eddyb:even-mirer-1, r=arielb1

MIR: hide .rodata constants vs by-ref ABI clash in trans.

Back in #45380, constants were copied into locals during MIR creation to ensure that arguments ' memory can be used by the callee, if the constant is placed in `.rodata` and the ABI passes it by-ref.

However, there are several drawbacks (see #45380 (comment)), most importantly the complication of constant propagation (UB if a constant ends up in `Call` arguments) and inconveniencing analyses.

Instead, I've modified the `rustc_trans` implementation of calls to copy an `Operand::Constant` argument locally if it's not immediate, and added a test that segfaults without the copy.

cc @dotdash @arielb1

Author: bors

src/librustc/mir/mod.rs

@@ -684,9 +684,10 @@ pub enum TerminatorKind<'tcx> {
     Call {
         /// The function that’s being called
         func: Operand<'tcx>,
-        /// Arguments the function is called with. These are owned by the callee, which is free to
-        /// modify them. This is important as "by-value" arguments might be passed by-reference at
-        /// the ABI level.
+        /// Arguments the function is called with.
+        /// These are owned by the callee, which is free to modify them.
+        /// This allows the memory occupied by "by-value" arguments to be
+        /// reused across function calls without duplicating the contents.
         args: Vec<Operand<'tcx>>,
         /// Destination for the return value. If some, the call is converging.
         destination: Option<(Lvalue<'tcx>, BasicBlock)>,

src/librustc_mir/build/expr/into.rs

@@ -247,13 +247,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 } else {
                     let args: Vec<_> =
                         args.into_iter()
-                            .map(|arg| {
-                                let scope = this.local_scope();
-                                // Function arguments are owned by the callee, so we need as_temp()
-                                // instead of as_operand() to enforce copies
-                                let operand = unpack!(block = this.as_temp(block, scope, arg));
-                                Operand::Consume(Lvalue::Local(operand))
-                            })
+                            .map(|arg| unpack!(block = this.as_local_operand(block, arg)))
                             .collect();
 
                     let success = this.cfg.start_new_block();

src/librustc_trans/mir/block.rs

@@ -524,7 +524,16 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
                         }
                     }
 
-                    let op = self.trans_operand(&bcx, arg);
+                    let mut op = self.trans_operand(&bcx, arg);
+
+                    // The callee needs to own the argument memory if we pass it
+                    // by-ref, so make a local copy of non-immediate constants.
+                    if let (&mir::Operand::Constant(_), Ref(..)) = (arg, op.val) {
+                        let tmp = LvalueRef::alloca(&bcx, op.ty, "const");
+                        self.store_operand(&bcx, tmp.llval, tmp.alignment.to_align(), op);
+                        op.val = Ref(tmp.llval, tmp.alignment);
+                    }
+
                     self.trans_argument(&bcx, op, &mut llargs, &fn_ty,
                                         &mut idx, &mut llfn, &def);
                 }

src/test/mir-opt/nll/liveness-call-subtlety.rs

@@ -31,21 +31,15 @@ fn main() {
 //            | Live variables at bb0[0]: []
 //        StorageLive(_1);
 //            | Live variables at bb0[1]: []
-//        StorageLive(_2);
-//            | Live variables at bb0[2]: []
-//        _2 = const 22usize;
-//            | Live variables at bb0[3]: [_2]
-//        _1 = const <std::boxed::Box<T>>::new(_2) -> bb1;
+//        _1 = const <std::boxed::Box<T>>::new(const 22usize) -> bb1;
 //    }
 // END rustc.main.nll.0.mir
 // START rustc.main.nll.0.mir
 //    | Live variables on entry to bb1: [_1 (drop)]
 //    bb1: {
 //            | Live variables at bb1[0]: [_1 (drop)]
-//        StorageDead(_2);
+//        StorageLive(_2);
 //            | Live variables at bb1[1]: [_1 (drop)]
-//        StorageLive(_3);
-//            | Live variables at bb1[2]: [_1 (drop)]
-//        _3 = const can_panic() -> [return: bb2, unwind: bb4];
+//        _2 = const can_panic() -> [return: bb2, unwind: bb4];
 //    }
 // END rustc.main.nll.0.mir

src/test/mir-opt/nll/region-liveness-drop-no-may-dangle.rs

@@ -46,5 +46,5 @@ impl<T> Drop for Wrap<T> {
 
 // END RUST SOURCE
 // START rustc.main.nll.0.mir
-// | '_#5r: {bb1[3], bb1[4], bb1[5], bb2[0], bb2[1], bb2[2], bb3[0], bb3[1], bb3[2], bb4[0], bb4[1], bb4[2], bb6[0], bb7[0], bb7[1], bb7[2], bb8[0]}
+// | '_#5r: {bb1[3], bb1[4], bb1[5], bb2[0], bb2[1], bb2[2], bb3[0], bb4[0], bb4[1], bb4[2], bb6[0], bb7[0], bb7[1], bb8[0]}
 // END rustc.main.nll.0.mir

src/test/mir-opt/nll/region-liveness-two-disjoint-uses.rs

@@ -45,5 +45,5 @@ fn main() {
 // ...
 // _2 = &'_#1r _1[_3];
 // ...
-// _2 = &'_#3r (*_11);
+// _2 = &'_#3r (*_10);
 // END rustc.main.nll.0.mir

src/test/run-pass/mir_trans_calls.rs

@@ -8,7 +8,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-#![feature(fn_traits)]
+#![feature(fn_traits, test)]
+
+extern crate test;
 
 fn test1(a: isize, b: (i32, i32), c: &[i32]) -> (isize, (i32, i32), &[i32]) {
     // Test passing a number of arguments including a fat pointer.
@@ -156,6 +158,16 @@ fn test_fn_nested_pair(x: &((f32, f32), u32)) -> (f32, f32) {
     (z.0, z.1)
 }
 
+fn test_fn_const_arg_by_ref(mut a: [u64; 4]) -> u64 {
+    // Mutate the by-reference argument, which won't work with
+    // a non-immediate constant unless it's copied to the stack.
+    let a = test::black_box(&mut a);
+    a[0] += a[1];
+    a[0] += a[2];
+    a[0] += a[3];
+    a[0]
+}
+
 fn main() {
     assert_eq!(test1(1, (2, 3), &[4, 5, 6]), (1, (2, 3), &[4, 5, 6][..]));
     assert_eq!(test2(98), 98);
@@ -182,4 +194,7 @@ fn main() {
     assert_eq!(test_fn_ignored_pair_0(), ());
     assert_eq!(test_fn_ignored_pair_named(), (Foo, Foo));
     assert_eq!(test_fn_nested_pair(&((1.0, 2.0), 0)), (1.0, 2.0));
+
+    const ARRAY: [u64; 4] = [1, 2, 3, 4];
+    assert_eq!(test_fn_const_arg_by_ref(ARRAY), 1 + 2 + 3 + 4);
 }