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

"Calculated wrong layout" for structs with reference member #1443

Closed
scoopr opened this issue Nov 13, 2018 · 3 comments
Closed

"Calculated wrong layout" for structs with reference member #1443

scoopr opened this issue Nov 13, 2018 · 3 comments

Comments

@scoopr
Copy link
Contributor

scoopr commented Nov 13, 2018

Input C/C++ Header

struct Foo {
    int a, b, c;
};

struct Bar {
    Foo &foo;
};

Bindgen Invocation

$ bindgen input.h -- -x c++

Actual Results

[2018-11-13T21:19:44Z ERROR bindgen::codegen::struct_layout] Calculated wrong layout for Bar, too more 4 bytes

The resulting conversion looks ok to me, so I guess that is just noise, which would make it a really minor issue, but I just wasn't sure if there is a hidden alignment bug when handling references, that should be checked by someone know who knows the code better.

@emilio
Copy link
Contributor

emilio commented Nov 13, 2018

Yeah, looks like there's a subtle bug somewhere, if you look at the debug log, we think that foo has a size of 12, which is really the size of the struct, not of the reference.

Looks like clang may return the wrong layout here for references. In fact, if you change the 'give me a size for this type function' like:

diff --git a/src/clang.rs b/src/clang.rs
index f8f7e581..cf4bb7b0 100644
--- a/src/clang.rs
+++ b/src/clang.rs
@@ -862,16 +862,19 @@ impl Type {
     }
 
     /// What is the size of this type?
     pub fn fallible_size(&self) -> Result<usize, LayoutError> {
         let val = unsafe { clang_Type_getSizeOf(self.x) };
         if val < 0 {
             Err(LayoutError::from(val as i32))
         } else {
+            if self.kind() == CXType_LValueReference {
+                println!("{:?} -> {:?}", self, val);
+            }
             Ok(val as usize)
         }
     }

You can then see the following printed:

Type(Foo &, kind: LValueReference, cconv: 100, decl: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None), canon: Cursor( kind: NoDeclFound, loc: builtin definitions, usr: None)) -> 12

Which is clearly bogus. This can produce incorrect results, so it's a bug.

Good thing is that we actually know the layout of a pointer, and know the thing we have is a reference, so this is fixable, but I don't know yet where's the best place to fix it... Probably we can just override the clang value from from_clang_ty, I don't think anything else depends on it.

@emilio
Copy link
Contributor

emilio commented Mar 6, 2019

A more scary case:

struct Foo;

struct Bar {
  const Foo& f;
  unsigned m;
};

Which generates:

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct Foo {
    _unused: [u8; 0],
}
#[repr(C)]
#[repr(align(8))]
#[derive(Debug, Copy, Clone)]
pub struct Bar {
    pub f: *const Foo,
    pub __bindgen_padding_0: [u32; 2usize],
    pub m: ::std::os::raw::c_uint,
}

Which is clearly bogus.

@emilio
Copy link
Contributor

emilio commented Mar 6, 2019

Fixed by #1531.

@emilio emilio closed this as completed Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants