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

Generate struct of wrong size with bitfields #726

Closed
fitzgen opened this issue May 30, 2017 · 1 comment
Closed

Generate struct of wrong size with bitfields #726

fitzgen opened this issue May 30, 2017 · 1 comment

Comments

@fitzgen
Copy link
Member

fitzgen commented May 30, 2017

Input C/C++ Header

enum MyEnum {
    ONE,
    TWO,
    THREE,
    FOUR
};

class TaggedPtr {
    MyEnum tag : 2;
    long   ptr : 62;
};

static_assert(sizeof(TaggedPtr) == 8);

Bindgen Invocation

$ ./target/debug/bindgen input.hpp -- -std=c++1z

Actual Results

/* automatically generated by rust-bindgen */

#[repr(u32)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub enum MyEnum { ONE = 0, TWO = 1, THREE = 2, FOUR = 3, }
#[repr(C)]
#[derive(Debug, Copy)]
pub struct TaggedPtr {
    pub _bitfield_1: u8,
    pub _bitfield_2: u64,
    pub __bindgen_align: [u64; 0usize],
}
#[test]
fn bindgen_test_layout_TaggedPtr() {
    assert_eq!(::std::mem::size_of::<TaggedPtr>() , 8usize , concat ! (
               "Size of: " , stringify ! ( TaggedPtr ) ));
    assert_eq! (::std::mem::align_of::<TaggedPtr>() , 8usize , concat ! (
                "Alignment of " , stringify ! ( TaggedPtr ) ));
}
impl Clone for TaggedPtr {
    fn clone(&self) -> Self { *self }
}
impl TaggedPtr {
    #[inline]
    pub fn tag(&self) -> MyEnum {
        let mask = 3usize as u8;
        let unit_field_val: u8 =
            unsafe { ::std::mem::transmute(self._bitfield_1) };
        let val = (unit_field_val & mask) >> 0usize;
        unsafe { ::std::mem::transmute(val as u32) }
    }
    #[inline]
    pub fn set_tag(&mut self, val: MyEnum) {
        let mask = 3usize as u8;
        let val = val as u32 as u8;
        let mut unit_field_val: u8 =
            unsafe { ::std::mem::transmute(self._bitfield_1) };
        unit_field_val &= !mask;
        unit_field_val |= (val << 0usize) & mask;
        self._bitfield_1 = unsafe { ::std::mem::transmute(unit_field_val) };
    }
    #[inline]
    pub const fn new_bitfield_1(tag: MyEnum) -> u8 {
        ({ 0 } | ((tag as u32 as u8) << 0usize) & (3usize as u8))
    }
    #[inline]
    pub fn ptr(&self) -> ::std::os::raw::c_long {
        let mask = 4611686018427387903usize as u64;
        let unit_field_val: u64 =
            unsafe { ::std::mem::transmute(self._bitfield_2) };
        let val = (unit_field_val & mask) >> 0usize;
        unsafe { ::std::mem::transmute(val as u64) }
    }
    #[inline]
    pub fn set_ptr(&mut self, val: ::std::os::raw::c_long) {
        let mask = 4611686018427387903usize as u64;
        let val = val as u64 as u64;
        let mut unit_field_val: u64 =
            unsafe { ::std::mem::transmute(self._bitfield_2) };
        unit_field_val &= !mask;
        unit_field_val |= (val << 0usize) & mask;
        self._bitfield_2 = unsafe { ::std::mem::transmute(unit_field_val) };
    }
    #[inline]
    pub const fn new_bitfield_2(ptr: ::std::os::raw::c_long) -> u64 {
        ({ 0 } |
             ((ptr as u64 as u64) << 0usize) &
                 (4611686018427387903usize as u64))
    }
}

Which results in the following generated layout test failure:

$ ~/scratch/bitfield 

running 1 test
test bindgen_test_layout_TaggedPtr ... FAILED

failures:

---- bindgen_test_layout_TaggedPtr stdout ----
	thread 'bindgen_test_layout_TaggedPtr' panicked at 'assertion failed: `(left == right)` (left: `16`, right: `8`): Size of: TaggedPtr', /home/fitzgen/scratch/bitfield.rs:17
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:355
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:365
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:549
   5: std::panicking::begin_panic
             at /checkout/src/libstd/panicking.rs:511
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:495
   7: bitfield::bindgen_test_layout_TaggedPtr
   8: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1440
             at /checkout/src/libcore/ops.rs:2662
             at /checkout/src/libtest/lib.rs:140
   9: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98


failures:
    bindgen_test_layout_TaggedPtr

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured

Expected Results

We generate a struct of size 8, not 9. The generated layout tests pass.

@emilio
Copy link
Contributor

emilio commented Jun 3, 2017

I investigated a bit, taking this one.

@emilio emilio self-assigned this Jun 3, 2017
emilio added a commit to emilio/rust-bindgen that referenced this issue Jun 4, 2017
In particular, the "flush the allocation unit" logic is only valid for
ms_structs (that is, MSVC).

It's slightly annoying to have this different behavior, but it'd work just fine
if we'd turn that on for MSVC.

This patch doesn't do that, yet at least, and adds tests for all the weird
bitfield alignments around.

Fixes rust-lang#726 (and another set of hidden issues by the old code).
emilio added a commit to emilio/rust-bindgen that referenced this issue Jun 5, 2017
In particular, the "flush the allocation unit" logic is only valid for
ms_structs (that is, MSVC).

It's slightly annoying to have this different behavior, but it'd work just fine
if we'd turn that on for MSVC.

This patch doesn't do that, yet at least, and adds tests for all the weird
bitfield alignments around.

Fixes rust-lang#726 (and another set of hidden issues by the old code).
bors-servo pushed a commit that referenced this issue Jun 5, 2017
ir: Fix a bunch of bitfield correctness issues.

In particular, the "flush the allocation unit" logic is only valid for
ms_structs (that is, MSVC).

It's slightly annoying to have this different behavior, but it'd work just fine
if we'd turn that on for MSVC.

This patch doesn't do that, yet at least, and adds tests for all the weird
bitfield alignments around.

Fixes #726 (and another set of hidden issues by the old code).
tmfink pushed a commit to tmfink/rust-bindgen that referenced this issue Jun 10, 2017
In particular, the "flush the allocation unit" logic is only valid for
ms_structs (that is, MSVC).

It's slightly annoying to have this different behavior, but it'd work just fine
if we'd turn that on for MSVC.

This patch doesn't do that, yet at least, and adds tests for all the weird
bitfield alignments around.

Fixes rust-lang#726 (and another set of hidden issues by the old code).
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