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

Bindgen generates unnecessary padding inside nsIDocument on Windows #520

Closed
upsuper opened this issue Feb 16, 2017 · 5 comments
Closed

Bindgen generates unnecessary padding inside nsIDocument on Windows #520

upsuper opened this issue Feb 16, 2017 · 5 comments

Comments

@upsuper
Copy link
Contributor

upsuper commented Feb 16, 2017

On my machine, the generated nsIDocument is like this:

    #[repr(C)]
    #[derive(Debug)]
    pub struct nsIDocument {
        pub _base: root::nsINode,
        pub _base_1: root::mozilla::dom::DispatcherTrait,
        pub mDeprecationWarnedAbout: u64,
        pub mDocWarningWarnedAbout: u32,
        pub mSelectorCache: root::nsIDocument_SelectorCache,
        pub __bindgen_padding_0: [u64; 10usize],
        pub mReferrer: root::nsCString,
        pub mLastModified: ::nsstring::nsStringRepr,
        // ...
    }

I see no reason why there needs to be a 80byte padding.

This causes a mismatch between offset from C++ and from Rust (mReferrer's offset is 248 in C++ but 328 in Rust), which leads to servo/servo#15483 on Windows.

It seems that this doesn't happen on Mac or Linux, so --target=x86_64-pc-win32 is probably a necessary condition.

@emilio
Copy link
Contributor

emilio commented Feb 16, 2017

That is fixed in #512, and in servo/servo#15216. That was why the bindgenup has been blocked for so long. Sorry about that.

@emilio
Copy link
Contributor

emilio commented Feb 16, 2017

This was not windows specific, it was a regression from #468, that isn't yet in the rest of platforms (I believe you updated your bindgen checkout in msvc, right?).

@upsuper
Copy link
Contributor Author

upsuper commented Feb 16, 2017

Oh, the same issue? Great... so probably I'm seeing this because I'm using a more recent version of bindgen for the crash.

@upsuper
Copy link
Contributor Author

upsuper commented Feb 16, 2017

Let me try the latest master with your patch in #512.

@emilio
Copy link
Contributor

emilio commented Feb 16, 2017

This should be fixed on master now, feel free to reopen if the problem persists (though I hope not! :P).

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

No branches or pull requests

2 participants