Skip to content

Commit

Permalink
Work around a libclang bug / limitation.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
emilio committed Mar 6, 2019
1 parent 3a6864c commit 9b6d0e8
Show file tree
Hide file tree
Showing 10 changed files with 225 additions and 33 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

--------------------------------------------------------------------------------

Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
50 changes: 30 additions & 20 deletions src/clang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<usize, LayoutError> {
let val = self.clang_size_of();
pub fn fallible_size(&self, ctx: &BindgenContext) -> Result<usize, LayoutError> {
let val = self.clang_size_of(ctx);
if val < 0 {
Err(LayoutError::from(val as i32))
} else {
Expand All @@ -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<usize, LayoutError> {
let val = self.clang_align_of();
pub fn fallible_align(&self, ctx: &BindgenContext) -> Result<usize, LayoutError> {
let val = self.clang_align_of(ctx);
if val < 0 {
Err(LayoutError::from(val as i32))
} else {
Expand All @@ -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))
}

Expand Down
10 changes: 5 additions & 5 deletions src/ir/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
);
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion src/ir/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/ir/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions src/ir/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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),
);
}

Expand Down Expand Up @@ -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),
);
Expand Down
150 changes: 150 additions & 0 deletions tests/expectations/tests/issue-1443.rs
Original file line number Diff line number Diff line change
@@ -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::<Bar>(),
16usize,
concat!("Size of: ", stringify!(Bar))
);
assert_eq!(
::std::mem::align_of::<Bar>(),
8usize,
concat!("Alignment of ", stringify!(Bar))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Bar>())).f as *const _ as usize },
0usize,
concat!("Offset of field: ", stringify!(Bar), "::", stringify!(f))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Bar>())).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::<Baz>(),
16usize,
concat!("Size of: ", stringify!(Baz))
);
assert_eq!(
::std::mem::align_of::<Baz>(),
8usize,
concat!("Alignment of ", stringify!(Baz))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Baz>())).f as *const _ as usize },
0usize,
concat!("Offset of field: ", stringify!(Baz), "::", stringify!(f))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Baz>())).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::<Tar>(),
16usize,
concat!("Size of: ", stringify!(Tar))
);
assert_eq!(
::std::mem::align_of::<Tar>(),
8usize,
concat!("Alignment of ", stringify!(Tar))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Tar>())).f as *const _ as usize },
0usize,
concat!("Offset of field: ", stringify!(Tar), "::", stringify!(f))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Tar>())).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::<Taz>(),
16usize,
concat!("Size of: ", stringify!(Taz))
);
assert_eq!(
::std::mem::align_of::<Taz>(),
8usize,
concat!("Alignment of ", stringify!(Taz))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Taz>())).f as *const _ as usize },
0usize,
concat!("Offset of field: ", stringify!(Taz), "::", stringify!(f))
);
assert_eq!(
unsafe { &(*(::std::ptr::null::<Taz>())).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() }
}
}
21 changes: 21 additions & 0 deletions tests/headers/issue-1443.hpp
Original file line number Diff line number Diff line change
@@ -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;
};

0 comments on commit 9b6d0e8

Please sign in to comment.