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

Fix calling convention propagation for function pointers. #549

Merged
merged 3 commits into from
Mar 1, 2017
Merged
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
5 changes: 4 additions & 1 deletion src/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -633,9 +633,10 @@ impl Eq for Type {}
impl fmt::Debug for Type {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
write!(fmt,
"Type({}, kind: {}, decl: {:?}, canon: {:?})",
"Type({}, kind: {}, cconv: {}, decl: {:?}, canon: {:?})",
self.spelling(),
type_to_str(self.kind()),
self.call_conv(),
self.declaration(),
self.declaration().canonical())
}
Expand Down Expand Up @@ -1520,6 +1521,8 @@ pub fn ast_dump(c: &Cursor, depth: isize) -> CXChildVisitResult {
return;
}

print_indent(depth, format!(" {}cconv = {}", prefix, ty.call_conv()));

print_indent(depth,
format!(" {}spelling = \"{}\"", prefix, ty.spelling()));
let num_template_args =
Expand Down
24 changes: 24 additions & 0 deletions src/ir/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use super::dot::DotAttributes;
use super::item::Item;
use super::traversal::{EdgeKind, Trace, Tracer};
use super::ty::TypeKind;
use ir::derive::CanDeriveDebug;
use clang;
use clang_sys::CXCallingConv;
use parse::{ClangItemParser, ClangSubItemParser, ParseError, ParseResult};
Expand Down Expand Up @@ -349,3 +350,26 @@ impl Trace for FunctionSig {
}
}
}

// Function pointers follow special rules, see:
//
// https://github.com/servo/rust-bindgen/issues/547,
// https://github.com/rust-lang/rust/issues/38848,
// and https://github.com/rust-lang/rust/issues/40158
//
// Note that copy is always derived, so we don't need to implement it.
impl CanDeriveDebug for FunctionSig {
Copy link
Member

Choose a reason for hiding this comment

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

At least extending these checks is obvious and straight forward.

type Extra = ();

fn can_derive_debug(&self, _ctx: &BindgenContext, _: ()) -> bool {
const RUST_DERIVE_FUNPTR_LIMIT: usize = 12;
if self.argument_types.len() > RUST_DERIVE_FUNPTR_LIMIT {
return false;
}

match self.abi {
Some(abi::Abi::C) | None => true,
_ => false,
}
}
}
36 changes: 35 additions & 1 deletion src/ir/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,13 @@ impl CanDeriveDebug for Type {
TypeKind::Comp(ref info) => {
info.can_derive_debug(ctx, self.layout(ctx))
}
TypeKind::Pointer(inner) => {
let inner = ctx.resolve_type(inner);
if let TypeKind::Function(ref sig) = *inner.canonical_type(ctx).kind() {
return sig.can_derive_debug(ctx, ());
}
return true;
}
_ => true,
}
}
Expand Down Expand Up @@ -636,6 +643,7 @@ impl CanDeriveDefault for Type {
TypeKind::ObjCSel |
TypeKind::ObjCInterface(..) |
TypeKind::Enum(..) => false,

TypeKind::Function(..) |
TypeKind::Int(..) |
TypeKind::Float(..) |
Expand Down Expand Up @@ -877,6 +885,7 @@ impl Type {
let kind = match ty_kind {
CXType_Unexposed if *ty != canonical_ty &&
canonical_ty.kind() != CXType_Invalid &&
ty.ret_type().is_none() &&
// Sometime clang desugars some types more than
// what we need, specially with function
// pointers.
Expand Down Expand Up @@ -1132,7 +1141,32 @@ impl Type {
CXType_ObjCObjectPointer |
CXType_MemberPointer |
CXType_Pointer => {
let inner = Item::from_ty_or_ref(ty.pointee_type().unwrap(),
// Fun fact: the canonical type of a pointer type may sometimes
// contain information we need but isn't present in the concrete
// type (yeah, I'm equally wat'd).
//
// Yet we still have trouble if we unconditionally trust the
// canonical type, like too-much desugaring (sigh).
//
// See tests/headers/call-conv-field.h for an example.
//
// Since for now the only identifier cause of breakage is the
// ABI for function pointers, and different ABI mixed with
// problematic stuff like that one is _extremely_ unlikely and
// can be bypassed via blacklisting, we do the check explicitly
// (as hacky as it is).
//
// Yet we should probably (somehow) get the best of both worlds,
// presumably special-casing function pointers as a whole, yet
// someone is going to need to care about typedef'd function
// pointers, etc, which isn't trivial given function pointers
// are mostly unexposed. I don't have the time for it right now.
let mut pointee = ty.pointee_type().unwrap();
let canonical_pointee = canonical_ty.pointee_type().unwrap();
if pointee.call_conv() != canonical_pointee.call_conv() {
pointee = canonical_pointee;
}
let inner = Item::from_ty_or_ref(pointee,
location,
None,
ctx);
Expand Down
42 changes: 42 additions & 0 deletions tests/expectations/tests/call-conv-field.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/* automatically generated by rust-bindgen */


#![allow(non_snake_case)]


#[repr(C)]
#[derive(Copy)]
pub struct JNINativeInterface_ {
pub GetVersion: ::std::option::Option<unsafe extern "stdcall" fn(env:
*mut ::std::os::raw::c_void)
-> ::std::os::raw::c_int>,
pub __hack: ::std::os::raw::c_ulonglong,
}
#[test]
fn bindgen_test_layout_JNINativeInterface_() {
assert_eq!(::std::mem::size_of::<JNINativeInterface_>() , 16usize , concat
! ( "Size of: " , stringify ! ( JNINativeInterface_ ) ));
assert_eq! (::std::mem::align_of::<JNINativeInterface_>() , 8usize ,
concat ! (
"Alignment of " , stringify ! ( JNINativeInterface_ ) ));
assert_eq! (unsafe {
& ( * ( 0 as * const JNINativeInterface_ ) ) . GetVersion as *
const _ as usize } , 0usize , concat ! (
"Alignment of field: " , stringify ! ( JNINativeInterface_ ) ,
"::" , stringify ! ( GetVersion ) ));
assert_eq! (unsafe {
& ( * ( 0 as * const JNINativeInterface_ ) ) . __hack as *
const _ as usize } , 8usize , concat ! (
"Alignment of field: " , stringify ! ( JNINativeInterface_ ) ,
"::" , stringify ! ( __hack ) ));
}
impl Clone for JNINativeInterface_ {
fn clone(&self) -> Self { *self }
}
impl Default for JNINativeInterface_ {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
extern "stdcall" {
#[link_name = "_bar@0"]
pub fn bar();
}
46 changes: 46 additions & 0 deletions tests/expectations/tests/derive-fn-ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/* automatically generated by rust-bindgen */


#![allow(non_snake_case)]


pub type my_fun_t =
::std::option::Option<unsafe extern "C" fn(arg1: ::std::os::raw::c_int,
arg2: ::std::os::raw::c_int,
arg3: ::std::os::raw::c_int,
arg4: ::std::os::raw::c_int,
arg5: ::std::os::raw::c_int,
arg6: ::std::os::raw::c_int,
arg7: ::std::os::raw::c_int,
arg8: ::std::os::raw::c_int,
arg9: ::std::os::raw::c_int,
arg10: ::std::os::raw::c_int,
arg11: ::std::os::raw::c_int,
arg12: ::std::os::raw::c_int,
arg13: ::std::os::raw::c_int,
arg14: ::std::os::raw::c_int,
arg15: ::std::os::raw::c_int,
arg16: ::std::os::raw::c_int)>;
#[repr(C)]
#[derive(Copy)]
pub struct Foo {
pub callback: my_fun_t,
}
#[test]
fn bindgen_test_layout_Foo() {
assert_eq!(::std::mem::size_of::<Foo>() , 8usize , concat ! (
"Size of: " , stringify ! ( Foo ) ));
assert_eq! (::std::mem::align_of::<Foo>() , 8usize , concat ! (
"Alignment of " , stringify ! ( Foo ) ));
assert_eq! (unsafe {
& ( * ( 0 as * const Foo ) ) . callback as * const _ as usize
} , 0usize , concat ! (
"Alignment of field: " , stringify ! ( Foo ) , "::" ,
stringify ! ( callback ) ));
}
impl Clone for Foo {
fn clone(&self) -> Self { *self }
}
impl Default for Foo {
fn default() -> Self { unsafe { ::std::mem::zeroed() } }
}
10 changes: 10 additions & 0 deletions tests/headers/call-conv-field.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// bindgen-flags: -- -target i686-pc-win32
// bindgen-unstable

struct JNINativeInterface_ {
int (__stdcall *GetVersion)(void *env);
unsigned long long __hack; // A hack so the field alignment is the same than
// for 64-bit, where we run CI.
};

__stdcall void bar();
8 changes: 8 additions & 0 deletions tests/headers/derive-fn-ptr.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
typedef void (*my_fun_t)(int, int, int, int,
int, int, int, int,
int, int, int, int,
int, int, int, int);
Copy link
Member

Choose a reason for hiding this comment

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

Nice :-|


struct Foo {
my_fun_t callback;
};