From 893843fd455aa515a5d05cc8e70a970a84dab35d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 14 Mar 2021 15:30:37 +0100 Subject: [PATCH 1/2] when Miri calls a function ptr, make sure it has the right ABI --- src/eval.rs | 2 ++ src/helpers.rs | 6 ++++++ src/machine.rs | 1 + src/shims/panic.rs | 5 +++++ src/shims/posix/thread.rs | 2 ++ src/shims/tls.rs | 4 ++++ tests/compile-fail/concurrency/unwind_top_of_stack.rs | 5 ++++- 7 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/eval.rs b/src/eval.rs index 7a29d91d2d8..bd1aebe00de 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -8,6 +8,7 @@ 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 }, diff --git a/src/helpers.rs b/src/helpers.rs index e98488b9bf6..1f1c9922754 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -161,11 +161,17 @@ fn gen_random(&mut self, ptr: Scalar, len: u64) -> InterpResult<'tcx> { fn call_function( &mut self, f: ty::Instance<'tcx>, + caller_abi: Abi, args: &[Immediate], 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)?; diff --git a/src/machine.rs b/src/machine.rs index 1415f7506a4..99cb3bf2b29 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -430,6 +430,7 @@ fn box_alloc( 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 diff --git a/src/shims/panic.rs b/src/shims/panic.rs index abc7aa2ad1b..2c6d31549c3 100644 --- a/src/shims/panic.rs +++ b/src/shims/panic.rs @@ -15,6 +15,7 @@ use rustc_middle::{mir, ty}; use rustc_target::spec::PanicStrategy; +use rustc_target::spec::abi::Abi; use crate::*; use helpers::check_arg_count; @@ -94,6 +95,7 @@ fn handle_try( 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. @@ -145,6 +147,7 @@ fn handle_stack_pop( 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`. @@ -174,6 +177,7 @@ fn start_panic( let panic = ty::Instance::mono(this.tcx.tcx, panic); this.call_function( panic, + Abi::Rust, &[msg.to_ref()], None, StackPopCleanup::Goto { ret: None, unwind }, @@ -202,6 +206,7 @@ fn assert_panic( 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 }, diff --git a/src/shims/posix/thread.rs b/src/shims/posix/thread.rs index 40663326b46..fb1c018fc34 100644 --- a/src/shims/posix/thread.rs +++ b/src/shims/posix/thread.rs @@ -2,6 +2,7 @@ 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 @@ fn pthread_create( this.call_function( instance, + Abi::C { unwind: false }, &[*func_arg], Some(&ret_place.into()), StackPopCleanup::None { cleanup: true }, diff --git a/src/shims/tls.rs b/src/shims/tls.rs index 36bae2af9cd..ef77949efad 100644 --- a/src/shims/tls.rs +++ b/src/shims/tls.rs @@ -9,6 +9,7 @@ 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 @@ fn schedule_windows_tls_dtors(&mut self) -> InterpResult<'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 @@ fn schedule_macos_tls_dtor(&mut self) -> InterpResult<'tcx, bool> { 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 @@ fn schedule_next_pthread_tls_dtor(&mut self) -> InterpResult<'tcx, bool> { 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 }, diff --git a/tests/compile-fail/concurrency/unwind_top_of_stack.rs b/tests/compile-fail/concurrency/unwind_top_of_stack.rs index c13a2ac8bb0..8f3bb17470b 100644 --- a/tests/compile-fail/concurrency/unwind_top_of_stack.rs +++ b/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)] From d1dec9cd233bf28f2bab14126c3d9df93683ae17 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 14 Mar 2021 15:38:22 +0100 Subject: [PATCH 2/2] don't ICE when callee has the wrong number of arguments --- src/helpers.rs | 8 ++++-- .../compile-fail/concurrency/too_few_args.rs | 26 +++++++++++++++++++ .../compile-fail/concurrency/too_many_args.rs | 26 +++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 tests/compile-fail/concurrency/too_few_args.rs create mode 100644 tests/compile-fail/concurrency/too_many_args.rs diff --git a/src/helpers.rs b/src/helpers.rs index 1f1c9922754..7fe0ae0a97c 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -181,11 +181,15 @@ fn call_function( 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(()) } diff --git a/tests/compile-fail/concurrency/too_few_args.rs b/tests/compile-fail/concurrency/too_few_args.rs new file mode 100644 index 00000000000..e2dfa33af89 --- /dev/null +++ b/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); + } +} diff --git a/tests/compile-fail/concurrency/too_many_args.rs b/tests/compile-fail/concurrency/too_many_args.rs new file mode 100644 index 00000000000..0ef12b07073 --- /dev/null +++ b/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); + } +}