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

Padding not generated #2050

Closed
danobi opened this issue May 7, 2021 · 1 comment · Fixed by #2051
Closed

Padding not generated #2050

danobi opened this issue May 7, 2021 · 1 comment · Fixed by #2051

Comments

@danobi
Copy link
Contributor

danobi commented May 7, 2021

Input C/C++ Header

Apologies for the #includes, not sure how to avoid it here.

#include <stddef.h>
#include <stdbool.h>

struct bpf_object_open_opts {
        size_t sz;
        const char *object_name;
        bool relaxed_maps;
        bool relaxed_core_relocs;
        const char *pin_root_path;
        int attach_prog_fd;
        const char *kconfig;
};

Bindgen Invocation

$ bindgen input.h --with-derive-default --no-layout-tests

Actual Results

/* automatically generated by rust-bindgen 0.58.1 */

pub const true_: u32 = 1;
pub const false_: u32 = 0;
pub const __bool_true_false_are_defined: u32 = 1;
pub type size_t = ::std::os::raw::c_ulong;
pub type wchar_t = ::std::os::raw::c_int;
#[repr(C)]
#[repr(align(16))]
#[derive(Debug, Default, Copy, Clone)]
pub struct max_align_t {
    pub __clang_max_align_nonce1: ::std::os::raw::c_longlong,
    pub __bindgen_padding_0: u64,
    pub __clang_max_align_nonce2: u128,
}
#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct bpf_object_open_opts {
    pub sz: size_t,
    pub object_name: *const ::std::os::raw::c_char,
    pub relaxed_maps: bool,
    pub relaxed_core_relocs: bool,
    pub pin_root_path: *const ::std::os::raw::c_char,
    pub attach_prog_fd: ::std::os::raw::c_int,
    pub kconfig: *const ::std::os::raw::c_char,
}
impl Default for bpf_object_open_opts {
    fn default() -> Self {
        unsafe { ::std::mem::zeroed() }
    }
}

Expected Results

If you take the bindgen generated struct and also create a packed version (see https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=748d6dc00a88d3218bdb0bcf5912d667 ) and then print the sizes of regular and packed, there's a size difference. I think that means there's padding.

According to https://doc.rust-lang.org/stable/std/mem/fn.zeroed.html docs, std::mem::zeroed() on a struct with padding doesn't necessarily zero out the padding (which isn't very nice for libraries which do forward/backwards compat based on struct sizes.

Should there be padding generated for this struct? Or should the default() implementation be changed to actually zero padding?

@emilio
Copy link
Contributor

emilio commented May 8, 2021

We try to not generate padding when not needed. I think a simple fix would be to add an option to change default() to not use zeroed(), but instead use uninitialized() then zero the relevant size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants