Skip to content

Bindgen: Issues with packed named structs that contain unnamed union structs with bitfields presents issues #383

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

Open
JoseTeuttli opened this issue Jun 18, 2021 · 1 comment
Labels
• toolchain Related to `rustc`, `bindgen`, `rustdoc`, LLVM, Clippy...

Comments

@JoseTeuttli
Copy link

Currently for any c structure (the parent struct) that contains an unnamed union struct with a bitfield (the child struct) bindgen will generate a struct with BindgenBitFieldUnit, that generated struct will also have the #[repr(align(#))] trait.

If the parent struct is packed then this gives the "error[E0588]: packed type cannot transitively contain a #[repr(align)] type" during build.

This can normally be avoided if one doesn't need to use this struct my making the struct an opaque blob of bytes by signaling that in "bindgen_parameters".

However, some issues are present. Firstly, when the struct is unnamed this becomes more difficult. The only way I have found to signal that an unnamed struct needs to be an opaque blob is by identifying the unnamed struct's generated name (in rust/bindings_generated.rs) and putting that in "bindgen_parameters", which means one needs to have to build fail, so the bindings get generated, then find the name of the unnamed struct manually and add that to "bindgen_parameters"

Second this means that if one needs to use a stuct that contains such a child struct, the only solution is to manually modify bindings_generated.rs to remove the align trait on the generated child struct. This works in all scenarios I have run into since the align trait is the same size as the generated struct.

Example:

struct hv_ring_buffer { 
    u32 write_index; 
    u32 read_index; 
    u32 interrupt_mask; 
    u32 pending_send_sz; 
    u32 reserved1[12]; 
    union { 
        struct { 
            u32 feat_pending_send_sz:1;  
        }; 
        u32 value; 
    } feature_bits; 
    u8 reserved2[PAGE_SIZE - 68]; 
    u8 buffer[]; 
} __packed; 

generates

#[repr(C, packed)] 
pub struct hv_ring_buffer { 
    pub write_index: u32_, 
    pub read_index: u32_, 
    pub interrupt_mask: u32_, 
    pub pending_send_sz: u32_, 
    pub reserved1: [u32_; 12usize], 
    pub feature_bits: hv_ring_buffer__bindgen_ty_1, 
    pub reserved2: [u8_; 4028usize], 
    pub buffer: __IncompleteArrayField<u8_>, 
} 
#[repr(C)] 
#[derive(Copy, Clone)] 
pub union hv_ring_buffer__bindgen_ty_1 { 
    pub __bindgen_anon_1: hv_ring_buffer__bindgen_ty_1__bindgen_ty_1, 
    pub value: u32_, 
    _bindgen_union_align: u32, 
} 
#[repr(C)] 
#[repr(align(4))] 
#[derive(Default, Copy, Clone)] 
pub struct hv_ring_buffer__bindgen_ty_1__bindgen_ty_1 { 
    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 1usize], u8>, 
    pub __bindgen_padding_0: [u8; 3usize], 
} 

This means without manually modifying hv_ring_buffer__bindgen_ty_1__bindgen_ty_1 and removing the align trait, the build will fail. This is tedious due to it requiring running the build once for it to fail after generating the bindings, then manually modifying them, to run the build a second time where it will succeed.

Third, having to add every struct that contains an unnamed child struct with bitfields leads to a lot of structs having to be added to “bindgen_parameters” which in the long term seems unsustainable. I am not sure what the best solution could be, but perhaps splitting bindgen_parameters similar to makefiles, where each directory would have its own bindgen_parameters, that way it doesn’t all get put in one monolithic file.

@JoseTeuttli JoseTeuttli added the • kbuild Related to building the kernel, `make`, `Kbuild`, `Kconfig` options... label Jun 18, 2021
@ojeda
Copy link
Member

ojeda commented Jun 18, 2021

manually modify bindings_generated.rs

Yeah, that is not workable. We will need to work with the bindgen project to improve this (related: rust-lang/rust-bindgen#1896).

In the meantime, a workaround is using a helper (rust/helpers.c).

@ojeda ojeda added prio: normal • toolchain Related to `rustc`, `bindgen`, `rustdoc`, LLVM, Clippy... and removed • kbuild Related to building the kernel, `make`, `Kbuild`, `Kconfig` options... labels Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
• toolchain Related to `rustc`, `bindgen`, `rustdoc`, LLVM, Clippy...
Development

No branches or pull requests

2 participants