Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FnAbi Compatability check #4185

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 76 additions & 1 deletion src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -1015,6 +1015,81 @@ 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]>
where
&'a [OpTy<'tcx>; N]: TryFrom<&'a [OpTy<'tcx>]>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an odd trait bound, afaict the libcore impls are generic and not macro defined for only a limited number of lengths. Is this bound really necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this bound. Btw I just copied it from check_shim.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, probably from the olden days when we only had impls for arrays of length 32

{
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: unwinding behavior differs called function {} unwind, but expected function {}",
if caller_fn_abi.can_unwind { "can" } else { "cannot" },
if callee_fn_abi.can_unwind { "can" } else { "cannot" }
);
}
if !this.check_argument_compat(&caller_fn_abi.ret, &callee_fn_abi.ret)? {
throw_ub_format!("ABI mismatch: return type mismatches expected return type");
}
if !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()
.all(|b| b)
{
throw_ub_format!(
"ABI mismatch: function argument types mismatches expected argument types"
);
}

if let Ok(ops) = caller_args.try_into() {
return interp_ok(ops);
}
throw_ub_format!(
"incorrect number of arguments for `{link_name}`: got {}, expected {}",
caller_args.len(),
N
)
}

/// 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>(
Expand Down
11 changes: 9 additions & 2 deletions src/shims/unix/foreign_items.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)?;
}
Expand Down
21 changes: 21 additions & 0 deletions tests/fail/shims/input_arg_mismatch.rs
Original file line number Diff line number Diff line change
@@ -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: ABI mismatch: function argument types mismatches expected argument types
};
}
15 changes: 15 additions & 0 deletions tests/fail/shims/input_arg_mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: Undefined Behavior: ABI mismatch: function argument types mismatches expected argument types
--> tests/fail/shims/input_arg_mismatch.rs:LL:CC
|
LL | close(fd);
| ^^^^^^^^^ ABI mismatch: function argument types mismatches expected argument types
|
= 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
= 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