From 56021b28a5150d2f7f7b662a261956d9eff8fe9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 1 Mar 2017 01:43:37 +0100 Subject: [PATCH 1/3] Fix calling convention propagation for function pointers. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This sucks, but works. The full solution is a refactoring that needs more thought than the time I'm able to dedicate to bindgen right now, see the comment for details. Signed-off-by: Emilio Cobos Álvarez --- src/clang.rs | 5 ++- src/ir/ty.rs | 28 +++++++++++++++- tests/expectations/tests/call-conv-field.rs | 36 +++++++++++++++++++++ tests/headers/call-conv-field.h | 7 ++++ 4 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 tests/expectations/tests/call-conv-field.rs create mode 100644 tests/headers/call-conv-field.h diff --git a/src/clang.rs b/src/clang.rs index 1a45eefa06..94463723ad 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -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()) } @@ -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 = diff --git a/src/ir/ty.rs b/src/ir/ty.rs index ce42a171af..a664fa79ed 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -877,6 +877,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. @@ -1132,7 +1133,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); diff --git a/tests/expectations/tests/call-conv-field.rs b/tests/expectations/tests/call-conv-field.rs new file mode 100644 index 0000000000..e9a4d873fa --- /dev/null +++ b/tests/expectations/tests/call-conv-field.rs @@ -0,0 +1,36 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +#[repr(C)] +#[derive(Debug, Copy)] +pub struct JNINativeInterface_ { + pub GetVersion: ::std::option::Option ::std::os::raw::c_int>, +} +#[test] +fn bindgen_test_layout_JNINativeInterface_() { + assert_eq!(::std::mem::size_of::() , 4usize , concat + ! ( "Size of: " , stringify ! ( JNINativeInterface_ ) )); + assert_eq! (::std::mem::align_of::() , 4usize , + 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 ) )); +} +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(); +} diff --git a/tests/headers/call-conv-field.h b/tests/headers/call-conv-field.h new file mode 100644 index 0000000000..b6d306ebd3 --- /dev/null +++ b/tests/headers/call-conv-field.h @@ -0,0 +1,7 @@ +// bindgen-flags: -- -target i686-pc-win32 + +struct JNINativeInterface_ { + int (__stdcall *GetVersion)(void *env); +}; + +__stdcall void bar(); From 0ce79c3ed392e5f0bcf459c24a2ba7f0cb6136dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 1 Mar 2017 14:57:19 +0100 Subject: [PATCH 2/3] Follow proper derive rules for function pointers. --- src/ir/function.rs | 24 +++++++++++ src/ir/ty.rs | 8 ++++ tests/expectations/tests/call-conv-field.rs | 12 ++++-- tests/expectations/tests/derive-fn-ptr.rs | 46 +++++++++++++++++++++ tests/headers/call-conv-field.h | 2 + tests/headers/derive-fn-ptr.h | 8 ++++ 6 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 tests/expectations/tests/derive-fn-ptr.rs create mode 100644 tests/headers/derive-fn-ptr.h diff --git a/src/ir/function.rs b/src/ir/function.rs index 5864bbf86b..ad336c4ba3 100644 --- a/src/ir/function.rs +++ b/src/ir/function.rs @@ -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}; @@ -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 { + 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, + } + } +} diff --git a/src/ir/ty.rs b/src/ir/ty.rs index a664fa79ed..329f4f5408 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -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, } } @@ -636,6 +643,7 @@ impl CanDeriveDefault for Type { TypeKind::ObjCSel | TypeKind::ObjCInterface(..) | TypeKind::Enum(..) => false, + TypeKind::Function(..) | TypeKind::Int(..) | TypeKind::Float(..) | diff --git a/tests/expectations/tests/call-conv-field.rs b/tests/expectations/tests/call-conv-field.rs index e9a4d873fa..d6aa9e4e31 100644 --- a/tests/expectations/tests/call-conv-field.rs +++ b/tests/expectations/tests/call-conv-field.rs @@ -5,17 +5,18 @@ #[repr(C)] -#[derive(Debug, Copy)] +#[derive(Copy)] pub struct JNINativeInterface_ { pub GetVersion: ::std::option::Option ::std::os::raw::c_int>, + pub __hack: ::std::os::raw::c_ulonglong, } #[test] fn bindgen_test_layout_JNINativeInterface_() { - assert_eq!(::std::mem::size_of::() , 4usize , concat + assert_eq!(::std::mem::size_of::() , 16usize , concat ! ( "Size of: " , stringify ! ( JNINativeInterface_ ) )); - assert_eq! (::std::mem::align_of::() , 4usize , + assert_eq! (::std::mem::align_of::() , 8usize , concat ! ( "Alignment of " , stringify ! ( JNINativeInterface_ ) )); assert_eq! (unsafe { @@ -23,6 +24,11 @@ fn bindgen_test_layout_JNINativeInterface_() { 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 } diff --git a/tests/expectations/tests/derive-fn-ptr.rs b/tests/expectations/tests/derive-fn-ptr.rs new file mode 100644 index 0000000000..b6a4f3516a --- /dev/null +++ b/tests/expectations/tests/derive-fn-ptr.rs @@ -0,0 +1,46 @@ +/* automatically generated by rust-bindgen */ + + +#![allow(non_snake_case)] + + +pub type my_fun_t = + ::std::option::Option; +#[repr(C)] +#[derive(Copy)] +pub struct Foo { + pub callback: my_fun_t, +} +#[test] +fn bindgen_test_layout_Foo() { + assert_eq!(::std::mem::size_of::() , 8usize , concat ! ( + "Size of: " , stringify ! ( Foo ) )); + assert_eq! (::std::mem::align_of::() , 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() } } +} diff --git a/tests/headers/call-conv-field.h b/tests/headers/call-conv-field.h index b6d306ebd3..5702a09a2e 100644 --- a/tests/headers/call-conv-field.h +++ b/tests/headers/call-conv-field.h @@ -2,6 +2,8 @@ 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(); diff --git a/tests/headers/derive-fn-ptr.h b/tests/headers/derive-fn-ptr.h new file mode 100644 index 0000000000..39ff76d6f8 --- /dev/null +++ b/tests/headers/derive-fn-ptr.h @@ -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); + +struct Foo { + my_fun_t callback; +}; From 4c07a72f27cb729301c72df656c3f7d50d72b74f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 1 Mar 2017 18:16:34 +0100 Subject: [PATCH 3/3] tests: Mark the new test as unstable, since it seems to fail in 3.8. And I don't want nor have the time to debug it right now. --- tests/headers/call-conv-field.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/headers/call-conv-field.h b/tests/headers/call-conv-field.h index 5702a09a2e..310b5c32fb 100644 --- a/tests/headers/call-conv-field.h +++ b/tests/headers/call-conv-field.h @@ -1,4 +1,5 @@ // bindgen-flags: -- -target i686-pc-win32 +// bindgen-unstable struct JNINativeInterface_ { int (__stdcall *GetVersion)(void *env);