Merge pull request #4185 from geetanshjuneja/abi_check

FnAbi Compatability check

Author: RalfJung

src/tools/miri/src/helpers.rs

@@ -14,7 +14,7 @@ use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::middle::dependency_format::Linkage;
 use rustc_middle::middle::exported_symbols::ExportedSymbol;
 use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, MaybeResult, TyAndLayout};
-use rustc_middle::ty::{self, FloatTy, IntTy, Ty, TyCtxt, UintTy};
+use rustc_middle::ty::{self, Binder, FloatTy, FnSig, IntTy, Ty, TyCtxt, UintTy};
 use rustc_session::config::CrateType;
 use rustc_span::{Span, Symbol};
 use rustc_target::callconv::{Conv, FnAbi};
@@ -994,10 +994,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         exp_abi: Conv,
         link_name: Symbol,
         args: &'a [OpTy<'tcx>],
-    ) -> InterpResult<'tcx, &'a [OpTy<'tcx>; N]>
-    where
-        &'a [OpTy<'tcx>; N]: TryFrom<&'a [OpTy<'tcx>]>,
-    {
+    ) -> InterpResult<'tcx, &'a [OpTy<'tcx>; N]> {
         self.check_abi_and_shim_symbol_clash(abi, exp_abi, link_name)?;
 
         if abi.c_variadic {
@@ -1015,6 +1012,80 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         )
     }
 
+    /// Check that the given `caller_fn_abi` matches the expected ABI described by
+    /// `callee_abi`, `callee_input_tys`, `callee_output_ty`, and the return the list of
+    /// arguments.
+    fn check_shim_abi<'a, const N: usize>(
+        &mut self,
+        link_name: Symbol,
+        caller_fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
+        callee_abi: ExternAbi,
+        callee_input_tys: [Ty<'tcx>; N],
+        callee_output_ty: Ty<'tcx>,
+        caller_args: &'a [OpTy<'tcx>],
+    ) -> InterpResult<'tcx, &'a [OpTy<'tcx>; N]> {
+        let this = self.eval_context_mut();
+        let mut inputs_and_output = callee_input_tys.to_vec();
+        inputs_and_output.push(callee_output_ty);
+        let fn_sig_binder = Binder::dummy(FnSig {
+            inputs_and_output: this.machine.tcx.mk_type_list(&inputs_and_output),
+            c_variadic: false,
+            // This does not matter for the ABI.
+            safety: Safety::Safe,
+            abi: callee_abi,
+        });
+        let callee_fn_abi = this.fn_abi_of_fn_ptr(fn_sig_binder, Default::default())?;
+
+        this.check_abi_and_shim_symbol_clash(caller_fn_abi, callee_fn_abi.conv, link_name)?;
+
+        if caller_fn_abi.c_variadic {
+            throw_ub_format!(
+                "ABI mismatch: calling a non-variadic function with a variadic caller-side signature"
+            );
+        }
+
+        if callee_fn_abi.fixed_count != caller_fn_abi.fixed_count {
+            throw_ub_format!(
+                "ABI mismatch: expected {} arguments, found {} arguments ",
+                callee_fn_abi.fixed_count,
+                caller_fn_abi.fixed_count
+            );
+        }
+
+        if callee_fn_abi.can_unwind && !caller_fn_abi.can_unwind {
+            throw_ub_format!(
+                "ABI mismatch: callee may unwind, but caller-side signature prohibits unwinding",
+            );
+        }
+
+        if !this.check_argument_compat(&caller_fn_abi.ret, &callee_fn_abi.ret)? {
+            throw_ub!(AbiMismatchReturn {
+                caller_ty: caller_fn_abi.ret.layout.ty,
+                callee_ty: callee_fn_abi.ret.layout.ty
+            });
+        }
+
+        if let Some(index) = caller_fn_abi
+            .args
+            .iter()
+            .zip(callee_fn_abi.args.iter())
+            .map(|(caller_arg, callee_arg)| this.check_argument_compat(caller_arg, callee_arg))
+            .collect::<InterpResult<'tcx, Vec<bool>>>()?
+            .into_iter()
+            .position(|b| !b)
+        {
+            throw_ub!(AbiMismatchArgument {
+                caller_ty: caller_fn_abi.args[index].layout.ty,
+                callee_ty: callee_fn_abi.args[index].layout.ty
+            });
+        }
+
+        if let Ok(ops) = caller_args.try_into() {
+            return interp_ok(ops);
+        }
+        unreachable!()
+    }
+
     /// Check shim for variadic function.
     /// Returns a tuple that consisting of an array of fixed args, and a slice of varargs.
     fn check_shim_variadic<'a, const N: usize>(

src/tools/miri/src/shims/unix/foreign_items.rs

@@ -1,7 +1,7 @@
 use std::ffi::OsStr;
 use std::str;
 
-use rustc_abi::Size;
+use rustc_abi::{ExternAbi, Size};
 use rustc_middle::ty::Ty;
 use rustc_middle::ty::layout::LayoutOf;
 use rustc_span::Symbol;
@@ -200,7 +200,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                 this.write(fd, buf, count, Some(offset), dest)?;
             }
             "close" => {
-                let [fd] = this.check_shim(abi, Conv::C, link_name, args)?;
+                let [fd] = this.check_shim_abi(
+                    link_name,
+                    abi,
+                    ExternAbi::C { unwind: false },
+                    [this.tcx.types.i32],
+                    this.tcx.types.i32,
+                    args,
+                )?;
                 let result = this.close(fd)?;
                 this.write_scalar(result, dest)?;
             }

src/tools/miri/tests/fail/shims/input_arg_mismatch.rs

@@ -0,0 +1,21 @@
+//@ignore-target: windows # File handling is not implemented yet
+//@compile-flags: -Zmiri-disable-isolation
+use std::ffi::{CString, OsStr, c_char, c_int};
+use std::os::unix::ffi::OsStrExt;
+
+extern "C" {
+    fn open(path: *const c_char, oflag: c_int, ...) -> c_int;
+    // correct fd type is i32
+    fn close(fd: u32) -> c_int;
+}
+
+fn main() {
+    let c_path = CString::new(OsStr::new("./text").as_bytes()).expect("CString::new failed");
+    let fd = unsafe {
+        open(c_path.as_ptr(), /* value does not matter */ 0)
+    } as u32;
+    let _ = unsafe {
+        close(fd);
+        //~^ ERROR: calling a function with argument of type i32 passing data of type u32
+    };
+}

src/tools/miri/tests/fail/shims/input_arg_mismatch.stderr

@@ -0,0 +1,17 @@
+error: Undefined Behavior: calling a function with argument of type i32 passing data of type u32
+  --> tests/fail/shims/input_arg_mismatch.rs:LL:CC
+   |
+LL |         close(fd);
+   |         ^^^^^^^^^ calling a function with argument of type i32 passing data of type u32
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets
+   = help: if you think this code should be accepted anyway, please report an issue with Miri
+   = note: BACKTRACE:
+   = note: inside `main` at tests/fail/shims/input_arg_mismatch.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

src/tools/miri/tests/fail/shims/return_type_mismatch.rs

@@ -0,0 +1,21 @@
+//@ignore-target: windows # File handling is not implemented yet
+//@compile-flags: -Zmiri-disable-isolation
+use std::ffi::{CString, OsStr, c_char, c_int, c_short};
+use std::os::unix::ffi::OsStrExt;
+
+extern "C" {
+    fn open(path: *const c_char, oflag: c_int, ...) -> c_int;
+    // correct return type is i32
+    fn close(fd: c_int) -> c_short;
+}
+
+fn main() {
+    let c_path = CString::new(OsStr::new("./text").as_bytes()).expect("CString::new failed");
+    let fd = unsafe {
+        open(c_path.as_ptr(), /* value does not matter */ 0)
+    };
+    let _ = unsafe {
+        close(fd);
+        //~^ ERROR: calling a function with return type i32 passing return place of type i16
+    };
+}

src/tools/miri/tests/fail/shims/return_type_mismatch.stderr

@@ -0,0 +1,17 @@
+error: Undefined Behavior: calling a function with return type i32 passing return place of type i16
+  --> tests/fail/shims/return_type_mismatch.rs:LL:CC
+   |
+LL |         close(fd);
+   |         ^^^^^^^^^ calling a function with return type i32 passing return place of type i16
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = help: this means these two types are not *guaranteed* to be ABI-compatible across all targets
+   = help: if you think this code should be accepted anyway, please report an issue with Miri
+   = note: BACKTRACE:
+   = note: inside `main` at tests/fail/shims/return_type_mismatch.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+