Fix ABI for Indirect arguments

In case of PassMode::Indirect, the ownership of the backing storage is
transfered to the callee. This means that the caller must copy the
argument if it wants to use it again later.

Fixes #691

Author: bjorn3

src/abi/mod.rs

@@ -235,27 +235,20 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
         // not mutated by the current function, this is necessary to support unsized arguments.
         if let ArgKind::Normal(Some(val)) = arg_kind {
             if let Some((addr, meta)) = val.try_to_ptr() {
-                let local_decl = &fx.mir.local_decls[local];
-                //                       v this ! is important
-                let internally_mutable = !val
-                    .layout()
-                    .ty
-                    .is_freeze(fx.tcx.at(local_decl.source_info.span), ParamEnv::reveal_all());
-                if local_decl.mutability == mir::Mutability::Not && !internally_mutable {
-                    // We wont mutate this argument, so it is fine to borrow the backing storage
-                    // of this argument, to prevent a copy.
-
-                    let place = if let Some(meta) = meta {
-                        CPlace::for_ptr_with_extra(addr, meta, val.layout())
-                    } else {
-                        CPlace::for_ptr(addr, val.layout())
-                    };
-
-                    self::comments::add_local_place_comments(fx, place, local);
-
-                    assert_eq!(fx.local_map.push(place), local);
-                    continue;
-                }
+                // Ownership of the value at the backing storage for an argument is passed to the
+                // callee per the ABI, so it is fine to borrow the backing storage of this argument
+                // to prevent a copy.
+
+                let place = if let Some(meta) = meta {
+                    CPlace::for_ptr_with_extra(addr, meta, val.layout())
+                } else {
+                    CPlace::for_ptr(addr, val.layout())
+                };
+
+                self::comments::add_local_place_comments(fx, place, local);
+
+                assert_eq!(fx.local_map.push(place), local);
+                continue;
             }
         }
 
@@ -291,6 +284,22 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
     fx.bcx.ins().jump(*fx.block_map.get(START_BLOCK).unwrap(), &[]);
 }
 
+struct CallArgument<'tcx> {
+    value: CValue<'tcx>,
+    is_owned: bool,
+}
+
+// FIXME avoid intermediate `CValue` before calling `adjust_arg_for_abi`
+fn codegen_call_argument_operand<'tcx>(
+    fx: &mut FunctionCx<'_, '_, 'tcx>,
+    operand: &Operand<'tcx>,
+) -> CallArgument<'tcx> {
+    CallArgument {
+        value: codegen_operand(fx, operand),
+        is_owned: matches!(operand, Operand::Move(_)),
+    }
+}
+
 pub(crate) fn codegen_terminator_call<'tcx>(
     fx: &mut FunctionCx<'_, '_, 'tcx>,
     span: Span,
@@ -361,28 +370,31 @@ pub(crate) fn codegen_terminator_call<'tcx>(
     // Unpack arguments tuple for closures
     let mut args = if fn_sig.abi == Abi::RustCall {
         assert_eq!(args.len(), 2, "rust-call abi requires two arguments");
-        let self_arg = codegen_operand(fx, &args[0]);
-        let pack_arg = codegen_operand(fx, &args[1]);
+        let self_arg = codegen_call_argument_operand(fx, &args[0]);
+        let pack_arg = codegen_call_argument_operand(fx, &args[1]);
 
-        let tupled_arguments = match pack_arg.layout().ty.kind() {
+        let tupled_arguments = match pack_arg.value.layout().ty.kind() {
             ty::Tuple(ref tupled_arguments) => tupled_arguments,
             _ => bug!("argument to function with \"rust-call\" ABI is not a tuple"),
         };
 
         let mut args = Vec::with_capacity(1 + tupled_arguments.len());
         args.push(self_arg);
         for i in 0..tupled_arguments.len() {
-            args.push(pack_arg.value_field(fx, mir::Field::new(i)));
+            args.push(CallArgument {
+                value: pack_arg.value.value_field(fx, mir::Field::new(i)),
+                is_owned: pack_arg.is_owned,
+            });
         }
         args
     } else {
-        args.iter().map(|arg| codegen_operand(fx, arg)).collect::<Vec<_>>()
+        args.iter().map(|arg| codegen_call_argument_operand(fx, arg)).collect::<Vec<_>>()
     };
 
     // Pass the caller location for `#[track_caller]`.
     if instance.map(|inst| inst.def.requires_caller_location(fx.tcx)).unwrap_or(false) {
         let caller_location = fx.get_caller_location(span);
-        args.push(caller_location);
+        args.push(CallArgument { value: caller_location, is_owned: false });
     }
 
     let args = args;
@@ -404,7 +416,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(
                 );
             }
 
-            let (ptr, method) = crate::vtable::get_ptr_and_method_ref(fx, args[0], idx);
+            let (ptr, method) = crate::vtable::get_ptr_and_method_ref(fx, args[0].value, idx);
             let sig = clif_sig_from_fn_abi(fx.tcx, fx.triple(), &fn_abi);
             let sig = fx.bcx.import_signature(sig);
 
@@ -441,7 +453,9 @@ pub(crate) fn codegen_terminator_call<'tcx>(
                 args.into_iter()
                     .enumerate()
                     .skip(if first_arg_override.is_some() { 1 } else { 0 })
-                    .map(|(i, arg)| adjust_arg_for_abi(fx, arg, &fn_abi.args[i]).into_iter())
+                    .map(|(i, arg)| {
+                        adjust_arg_for_abi(fx, arg.value, &fn_abi.args[i], arg.is_owned).into_iter()
+                    })
                     .flatten(),
             )
             .collect::<Vec<Value>>();
@@ -529,15 +543,15 @@ pub(crate) fn codegen_drop<'tcx>(
                         TypeAndMut { ty, mutbl: crate::rustc_hir::Mutability::Mut },
                     )),
                 );
-                let arg_value = adjust_arg_for_abi(fx, arg_value, &fn_abi.args[0]);
+                let arg_value = adjust_arg_for_abi(fx, arg_value, &fn_abi.args[0], true);
 
                 let mut call_args: Vec<Value> = arg_value.into_iter().collect::<Vec<_>>();
 
                 if drop_instance.def.requires_caller_location(fx.tcx) {
                     // Pass the caller location for `#[track_caller]`.
                     let caller_location = fx.get_caller_location(span);
                     call_args.extend(
-                        adjust_arg_for_abi(fx, caller_location, &fn_abi.args[1]).into_iter(),
+                        adjust_arg_for_abi(fx, caller_location, &fn_abi.args[1], false).into_iter(),
                     );
                 }
 

src/abi/pass_mode.rs

@@ -227,6 +227,7 @@ pub(super) fn adjust_arg_for_abi<'tcx>(
     fx: &mut FunctionCx<'_, '_, 'tcx>,
     arg: CValue<'tcx>,
     arg_abi: &ArgAbi<'tcx, Ty<'tcx>>,
+    is_owned: bool,
 ) -> SmallVec<[Value; 2]> {
     assert_assignable(fx, arg.layout().ty, arg_abi.layout.ty);
     match arg_abi.mode {
@@ -237,10 +238,21 @@ pub(super) fn adjust_arg_for_abi<'tcx>(
             smallvec![a, b]
         }
         PassMode::Cast(cast) => to_casted_value(fx, arg, cast),
-        PassMode::Indirect { .. } => match arg.force_stack(fx) {
-            (ptr, None) => smallvec![ptr.get_addr(fx)],
-            (ptr, Some(meta)) => smallvec![ptr.get_addr(fx), meta],
-        },
+        PassMode::Indirect { .. } => {
+            if is_owned {
+                match arg.force_stack(fx) {
+                    (ptr, None) => smallvec![ptr.get_addr(fx)],
+                    (ptr, Some(meta)) => smallvec![ptr.get_addr(fx), meta],
+                }
+            } else {
+                // Ownership of the value at the backing storage for an argument is passed to the
+                // callee per the ABI, so we must make a copy of the argument unless the argument
+                // local is moved.
+                let place = CPlace::new_stack_slot(fx, arg.layout());
+                place.write_cvalue(fx, arg);
+                smallvec![place.to_ptr().get_addr(fx)]
+            }
+        }
     }
 }