Auto merge of #1083 - RalfJung:fn-call-helper, r=oli-obk

Add and use helper function for calling a machine function and passing it some arguments

src/eval.rs

@@ -6,13 +6,8 @@
 use rustc::hir::def_id::DefId;
 use rustc::ty::layout::{LayoutOf, Size};
 use rustc::ty::{self, TyCtxt};
-use syntax::source_map::DUMMY_SP;
 
-use crate::{
+use crate::*;
-    EnvVars, Evaluator, FnVal, HelpersEvalContextExt, InterpCx, InterpError,
-    InterpResult, MemoryExtra, MiriMemoryKind, Pointer, Scalar, StackPopCleanup, Tag,
-    TlsEvalContextExt, MPlaceTy
-};
 
 /// Configuration needed to spawn a Miri instance.
 #[derive(Clone)]
@@ -65,107 +60,93 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
         tcx.mk_substs(::std::iter::once(ty::subst::GenericArg::from(main_ret_ty))),
     )
     .unwrap();
-    let start_mir = ecx.load_mir(start_instance.def, None)?;
-
-    if start_mir.arg_count != 3 {
-        bug!(
-            "'start' lang item should have three arguments, but has {}",
-            start_mir.arg_count
-        );
-    }
-
-    // Return value (in static memory so that it does not count as leak).
-    let ret = ecx.layout_of(start_mir.return_ty())?;
-    let ret_ptr = ecx.allocate(ret, MiriMemoryKind::Static.into());
-
-    // Push our stack frame.
-    ecx.push_stack_frame(
-        start_instance,
-        // There is no call site.
-        DUMMY_SP,
-        start_mir,
-        Some(ret_ptr.into()),
-        StackPopCleanup::None { cleanup: true },
-    )?;
-
-    let mut args = ecx.frame().body.args_iter();
 
     // First argument: pointer to `main()`.
     let main_ptr = ecx
         .memory
         .create_fn_alloc(FnVal::Instance(main_instance));
-    let dest = ecx.local_place(args.next().unwrap())?;
+    // Second argument (argc): length of `config.args`.
-    ecx.write_scalar(Scalar::Ptr(main_ptr), dest)?;
+    let argc = Scalar::from_uint(config.args.len() as u128, ecx.pointer_size());
-
-    // Second argument (argc): `1`.
-    let dest = ecx.local_place(args.next().unwrap())?;
-    let argc = Scalar::from_uint(config.args.len() as u128, dest.layout.size);
-    ecx.write_scalar(argc, dest)?;
-    // Store argc for macOS's `_NSGetArgc`.
-    {
-        let argc_place = ecx.allocate(dest.layout, MiriMemoryKind::Env.into());
-        ecx.write_scalar(argc, argc_place.into())?;
-        ecx.machine.argc = Some(argc_place.ptr);
-    }
-
     // Third argument (`argv`): created from `config.args`.
-    let dest = ecx.local_place(args.next().unwrap())?;
+    let argv = {
-    // For Windows, construct a command string with all the aguments.
+        // For Windows, construct a command string with all the aguments (before we take apart `config.args`).
-    let mut cmd = String::new();
+        let mut cmd = String::new();
-    for arg in config.args.iter() {
+        for arg in config.args.iter() {
-        if !cmd.is_empty() {
+            if !cmd.is_empty() {
-            cmd.push(' ');
+                cmd.push(' ');
+            }
+            cmd.push_str(&*shell_escape::windows::escape(arg.as_str().into()));
         }
-        cmd.push_str(&*shell_escape::windows::escape(arg.as_str().into()));
+        // Don't forget `0` terminator.
-    }
+        cmd.push(std::char::from_u32(0).unwrap());
-    // Don't forget `0` terminator.
+        // Collect the pointers to the individual strings.
-    cmd.push(std::char::from_u32(0).unwrap());
+        let mut argvs = Vec::<Pointer<Tag>>::new();
-    // Collect the pointers to the individual strings.
+        for arg in config.args {
-    let mut argvs = Vec::<Pointer<Tag>>::new();
+            // Add `0` terminator.
-    for arg in config.args {
+            let mut arg = arg.into_bytes();
-        // Add `0` terminator.
+            arg.push(0);
-        let mut arg = arg.into_bytes();
+            argvs.push(
-        arg.push(0);
+                ecx.memory
-        argvs.push(
+                    .allocate_static_bytes(arg.as_slice(), MiriMemoryKind::Static.into()),
-            ecx.memory
+            );
-                .allocate_static_bytes(arg.as_slice(), MiriMemoryKind::Static.into()),
-        );
-    }
-    // Make an array with all these pointers, in the Miri memory.
-    let argvs_layout = ecx.layout_of(
-        tcx.mk_array(tcx.mk_imm_ptr(tcx.types.u8), argvs.len() as u64),
-    )?;
-    let argvs_place = ecx.allocate(argvs_layout, MiriMemoryKind::Env.into());
-    for (idx, arg) in argvs.into_iter().enumerate() {
-        let place = ecx.mplace_field(argvs_place, idx as u64)?;
-        ecx.write_scalar(Scalar::Ptr(arg), place.into())?;
-    }
-    ecx.memory
-        .mark_immutable(argvs_place.ptr.assert_ptr().alloc_id)?;
-    // Write a pointer to that place as the argument.
-    let argv = argvs_place.ptr;
-    ecx.write_scalar(argv, dest)?;
-    // Store `argv` for macOS `_NSGetArgv`.
-    {
-        let argv_place = ecx.allocate(dest.layout, MiriMemoryKind::Env.into());
-        ecx.write_scalar(argv, argv_place.into())?;
-        ecx.machine.argv = Some(argv_place.ptr);
-    }
-    // Store command line as UTF-16 for Windows `GetCommandLineW`.
-    {
-        let cmd_utf16: Vec<u16> = cmd.encode_utf16().collect();
-        let cmd_type = tcx.mk_array(tcx.types.u16, cmd_utf16.len() as u64);
-        let cmd_place = ecx.allocate(ecx.layout_of(cmd_type)?, MiriMemoryKind::Env.into());
-        ecx.machine.cmd_line = Some(cmd_place.ptr);
-        // Store the UTF-16 string. We just allocated so we know the bounds are fine.
-        let char_size = Size::from_bytes(2);
-        for (idx, &c) in cmd_utf16.iter().enumerate() {
-            let place = ecx.mplace_field(cmd_place, idx as u64)?;
-            ecx.write_scalar(Scalar::from_uint(c, char_size), place.into())?;
         }
-    }
+        // Make an array with all these pointers, in the Miri memory.
+        let argvs_layout = ecx.layout_of(
+            tcx.mk_array(tcx.mk_imm_ptr(tcx.types.u8), argvs.len() as u64),
+        )?;
+        let argvs_place = ecx.allocate(argvs_layout, MiriMemoryKind::Env.into());
+        for (idx, arg) in argvs.into_iter().enumerate() {
+            let place = ecx.mplace_field(argvs_place, idx as u64)?;
+            ecx.write_scalar(Scalar::Ptr(arg), place.into())?;
+        }
+        ecx.memory
+            .mark_immutable(argvs_place.ptr.assert_ptr().alloc_id)?;
+        // A pointer to that place is the argument.
+        let argv = argvs_place.ptr;
+        // Store `argc` and `argv` for macOS `_NSGetArg{c,v}`.
+        {
+            let argc_place = ecx.allocate(
+                ecx.layout_of(tcx.types.isize)?,
+                MiriMemoryKind::Env.into(),
+            );
+            ecx.write_scalar(argc, argc_place.into())?;
+            ecx.machine.argc = Some(argc_place.ptr);
 
-    args.next().expect_none("start lang item has more arguments than expected");
+            let argv_place = ecx.allocate(
+                ecx.layout_of(tcx.mk_imm_ptr(tcx.types.unit))?,
+                MiriMemoryKind::Env.into(),
+            );
+            ecx.write_scalar(argv, argv_place.into())?;
+            ecx.machine.argv = Some(argv_place.ptr);
+        }
+        // Store command line as UTF-16 for Windows `GetCommandLineW`.
+        {
+            let cmd_utf16: Vec<u16> = cmd.encode_utf16().collect();
+            let cmd_type = tcx.mk_array(tcx.types.u16, cmd_utf16.len() as u64);
+            let cmd_place = ecx.allocate(ecx.layout_of(cmd_type)?, MiriMemoryKind::Env.into());
+            ecx.machine.cmd_line = Some(cmd_place.ptr);
+            // Store the UTF-16 string. We just allocated so we know the bounds are fine.
+            let char_size = Size::from_bytes(2);
+            for (idx, &c) in cmd_utf16.iter().enumerate() {
+                let place = ecx.mplace_field(cmd_place, idx as u64)?;
+                ecx.write_scalar(Scalar::from_uint(c, char_size), place.into())?;
+            }
+        }
+        argv
+    };
+
+    // Return place (in static memory so that it does not count as leak).
+    let ret_place = ecx.allocate(
+        ecx.layout_of(tcx.types.isize)?,
+        MiriMemoryKind::Static.into(),
+    );
+    // Call start function.
+    ecx.call_function(
+        start_instance,
+        &[main_ptr.into(), argc, argv],
+        Some(ret_place.into()),
+        StackPopCleanup::None { cleanup: true },
+    )?;
 
     // Set the last_error to 0
     let errno_layout = ecx.layout_of(tcx.types.u32)?;
@@ -173,14 +154,14 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
     ecx.write_scalar(Scalar::from_u32(0), errno_place.into())?;
     ecx.machine.last_error = Some(errno_place);
 
-    Ok((ecx, ret_ptr))
+    Ok((ecx, ret_place))
 }
 
 /// Evaluates the main function specified by `main_id`.
 /// Returns `Some(return_code)` if program executed completed.
 /// Returns `None` if an evaluation error occured.
 pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) -> Option<i64> {
-    let (mut ecx, ret_ptr) = match create_ecx(tcx, main_id, config) {
+    let (mut ecx, ret_place) = match create_ecx(tcx, main_id, config) {
         Ok(v) => v,
         Err(mut err) => {
             err.print_backtrace();
@@ -193,7 +174,7 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
         ecx.run()?;
         // Read the return code pointer *before* we run TLS destructors, to assert
         // that it was written to by the time that `start` lang item returned.
-        let return_code = ecx.read_scalar(ret_ptr.into())?.not_undef()?.to_machine_isize(&ecx)?;
+        let return_code = ecx.read_scalar(ret_place.into())?.not_undef()?.to_machine_isize(&ecx)?;
         ecx.run_tls_dtors()?;
         Ok(return_code)
     })();

src/helpers.rs

@@ -1,6 +1,7 @@
 use std::{mem, iter};
 use std::ffi::{OsStr, OsString};
 
+use syntax::source_map::DUMMY_SP;
 use rustc::hir::def_id::{DefId, CRATE_DEF_INDEX};
 use rustc::mir;
 use rustc::ty::{
@@ -118,6 +119,40 @@ fn gen_random(
         this.memory.write_bytes(ptr, data.iter().copied())
     }
 
+    /// Call a function: Push the stack frame and pass the arguments.
+    /// For now, arguments must be scalars (so that the caller does not have to know the layout).
+    fn call_function(
+        &mut self,
+        f: ty::Instance<'tcx>,
+        args: &[Scalar<Tag>],
+        dest: Option<PlaceTy<'tcx, Tag>>,
+        stack_pop: StackPopCleanup,
+    ) -> InterpResult<'tcx> {
+        let this = self.eval_context_mut();
+
+        // Push frame.
+        let mir = this.load_mir(f.def, None)?;
+        this.push_stack_frame(
+            f,
+            DUMMY_SP, // There is no call site.
+            mir,
+            dest,
+            stack_pop,
+        )?;
+
+        // Initialize arguments.
+        let mut callee_args = this.frame().body.args_iter();
+        for arg in args {
+            let callee_arg = this.local_place(
+                callee_args.next().expect("callee has fewer arguments than expected")
+            )?;
+            this.write_scalar(*arg, callee_arg)?;
+        }
+        callee_args.next().expect_none("callee has more arguments than expected");
+
+        Ok(())
+    }
+
     /// Visits the memory covered by `place`, sensitive to freezing: the 3rd parameter
     /// will be true if this is frozen, false if this is in an `UnsafeCell`.
     fn visit_freeze_sensitive(

src/machine.rs

@@ -230,37 +230,26 @@ fn box_alloc(
         dest: PlaceTy<'tcx, Tag>,
     ) -> InterpResult<'tcx> {
         trace!("box_alloc for {:?}", dest.layout.ty);
+        let layout = ecx.layout_of(dest.layout.ty.builtin_deref(false).unwrap().ty)?;
+        // First argument: `size`.
+        // (`0` is allowed here -- this is expected to be handled by the lang item).
+        let size = Scalar::from_uint(layout.size.bytes(), ecx.pointer_size());
+
+        // Second argument: `align`.
+        let align = Scalar::from_uint(layout.align.abi.bytes(), ecx.pointer_size());
+
         // Call the `exchange_malloc` lang item.
         let malloc = ecx.tcx.lang_items().exchange_malloc_fn().unwrap();
         let malloc = ty::Instance::mono(ecx.tcx.tcx, malloc);
-        let malloc_mir = ecx.load_mir(malloc.def, None)?;
+        ecx.call_function(
-        ecx.push_stack_frame(
             malloc,
-            malloc_mir.span,
+            &[size, align],
-            malloc_mir,
             Some(dest),
             // Don't do anything when we are done. The `statement()` function will increment
             // the old stack frame's stmt counter to the next statement, which means that when
             // `exchange_malloc` returns, we go on evaluating exactly where we want to be.
             StackPopCleanup::None { cleanup: true },
         )?;
-
-        let mut args = ecx.frame().body.args_iter();
-        let layout = ecx.layout_of(dest.layout.ty.builtin_deref(false).unwrap().ty)?;
-
-        // First argument: `size`.
-        // (`0` is allowed here -- this is expected to be handled by the lang item).
-        let arg = ecx.local_place(args.next().unwrap())?;
-        let size = layout.size.bytes();
-        ecx.write_scalar(Scalar::from_uint(size, arg.layout.size), arg)?;
-
-        // Second argument: `align`.
-        let arg = ecx.local_place(args.next().unwrap())?;
-        let align = layout.align.abi.bytes();
-        ecx.write_scalar(Scalar::from_uint(align, arg.layout.size), arg)?;
-
-        // No more arguments.
-        args.next().expect_none("`exchange_malloc` lang item has more arguments than expected");
         Ok(())
     }
 

src/shims/panic.rs

@@ -37,7 +37,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
     /// Handles the special "miri_start_panic" intrinsic, which is called
     /// by libpanic_unwind to delegate the actual unwinding process to Miri.
-    #[inline(always)]
     fn handle_miri_start_panic(
         &mut self,
         args: &[OpTy<'tcx, Tag>],
@@ -57,7 +56,6 @@ fn handle_miri_start_panic(
         return Ok(())
     }
 
-    #[inline(always)]
     fn handle_catch_panic(
         &mut self,
         args: &[OpTy<'tcx, Tag>],
@@ -83,30 +81,16 @@ fn handle_catch_panic(
         // Now we make a function call, and pass `f_arg` as first and only argument.
         let f_instance = this.memory.get_fn(f)?.as_instance()?;
         trace!("__rust_maybe_catch_panic: {:?}", f_instance);
-        // TODO: consider making this reusable? `InterpCx::step` does something similar
-        // for the TLS destructors, and of course `eval_main`.
-        let mir = this.load_mir(f_instance.def, None)?;
         let ret_place =
             MPlaceTy::dangling(this.layout_of(tcx.mk_unit())?, this).into();
-        this.push_stack_frame(
+        this.call_function(
             f_instance,
-            mir.span,
+            &[f_arg],
-            mir,
             Some(ret_place),
             // Directly return to caller.
             StackPopCleanup::Goto { ret: Some(ret), unwind: None },
         )?;
 
-        let mut args = this.frame().body.args_iter();
-        // First argument.
-        let arg_local = args
-            .next()
-            .expect("Argument to __rust_maybe_catch_panic does not take enough arguments.");
-        let arg_dest = this.local_place(arg_local)?;
-        this.write_scalar(f_arg, arg_dest)?;
-        // No more arguments.
-        args.next().expect_none("__rust_maybe_catch_panic argument has more arguments than expected");
-
         // We ourselves will return `0`, eventually (will be overwritten if we catch a panic).
         this.write_null(dest)?;
 
@@ -124,7 +108,6 @@ fn handle_catch_panic(
         return Ok(());
     }
 
-    #[inline(always)]
     fn handle_stack_pop(
         &mut self,
         mut extra: FrameData<'tcx>,

src/shims/tls.rs

@@ -146,22 +146,14 @@ fn run_tls_dtors(&mut self) -> InterpResult<'tcx> {
         while let Some((instance, ptr, key)) = dtor {
             trace!("Running TLS dtor {:?} on {:?}", instance, ptr);
             assert!(!this.is_null(ptr).unwrap(), "Data can't be NULL when dtor is called!");
-            // TODO: Potentially, this has to support all the other possible instances?
+
-            // See eval_fn_call in interpret/terminator/mod.rs
-            let mir = this.load_mir(instance.def, None)?;
             let ret_place = MPlaceTy::dangling(this.layout_of(this.tcx.mk_unit())?, this).into();
-            this.push_stack_frame(
+            this.call_function(
                 instance,
-                mir.span,
+                &[ptr],
-                mir,
                 Some(ret_place),
                 StackPopCleanup::None { cleanup: true },
             )?;
-            let arg_local = this.frame().body.args_iter().next().ok_or_else(
-                || err_ub_format!("TLS dtor does not take enough arguments."),
-            )?;
-            let dest = this.local_place(arg_local)?;
-            this.write_scalar(ptr, dest)?;
 
             // step until out of stackframes
             this.run()?;