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

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

Open
fitzgen opened this issue Mar 15, 2017 · 7 comments
Open

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

fitzgen opened this issue Mar 15, 2017 · 7 comments

Comments

@fitzgen
Copy link
Member

fitzgen commented Mar 15, 2017

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
Copy link
Contributor

emilio commented Mar 15, 2017

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
Copy link
Contributor

emilio commented Mar 15, 2017

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

@fitzgen
Copy link
Member Author

fitzgen commented Mar 15, 2017

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
Copy link
Member Author

fitzgen commented Mar 16, 2017

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
Copy link
Member Author

fitzgen commented Mar 16, 2017

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 fitzgen changed the title More layout failures when generating SpiderMonkey bindings Emit wrong layout with zero-sized base members Jul 25, 2017
@fitzgen
Copy link
Member Author

fitzgen commented Jul 25, 2017

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

I'd also expect this to fail:

Filed #851 for this version

@fitzgen
Copy link
Member Author

fitzgen commented Oct 24, 2017

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.

@fitzgen fitzgen changed the title Emit wrong layout with zero-sized base members Emit wrong layout with inheriting from zero-sized type parameters Oct 24, 2017
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