From 9b6d0e84b432d95b3bfdf5a19b8abadde768489c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 6 Mar 2019 13:01:18 +0100 Subject: [PATCH] Work around a libclang bug / limitation. For references, libclang returns the size and align of the pointee. This actually matches how C++'s sizeof() and alignof() works, for some reason, though in this case we really want to know the pointer's layout. Anyhow, we know the target pointer size, so manually handle this case. Filed https://bugs.llvm.org/show_bug.cgi?id=40975 for this. --- CHANGELOG.md | 11 ++ Cargo.lock | 2 +- Cargo.toml | 2 +- src/clang.rs | 50 +++++---- src/ir/context.rs | 10 +- src/ir/item.rs | 2 +- src/ir/layout.rs | 4 +- src/ir/ty.rs | 6 +- tests/expectations/tests/issue-1443.rs | 150 +++++++++++++++++++++++++ tests/headers/issue-1443.hpp | 21 ++++ 10 files changed, 225 insertions(+), 33 deletions(-) create mode 100644 tests/expectations/tests/issue-1443.rs create mode 100644 tests/headers/issue-1443.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index fbbf29c1e9..fe3dff50d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,17 @@ Released YYYY/MM/DD * TODO (or remove section if none) +-------------------------------------------------------------------------------- + +# 0.48.1 + +Released 2019/03/06 + +## Fixed + +* Bindgen will properly lay out types that use reference members. [#1531][] + +[#1531]: https://github.com/rust-lang-nursery/rust-bindgen/issues/1531 -------------------------------------------------------------------------------- diff --git a/Cargo.lock b/Cargo.lock index 34e54e9b31..39180066c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -49,7 +49,7 @@ dependencies = [ [[package]] name = "bindgen" -version = "0.48.0" +version = "0.48.1" dependencies = [ "bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)", "cexpr 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index 7e0b014b4d..f77d9d40b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ readme = "README.md" repository = "https://github.com/rust-lang/rust-bindgen" documentation = "https://docs.rs/bindgen" homepage = "https://rust-lang.github.io/rust-bindgen/" -version = "0.48.0" +version = "0.48.1" build = "build.rs" include = [ diff --git a/src/clang.rs b/src/clang.rs index 40ba60e8f2..e02d363fb9 100644 --- a/src/clang.rs +++ b/src/clang.rs @@ -7,6 +7,7 @@ use cexpr; use clang_sys::*; use regex; +use ir::context::BindgenContext; use std::{mem, ptr, slice}; use std::ffi::{CStr, CString}; use std::fmt; @@ -933,35 +934,44 @@ impl Type { #[inline] fn is_non_deductible_auto_type(&self) -> bool { - self.kind() == CXType_Auto && self.canonical_type() == *self + debug_assert_eq!(self.kind(), CXType_Auto); + self.canonical_type() == *self } #[inline] - fn clang_size_of(&self) -> c_longlong { - if self.is_non_deductible_auto_type() { - return -6; // Work-around https://bugs.llvm.org/show_bug.cgi?id=40813 + fn clang_size_of(&self, ctx: &BindgenContext) -> c_longlong { + match self.kind() { + // Work-around https://bugs.llvm.org/show_bug.cgi?id=40975 + CXType_RValueReference | + CXType_LValueReference => ctx.target_pointer_size() as c_longlong, + // Work-around https://bugs.llvm.org/show_bug.cgi?id=40813 + CXType_Auto if self.is_non_deductible_auto_type() => return -6, + _ => unsafe { clang_Type_getSizeOf(self.x) }, } - unsafe { clang_Type_getSizeOf(self.x) } } #[inline] - fn clang_align_of(&self) -> c_longlong { - if self.is_non_deductible_auto_type() { - return -6; // Work-around https://bugs.llvm.org/show_bug.cgi?id=40813 + fn clang_align_of(&self, ctx: &BindgenContext) -> c_longlong { + match self.kind() { + // Work-around https://bugs.llvm.org/show_bug.cgi?id=40975 + CXType_RValueReference | + CXType_LValueReference => ctx.target_pointer_size() as c_longlong, + // Work-around https://bugs.llvm.org/show_bug.cgi?id=40813 + CXType_Auto if self.is_non_deductible_auto_type() => return -6, + _ => unsafe { clang_Type_getAlignOf(self.x) }, } - unsafe { clang_Type_getAlignOf(self.x) } } /// What is the size of this type? Paper over invalid types by returning `0` /// for them. - pub fn size(&self) -> usize { - let val = self.clang_size_of(); + pub fn size(&self, ctx: &BindgenContext) -> usize { + let val = self.clang_size_of(ctx); if val < 0 { 0 } else { val as usize } } /// What is the size of this type? - pub fn fallible_size(&self) -> Result { - let val = self.clang_size_of(); + pub fn fallible_size(&self, ctx: &BindgenContext) -> Result { + let val = self.clang_size_of(ctx); if val < 0 { Err(LayoutError::from(val as i32)) } else { @@ -971,14 +981,14 @@ impl Type { /// What is the alignment of this type? Paper over invalid types by /// returning `0`. - pub fn align(&self) -> usize { - let val = self.clang_align_of(); + pub fn align(&self, ctx: &BindgenContext) -> usize { + let val = self.clang_align_of(ctx); if val < 0 { 0 } else { val as usize } } /// What is the alignment of this type? - pub fn fallible_align(&self) -> Result { - let val = self.clang_align_of(); + pub fn fallible_align(&self, ctx: &BindgenContext) -> Result { + let val = self.clang_align_of(ctx); if val < 0 { Err(LayoutError::from(val as i32)) } else { @@ -988,10 +998,10 @@ impl Type { /// Get the layout for this type, or an error describing why it does not /// have a valid layout. - pub fn fallible_layout(&self) -> Result<::ir::layout::Layout, LayoutError> { + pub fn fallible_layout(&self, ctx: &BindgenContext) -> Result<::ir::layout::Layout, LayoutError> { use ir::layout::Layout; - let size = self.fallible_size()?; - let align = self.fallible_align()?; + let size = self.fallible_size(ctx)?; + let align = self.fallible_align(ctx)?; Ok(Layout::new(size, align)) } diff --git a/src/ir/context.rs b/src/ir/context.rs index f9cd53f35a..2626b3e523 100644 --- a/src/ir/context.rs +++ b/src/ir/context.rs @@ -1755,7 +1755,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" sub_name, template_decl_cursor .cur_type() - .fallible_layout() + .fallible_layout(self) .ok(), sub_kind, false, @@ -1821,7 +1821,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" let name = if name.is_empty() { None } else { Some(name) }; let ty = Type::new( name, - ty.fallible_layout().ok(), + ty.fallible_layout(self).ok(), type_kind, ty.is_const(), ); @@ -1977,7 +1977,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" is_const: bool, ) -> TypeId { let spelling = ty.spelling(); - let layout = ty.fallible_layout().ok(); + let layout = ty.fallible_layout(self).ok(); let type_kind = TypeKind::ResolvedTypeRef(wrapped_id); let ty = Type::new(Some(spelling), layout, type_kind, is_const); let item = Item::new( @@ -2018,7 +2018,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" CXType_UShort => TypeKind::Int(IntKind::UShort), CXType_WChar => { TypeKind::Int(IntKind::WChar { - size: ty.fallible_size().expect("Couldn't compute size of wchar_t?"), + size: ty.fallible_size(self).expect("Couldn't compute size of wchar_t?"), }) }, CXType_Char16 => TypeKind::Int(IntKind::U16), @@ -2056,7 +2056,7 @@ If you encounter an error missing from this list, please file an issue or a PR!" let spelling = ty.spelling(); let is_const = ty.is_const(); - let layout = ty.fallible_layout().ok(); + let layout = ty.fallible_layout(self).ok(); let ty = Type::new(Some(spelling), layout, type_kind, is_const); let id = self.next_item_id(); let item = diff --git a/src/ir/item.rs b/src/ir/item.rs index eec64695c3..6df19be8c3 100644 --- a/src/ir/item.rs +++ b/src/ir/item.rs @@ -454,7 +454,7 @@ impl Item { ty: &clang::Type, ctx: &mut BindgenContext, ) -> TypeId { - let ty = Opaque::from_clang_ty(ty); + let ty = Opaque::from_clang_ty(ty, ctx); let kind = ItemKind::Type(ty); let parent = ctx.root_module().into(); ctx.add_item(Item::new(with_id, None, None, parent, kind), None, None); diff --git a/src/ir/layout.rs b/src/ir/layout.rs index c34da0e1de..c70d2884da 100644 --- a/src/ir/layout.rs +++ b/src/ir/layout.rs @@ -100,8 +100,8 @@ pub struct Opaque(pub Layout); impl Opaque { /// Construct a new opaque type from the given clang type. - pub fn from_clang_ty(ty: &clang::Type) -> Type { - let layout = Layout::new(ty.size(), ty.align()); + pub fn from_clang_ty(ty: &clang::Type, ctx: &BindgenContext) -> Type { + let layout = Layout::new(ty.size(ctx), ty.align(ctx)); let ty_kind = TypeKind::Opaque; let is_const = ty.is_const(); Type::new(None, Some(layout), ty_kind, is_const) diff --git a/src/ir/ty.rs b/src/ir/ty.rs index 922146ea8d..7330933007 100644 --- a/src/ir/ty.rs +++ b/src/ir/ty.rs @@ -730,7 +730,7 @@ impl Type { } } - let layout = ty.fallible_layout().ok(); + let layout = ty.fallible_layout(ctx).ok(); let cursor = ty.declaration(); let mut name = cursor.spelling(); @@ -780,7 +780,7 @@ impl Type { opaque type instead." ); return Ok( - ParseResult::New(Opaque::from_clang_ty(&canonical_ty), None), + ParseResult::New(Opaque::from_clang_ty(&canonical_ty, ctx), None), ); } @@ -912,7 +912,7 @@ impl Type { from class template or base \ specifier, using opaque blob" ); - let opaque = Opaque::from_clang_ty(ty); + let opaque = Opaque::from_clang_ty(ty, ctx); return Ok( ParseResult::New(opaque, None), ); diff --git a/tests/expectations/tests/issue-1443.rs b/tests/expectations/tests/issue-1443.rs new file mode 100644 index 0000000000..54045ef32c --- /dev/null +++ b/tests/expectations/tests/issue-1443.rs @@ -0,0 +1,150 @@ +/* automatically generated by rust-bindgen */ + +#![allow( + dead_code, + non_snake_case, + non_camel_case_types, + non_upper_case_globals +)] + +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Foo { + _unused: [u8; 0], +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Bar { + pub f: *const Foo, + pub m: ::std::os::raw::c_uint, +} +#[test] +fn bindgen_test_layout_Bar() { + assert_eq!( + ::std::mem::size_of::(), + 16usize, + concat!("Size of: ", stringify!(Bar)) + ); + assert_eq!( + ::std::mem::align_of::(), + 8usize, + concat!("Alignment of ", stringify!(Bar)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).f as *const _ as usize }, + 0usize, + concat!("Offset of field: ", stringify!(Bar), "::", stringify!(f)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).m as *const _ as usize }, + 8usize, + concat!("Offset of field: ", stringify!(Bar), "::", stringify!(m)) + ); +} +impl Default for Bar { + fn default() -> Self { + unsafe { ::std::mem::zeroed() } + } +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Baz { + pub f: *mut Foo, + pub m: ::std::os::raw::c_uint, +} +#[test] +fn bindgen_test_layout_Baz() { + assert_eq!( + ::std::mem::size_of::(), + 16usize, + concat!("Size of: ", stringify!(Baz)) + ); + assert_eq!( + ::std::mem::align_of::(), + 8usize, + concat!("Alignment of ", stringify!(Baz)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).f as *const _ as usize }, + 0usize, + concat!("Offset of field: ", stringify!(Baz), "::", stringify!(f)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).m as *const _ as usize }, + 8usize, + concat!("Offset of field: ", stringify!(Baz), "::", stringify!(m)) + ); +} +impl Default for Baz { + fn default() -> Self { + unsafe { ::std::mem::zeroed() } + } +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Tar { + pub f: *const Foo, + pub m: ::std::os::raw::c_uint, +} +#[test] +fn bindgen_test_layout_Tar() { + assert_eq!( + ::std::mem::size_of::(), + 16usize, + concat!("Size of: ", stringify!(Tar)) + ); + assert_eq!( + ::std::mem::align_of::(), + 8usize, + concat!("Alignment of ", stringify!(Tar)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).f as *const _ as usize }, + 0usize, + concat!("Offset of field: ", stringify!(Tar), "::", stringify!(f)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).m as *const _ as usize }, + 8usize, + concat!("Offset of field: ", stringify!(Tar), "::", stringify!(m)) + ); +} +impl Default for Tar { + fn default() -> Self { + unsafe { ::std::mem::zeroed() } + } +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct Taz { + pub f: *mut Foo, + pub m: ::std::os::raw::c_uint, +} +#[test] +fn bindgen_test_layout_Taz() { + assert_eq!( + ::std::mem::size_of::(), + 16usize, + concat!("Size of: ", stringify!(Taz)) + ); + assert_eq!( + ::std::mem::align_of::(), + 8usize, + concat!("Alignment of ", stringify!(Taz)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).f as *const _ as usize }, + 0usize, + concat!("Offset of field: ", stringify!(Taz), "::", stringify!(f)) + ); + assert_eq!( + unsafe { &(*(::std::ptr::null::())).m as *const _ as usize }, + 8usize, + concat!("Offset of field: ", stringify!(Taz), "::", stringify!(m)) + ); +} +impl Default for Taz { + fn default() -> Self { + unsafe { ::std::mem::zeroed() } + } +} diff --git a/tests/headers/issue-1443.hpp b/tests/headers/issue-1443.hpp new file mode 100644 index 0000000000..9b637ba708 --- /dev/null +++ b/tests/headers/issue-1443.hpp @@ -0,0 +1,21 @@ +struct Foo; + +struct Bar { + const Foo& f; + unsigned m; +}; + +struct Baz { + Foo& f; + unsigned m; +}; + +struct Tar { + const Foo&& f; + unsigned m; +}; + +struct Taz { + Foo&& f; + unsigned m; +};