Auto merge of #1743 - RalfJung:callee-checks, r=RalfJung

Check callee ABI when Miri calls closures

Fixes #1741

Author: bors

src/eval.rs

@@ -8,6 +8,7 @@ use log::info;
 use rustc_hir::def_id::DefId;
 use rustc_middle::ty::{self, layout::LayoutCx, TyCtxt};
 use rustc_target::abi::LayoutOf;
+use rustc_target::spec::abi::Abi;
 
 use crate::*;
 
@@ -189,6 +190,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
     // Call start function.
     ecx.call_function(
         start_instance,
+        Abi::Rust,
         &[main_ptr.into(), argc.into(), argv.into()],
         Some(&ret_place.into()),
         StackPopCleanup::None { cleanup: true },

src/helpers.rs

@@ -161,11 +161,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     fn call_function(
         &mut self,
         f: ty::Instance<'tcx>,
+        caller_abi: Abi,
         args: &[Immediate<Tag>],
         dest: Option<&PlaceTy<'tcx, Tag>>,
         stack_pop: StackPopCleanup,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
+        let param_env = ty::ParamEnv::reveal_all(); // in Miri this is always the param_env we use... and this.param_env is private.
+        let callee_abi = f.ty(*this.tcx, param_env).fn_sig(*this.tcx).abi();
+        if callee_abi != caller_abi {
+            throw_ub_format!("calling a function with ABI {} using caller ABI {}", callee_abi.name(), caller_abi.name())
+        }
 
         // Push frame.
         let mir = &*this.load_mir(f.def, None)?;
@@ -175,11 +181,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         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"),
+                callee_args.next().ok_or_else(||
+                    err_ub_format!("callee has fewer arguments than expected")
+                )?
             )?;
             this.write_immediate(*arg, &callee_arg)?;
         }
-        assert_eq!(callee_args.next(), None, "callee has more arguments than expected");
+        if callee_args.next().is_some() {
+            throw_ub_format!("callee has more arguments than expected");
+        }
 
         Ok(())
     }

src/machine.rs

@@ -430,6 +430,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
         let malloc = ty::Instance::mono(ecx.tcx.tcx, malloc);
         ecx.call_function(
             malloc,
+            Abi::Rust,
             &[size.into(), align.into()],
             Some(dest),
             // Don't do anything when we are done. The `statement()` function will increment

src/shims/panic.rs

@@ -15,6 +15,7 @@ use log::trace;
 
 use rustc_middle::{mir, ty};
 use rustc_target::spec::PanicStrategy;
+use rustc_target::spec::abi::Abi;
 
 use crate::*;
 use helpers::check_arg_count;
@@ -96,6 +97,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let ret_place = MPlaceTy::dangling(this.machine.layouts.unit, this).into();
         this.call_function(
             f_instance,
+            Abi::Rust,
             &[data.into()],
             Some(&ret_place),
             // Directly return to caller.
@@ -147,6 +149,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             let ret_place = MPlaceTy::dangling(this.machine.layouts.unit, this).into();
             this.call_function(
                 f_instance,
+                Abi::Rust,
                 &[catch_unwind.data.into(), payload.into()],
                 Some(&ret_place),
                 // Directly return to caller of `try`.
@@ -176,6 +179,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let panic = ty::Instance::mono(this.tcx.tcx, panic);
         this.call_function(
             panic,
+            Abi::Rust,
             &[msg.to_ref()],
             None,
             StackPopCleanup::Goto { ret: None, unwind },
@@ -204,6 +208,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let panic_bounds_check = ty::Instance::mono(this.tcx.tcx, panic_bounds_check);
                 this.call_function(
                     panic_bounds_check,
+                    Abi::Rust,
                     &[index.into(), len.into()],
                     None,
                     StackPopCleanup::Goto { ret: None, unwind },

src/shims/posix/thread.rs

@@ -2,6 +2,7 @@ use std::convert::TryInto;
 
 use crate::*;
 use rustc_target::abi::LayoutOf;
+use rustc_target::spec::abi::Abi;
 
 impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
@@ -50,6 +51,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.call_function(
             instance,
+            Abi::C { unwind: false },
             &[*func_arg],
             Some(&ret_place.into()),
             StackPopCleanup::None { cleanup: true },

src/shims/tls.rs

@@ -9,6 +9,7 @@ use log::trace;
 use rustc_data_structures::fx::FxHashMap;
 use rustc_middle::ty;
 use rustc_target::abi::{Size, HasDataLayout};
+use rustc_target::spec::abi::Abi;
 
 use crate::*;
 
@@ -244,6 +245,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let ret_place = MPlaceTy::dangling(this.machine.layouts.unit, this).into();
         this.call_function(
             thread_callback,
+            Abi::System { unwind: false },
             &[Scalar::null_ptr(this).into(), reason.into(), Scalar::null_ptr(this).into()],
             Some(&ret_place),
             StackPopCleanup::None { cleanup: true },
@@ -266,6 +268,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             let ret_place = MPlaceTy::dangling(this.machine.layouts.unit, this).into();
             this.call_function(
                 instance,
+                Abi::C { unwind: false },
                 &[data.into()],
                 Some(&ret_place),
                 StackPopCleanup::None { cleanup: true },
@@ -306,6 +309,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             let ret_place = MPlaceTy::dangling(this.machine.layouts.unit, this).into();
             this.call_function(
                 instance,
+                Abi::C { unwind: false },
                 &[ptr.into()],
                 Some(&ret_place),
                 StackPopCleanup::None { cleanup: true },

tests/compile-fail/concurrency/too_few_args.rs

@@ -0,0 +1,26 @@
+// ignore-windows: Concurrency on Windows is not supported yet.
+// error-pattern: callee has fewer arguments than expected
+
+//! The thread function must have exactly one argument.
+
+#![feature(rustc_private)]
+
+extern crate libc;
+
+use std::{mem, ptr};
+
+extern "C" fn thread_start() -> *mut libc::c_void {
+    panic!()
+}
+
+fn main() {
+    unsafe {
+        let mut native: libc::pthread_t = mem::zeroed();
+        let attr: libc::pthread_attr_t = mem::zeroed();
+        // assert_eq!(libc::pthread_attr_init(&mut attr), 0); FIXME: this function is not yet implemented.
+        let thread_start: extern "C" fn() -> *mut libc::c_void = thread_start;
+        let thread_start: extern "C" fn(*mut libc::c_void) -> *mut libc::c_void = mem::transmute(thread_start);
+        assert_eq!(libc::pthread_create(&mut native, &attr, thread_start, ptr::null_mut()), 0);
+        assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
+    }
+}

tests/compile-fail/concurrency/too_many_args.rs

@@ -0,0 +1,26 @@
+// ignore-windows: Concurrency on Windows is not supported yet.
+// error-pattern: callee has more arguments than expected
+
+//! The thread function must have exactly one argument.
+
+#![feature(rustc_private)]
+
+extern crate libc;
+
+use std::{mem, ptr};
+
+extern "C" fn thread_start(_null: *mut libc::c_void, _x: i32) -> *mut libc::c_void {
+    panic!()
+}
+
+fn main() {
+    unsafe {
+        let mut native: libc::pthread_t = mem::zeroed();
+        let attr: libc::pthread_attr_t = mem::zeroed();
+        // assert_eq!(libc::pthread_attr_init(&mut attr), 0); FIXME: this function is not yet implemented.
+        let thread_start: extern "C" fn(*mut libc::c_void, i32) -> *mut libc::c_void = thread_start;
+        let thread_start: extern "C" fn(*mut libc::c_void) -> *mut libc::c_void = mem::transmute(thread_start);
+        assert_eq!(libc::pthread_create(&mut native, &attr, thread_start, ptr::null_mut()), 0);
+        assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
+    }
+}

tests/compile-fail/concurrency/unwind_top_of_stack.rs

@@ -1,7 +1,10 @@
 // ignore-windows: Concurrency on Windows is not supported yet.
-// error-pattern: unwinding past the topmost frame of the stack
+// error-pattern: calling a function with ABI C-unwind using caller ABI C
 
 //! Unwinding past the top frame of a stack is Undefined Behavior.
+//! However, it is impossible to do that in pure Rust since one cannot write an unwinding
+//! function with `C` ABI... so let's instead test that we are indeed correctly checking
+//! the callee ABI in `pthread_create`.
 
 #![feature(rustc_private, c_unwind)]