Skip to content

Commit

Permalink
Handle return type promotion in the high layer
Browse files Browse the repository at this point in the history 
As described in #66, the
libffi library implicitly extends small integer types used as
return types, both in calls and closure callbacks.  This is not
currently addressed by the existing code base.

For the low and middle layers, the user can manually address this
issue by using appropriate return types.  However, for the high
layer, the types are automatically determined, and are currently
incorrect.  This leads to problems (primarily) on big-endian
platforms.

This PR addresses the return type extension in the high layer
by storing the extended type used by libffi as an associated
type `RetType` in the `CType` trait, and using that type when
performing calls or constructing closure callbacks.

This implementation does not change the libffi-rs API, and should
not add any significant run-time overhead.
  • Loading branch information
@uweigand
uweigand committed Feb 23, 2023
1 parent 7bee656 commit 223f054
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 32 deletions.
9 changes: 8 additions & 1 deletion libffi-rs/src/high/call.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
//! assert!((result - 5f32).abs() < 0.0001);
//! ```
use std::convert::TryInto;
use std::marker::PhantomData;

use crate::middle;
Expand Down Expand Up @@ -78,7 +79,13 @@ pub unsafe fn call<R: super::CType>(fun: CodePtr, args: &[Arg]) -> R {
let cif = middle::Cif::new(types, R::reify().into_middle());

let values = args.iter().map(|arg| arg.value.clone()).collect::<Vec<_>>();
cif.call(fun, &values)
// If `R` is a small integer type, libffi implicitly extends it to
// `ffi_arg` or `ffi_sarg`. To account for this, use `R::RetType`
// as return type for the low-level call, and convert the result back.
cif.call::<R::RetType>(fun, &values)
.try_into()
.ok()
.unwrap()
}

/// Performs a dynamic call to a C function.
Expand Down
48 changes: 27 additions & 21 deletions libffi-rs/src/high/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ macro_rules! define_closure_mod {
}
}

impl<'a, $( $T, )* R> $closure<'a, $( $T, )* R> {
impl<'a, $( $T, )* R: CType> $closure<'a, $( $T, )* R> {
/// Gets the C code pointer that is used to invoke the
/// closure.
pub fn code_ptr(&self) -> & $fnptr <'a, $( $T, )* R> {
Expand All @@ -221,12 +221,14 @@ macro_rules! define_closure_mod {
/// Constructs a typed closure callable from C from a CIF
/// describing the calling convention for the resulting
/// function, a callback for the function to call, and
/// userdata to pass to the callback.
/// userdata to pass to the callback. Note that the return
/// type of the callback must follow the libffi implicit
/// extension rules.
pub fn from_parts<U>(cif: $cif<$( $T, )* R>,
callback: $callback<U, $( $T, )* R>,
callback: $callback<U, $( $T, )* R::RetType>,
userdata: &'a U) -> Self
{
let callback: middle::Callback<U, R>
let callback: middle::Callback<U, R::RetType>
= unsafe { mem::transmute(callback) };
let closure
= middle::Closure::new(cif.untyped,
Expand All @@ -239,7 +241,7 @@ macro_rules! define_closure_mod {
}
}

impl<'a, $( $T: Copy, )* R> $closure<'a, $( $T, )* R> {
impl<'a, $( $T: Copy, )* R: CType> $closure<'a, $( $T, )* R> {
/// Constructs a typed closure callable from C from a CIF
/// describing the calling convention for the resulting
/// function and the Rust closure to call.
Expand All @@ -255,15 +257,15 @@ macro_rules! define_closure_mod {
#[allow(non_snake_case)]
extern "C" fn static_callback<Callback>
(_cif: &low::ffi_cif,
result: &mut R,
result: &mut R::RetType,
&($( &$T, )*):
&($( &$T, )*),
userdata: &Callback)
where Callback: Fn($( $T, )*) -> R + 'a
{
abort_on_panic!("Cannot panic inside FFI callback", {
unsafe {
ptr::write(result, userdata($( $T, )*));
ptr::write(result, userdata($( $T, )*).into());
}
});
}
Expand Down Expand Up @@ -295,7 +297,7 @@ macro_rules! define_closure_mod {
}
}

impl<'a, $( $T, )* R> $closure_mut<'a, $( $T, )* R> {
impl<'a, $( $T, )* R: CType> $closure_mut<'a, $( $T, )* R> {
/// Gets the C code pointer that is used to invoke the
/// closure.
pub fn code_ptr(&self) -> & $fnptr <'a, $( $T, )* R> {
Expand All @@ -307,12 +309,14 @@ macro_rules! define_closure_mod {
/// Constructs a typed closure callable from C from a CIF
/// describing the calling convention for the resulting
/// function, a callback for the function to call, and
/// userdata to pass to the callback.
/// userdata to pass to the callback. Note that the return
/// type of the callback must follow the libffi implicit
/// extension rules.
pub fn from_parts<U>(cif: $cif<$( $T, )* R>,
callback: $callback_mut<U, $( $T, )* R>,
callback: $callback_mut<U, $( $T, )* R::RetType>,
userdata: &'a mut U) -> Self
{
let callback: middle::CallbackMut<U, R>
let callback: middle::CallbackMut<U, R::RetType>
= unsafe { mem::transmute(callback) };
let closure
= middle::Closure::new_mut(cif.untyped,
Expand All @@ -325,7 +329,7 @@ macro_rules! define_closure_mod {
}
}

impl<'a, $( $T: Copy, )* R> $closure_mut<'a, $( $T, )* R> {
impl<'a, $( $T: Copy, )* R: CType> $closure_mut<'a, $( $T, )* R> {
/// Constructs a typed closure callable from C from a CIF
/// describing the calling convention for the resulting
/// function and the Rust closure to call.
Expand All @@ -342,15 +346,15 @@ macro_rules! define_closure_mod {
#[allow(non_snake_case)]
extern "C" fn static_callback<Callback>
(_cif: &low::ffi_cif,
result: &mut R,
result: &mut R::RetType,
&($( &$T, )*):
&($( &$T, )*),
userdata: &mut Callback)
where Callback: FnMut($( $T, )*) -> R + 'a
{
abort_on_panic!("Cannot panic inside FFI callback", {
unsafe {
ptr::write(result, userdata($( $T, )*));
ptr::write(result, userdata($( $T, )*).into());
}
});
}
Expand All @@ -377,7 +381,7 @@ macro_rules! define_closure_mod {
}
}

impl<$( $T: Copy, )* R> $closure_once<$( $T, )* R> {
impl<$( $T: Copy, )* R: CType> $closure_once<$( $T, )* R> {
/// Constructs a one-shot closure callable from C from a CIF
/// describing the calling convention for the resulting
/// function and the Rust closure to call.
Expand All @@ -393,7 +397,7 @@ macro_rules! define_closure_mod {
#[allow(non_snake_case)]
extern "C" fn static_callback<Callback>
(_cif: &low::ffi_cif,
result: &mut R,
result: &mut R::RetType,
&($( &$T, )*):
&($( &$T, )*),
userdata: &mut Option<Callback>)
Expand All @@ -402,7 +406,7 @@ macro_rules! define_closure_mod {
if let Some(userdata) = userdata.take() {
abort_on_panic!("Cannot panic inside FFI callback", {
unsafe {
ptr::write(result, userdata($( $T, )*));
ptr::write(result, userdata($( $T, )*).into());
}
});
} else {
Expand All @@ -414,7 +418,7 @@ macro_rules! define_closure_mod {
}
}

impl<$( $T, )* R> $closure_once<$( $T, )* R> {
impl<$( $T, )* R: CType> $closure_once<$( $T, )* R> {
/// Gets the C code pointer that is used to invoke the
/// closure.
pub fn code_ptr(&self) -> & $fnptr <'_, $( $T, )* R> {
Expand All @@ -426,14 +430,16 @@ macro_rules! define_closure_mod {
/// Constructs a one-shot closure callable from C from a CIF
/// describing the calling convention for the resulting
/// function, a callback for the function to call, and
/// userdata to pass to the callback.
/// userdata to pass to the callback. Note that the return
/// type of the callback must follow the libffi implicit
/// extension rules.
pub fn from_parts<U: Any>(
cif: $cif<$( $T, )* R>,
callback: $callback_once<U, $( $T, )* R>,
callback: $callback_once<U, $( $T, )* R::RetType>,
userdata: U)
-> Self
{
let callback: middle::CallbackOnce<U, R>
let callback: middle::CallbackOnce<U, R::RetType>
= unsafe { mem::transmute(callback) };
let closure
= middle::ClosureOnce::new(cif.untyped,
Expand Down
31 changes: 22 additions & 9 deletions libffi-rs/src/high/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
use std::marker::PhantomData;

use super::super::low;
use super::super::middle;

/// Represents a C type statically associated with a Rust type.
Expand Down Expand Up @@ -43,34 +44,44 @@ pub unsafe trait CType: Copy {
/// We can use the resulting object to assemble a CIF to set up
/// a call that uses type `T`.
fn reify() -> Type<Self>;
/// The low-level libffi library implicitly extends small integer
/// return values to `ffi_arg` or `ffi_sarg`. Track the possibly
/// extended variant of `T` as an associated type here.
type RetType: std::convert::From<Self> + std::convert::TryInto<Self>;
}

macro_rules! impl_ffi_type {
($type_:ty, $cons:ident) => {
($type_:ty, $ret_:ty, $cons:ident) => {
unsafe impl CType for $type_ {
fn reify() -> Type<Self> {
Type::make(middle::Type::$cons())
}
type RetType = $ret_;
}
};
($type_:ident, $ret_:ty) => {
impl_ffi_type!($type_, $ret_, $type_);
};
($type_:ident) => {
impl_ffi_type!($type_, $type_);
impl_ffi_type!($type_, $type_, $type_);
};
}

impl_ffi_type!(u8);
impl_ffi_type!(i8);
impl_ffi_type!(u16);
impl_ffi_type!(i16);
impl_ffi_type!(u32);
impl_ffi_type!(i32);
// We assume that `ffi_arg` and `ffi_sarg` are either 32-bit or 64-bit
// integer types on all supported platforms here.
impl_ffi_type!(u8, low::ffi_arg);
impl_ffi_type!(i8, low::ffi_sarg);
impl_ffi_type!(u16, low::ffi_arg);
impl_ffi_type!(i16, low::ffi_sarg);
impl_ffi_type!(u32, low::ffi_arg);
impl_ffi_type!(i32, low::ffi_sarg);
impl_ffi_type!(u64);
impl_ffi_type!(i64);
impl_ffi_type!(f32);
impl_ffi_type!(f64);
impl_ffi_type!(usize);
impl_ffi_type!(isize);
impl_ffi_type!((), void);
impl_ffi_type!((), (), void);

// Why is the complex stuff even here? It doesn’t work yet because
// libffi doesn’t support it, so it should probably go away and come
Expand Down Expand Up @@ -118,10 +129,12 @@ unsafe impl<T> CType for *const T {
fn reify() -> Type<Self> {
Type::make(middle::Type::pointer())
}
type RetType = *const T;
}

unsafe impl<T> CType for *mut T {
fn reify() -> Type<Self> {
Type::make(middle::Type::pointer())
}
type RetType = *mut T;
}
4 changes: 3 additions & 1 deletion libffi-rs/src/low.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,9 @@ impl CodePtr {
}
}

pub use raw::{ffi_abi, ffi_abi_FFI_DEFAULT_ABI, ffi_cif, ffi_closure, ffi_status, ffi_type};
pub use raw::{
ffi_abi, ffi_abi_FFI_DEFAULT_ABI, ffi_arg, ffi_cif, ffi_closure, ffi_sarg, ffi_status, ffi_type,
};

/// Re-exports the [`ffi_type`] objects used to describe the types of
/// arguments and results.
Expand Down

0 comments on commit 223f054

Please sign in to comment.