rust-lang / rust-bindgen

Automatically generates Rust FFI bindings to C (and some C++) libraries.
https://rust-lang.github.io/rust-bindgen/
BSD 3-Clause "New" or "Revised" License
4.43k stars 695 forks source link

Emit wrong layout with inheriting from zero-sized type parameters #586

Open fitzgen opened 7 years ago

fitzgen commented 7 years ago

Input C/C++ Header

namespace JS {
namespace detail {
class a {};
template <class b> class CallArgsBase : b {
  int *c;
  unsigned d;
};
}
}
class B : JS::detail::CallArgsBase<JS::detail::a> {};

Bindgen Invokation

$ bindgen input.hpp --enable-cxx-namespaces -- -std=c++14

Actual Results

Compiles into this:

/* automatically generated by rust-bindgen */

#[allow(non_snake_case, non_camel_case_types, non_upper_case_globals)]
pub mod root {
    #[allow(unused_imports)]
    use self::super::root;
    pub mod JS {
        #[allow(unused_imports)]
        use self::super::super::root;
        pub mod detail {
            #[allow(unused_imports)]
            use self::super::super::super::root;
            #[repr(C)]
            #[derive(Debug, Copy)]
            pub struct a {
                pub _address: u8,
            }
            #[test]
            fn bindgen_test_layout_a() {
                assert_eq!(::std::mem::size_of::<a>() , 1usize , concat ! (
                           "Size of: " , stringify ! ( a ) ));
                assert_eq! (::std::mem::align_of::<a>() , 1usize , concat ! (
                            "Alignment of " , stringify ! ( a ) ));
            }
            impl Clone for a {
                fn clone(&self) -> Self { *self }
            }
            #[repr(C)]
            #[derive(Debug, Copy, Clone)]
            pub struct CallArgsBase<b> {
                pub _base: b,
                pub c: *mut ::std::os::raw::c_int,
                pub d: ::std::os::raw::c_uint,
            }
        }
    }
    #[test]
    fn __bindgen_test_layout_CallArgsBase_instantiation_16() {
        assert_eq!(::std::mem::size_of::<root::JS::detail::CallArgsBase<root::JS::detail::a>>()
                   , 16usize , concat ! (
                   "Size of template specialization: " , stringify ! (
                   root::JS::detail::CallArgsBase<root::JS::detail::a> ) ));
        assert_eq!(::std::mem::align_of::<root::JS::detail::CallArgsBase<root::JS::detail::a>>()
                   , 8usize , concat ! (
                   "Alignment of template specialization: " , stringify ! (
                   root::JS::detail::CallArgsBase<root::JS::detail::a> ) ));
    }
}

And results in this layout test failure:

running 2 tests
test root::JS::detail::bindgen_test_layout_a ... ok
test root::__bindgen_test_layout_CallArgsBase_instantiation_16 ... FAILED

failures:

---- root::__bindgen_test_layout_CallArgsBase_instantiation_16 stdout ----
    thread 'root::__bindgen_test_layout_CallArgsBase_instantiation_16' panicked at 'assertion failed: `(left == right)` (left: `24`, right: `16`): Size of template specialization: root :: JS :: detail :: CallArgsBase < root :: JS :: detail :: a >', ./js.rs:41
note: Run with `RUST_BACKTRACE=1` for a backtrace.

failures:
    root::__bindgen_test_layout_CallArgsBase_instantiation_16

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

Expected Results

Maybe opaque blobs, but never any layout test failures.

emilio commented 7 years ago

I don't know how could we possibly fix this without monomorphization tbh.

I'd also expect this to fail:

class Foo {
    virtual void vtable();
};

template<typename T>
class Bar : public T {};
emilio commented 7 years ago

Mainly, the code we should generate for the template depends on the actual argument.

fitzgen commented 7 years ago

Mainly, the code we should generate for the template depends on the actual argument.

I would hope that libclang would give us a layout for a concrete instantiation, though. Haven't dug into it yet, however.

fitzgen commented 7 years ago

Cleaned up test case:

// bindgen-flags: -- -std=c++14

class Base {};

template <class BaseT>
class CallArgsBase : BaseT {
    int *c;
    unsigned d;
};

class CallArgs : CallArgsBase<Base> {};

I'm not sure what is going on here and why the expected layout is size 16. Does C++ allow zero-sized base classes as long as the derived class is not zero sized? If so, then I think we're hosed.

ir

fitzgen commented 7 years ago

Does C++ allow zero-sized base classes as long as the derived class is not zero sized?

Yes

If so, then I think we're hosed.

and yes

fitzgen commented 7 years ago

I don't know how could we possibly fix this without monomorphization tbh.

I'd also expect this to fail:

Filed https://github.com/servo/rust-bindgen/issues/851 for this version

fitzgen commented 7 years ago

With non-Copy types in unions, or a cosnt-fn version of std::mem::size_of, we could do this:

#![feature(untagged_unions)]

use std::mem;

struct Inherits<T> {
    _base: AtLeastOneByte<T>,
}

union AtLeastOneByte<T> {
    t: T,
    _address: u8,
}

struct SizedFoo {
    x: usize,
    y: usize,
}

struct UnsizedBar;

fn main() {
    println!("size of Inherits<SizedFoo> = {}", mem::size_of::<Inherits<SizedFoo>>());
    println!("size of Inherits<UnsizedBar> = {}", mem::size_of::<Inherits<UnsizedBar>>());
}

Emits:

size of Inherits<SizedFoo> = 16
size of Inherits<UnsizedBar> = 1

Although we would need to actually emit two versions of Inherits: One for when it is used as a base class, and another for when it is not. Only the latter would apply AtLeastOneByte.