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.23k stars 679 forks source link

add option to use DST structs for flex arrays #2772

Closed jsgf closed 3 months ago

jsgf commented 4 months ago

Add --flexarray-dst option to allow use of a dynamically sized struct to model a C flexible array member. For example, given the C struct:

struct msg {
    unsigned int tag;
    unsigned int len;
    char payload[];
};

generate the corresponding Rust type:

#[repr(C)]
#[derive(Debug)]
pub struct msg<FAM: ?Sized = [::std::os::raw::c_char; 0]> {
    pub tag: ::std::os::raw::c_uint,
    pub len: ::std::os::raw::c_uint,
    pub payload: FAM,
}

This single definition is used for both Sized (with a zero-length array as the FAM type) and DST (with a slice).

This also generates the helper methods to convert a raw pointer into a Rust view of the C type, with a given length.

// Sized implementations
impl msg<[::std::os::raw::c_char; 0]> {
    // Construct a DST reference from a Sized object.
    // SAFETY: there must be at least `len` initialized elements in the underlying storage
    pub unsafe fn flex_ref(
        &self,
        len: usize,
    ) -> &msg<[::std::os::raw::c_char]> {
        unsafe { Self::flex_ptr(self, len) }
    }

    // Same, but mutable
    pub unsafe fn flex_mut_ref(
        &mut self,
        len: usize,
    ) -> &mut msg<[::std::os::raw::c_char]> {
        unsafe { Self::flex_ptr_mut(self, len).assume_init() }
    }

    // Raw pointer variants
    pub unsafe fn flex_ptr<'unbounded>(
        ptr: *const Self,
        len: usize,
    ) -> &'unbounded msg<[::std::os::raw::c_char]> {
        unsafe { &*::std::ptr::from_raw_parts(ptr as *const (), len) }
    }

    pub unsafe fn flex_ptr_mut<'unbounded>(
        ptr: *mut Self,
        len: usize,
    ) -> ::std::mem::MaybeUninit<&'unbounded mut msg<[::std::os::raw::c_char]>>
    {
        unsafe {
            let mut uninit = ::std::mem::MaybeUninit::<
                &mut msg<[::std::os::raw::c_char]>,
            >::uninit();
            (uninit.as_mut_ptr() as *mut *mut msg<[::std::os::raw::c_char]>)
                .write(::std::ptr::from_raw_parts_mut(ptr as *mut (), len));
            uninit
        }
    }
}

// DST implementations
impl msg<[::std::os::raw::c_char]> {
    // Memory size & alignment for allocation
    pub fn layout(len: usize) -> ::std::alloc::Layout {
        unsafe {
            let p: *const Self =
                ::std::ptr::from_raw_parts(::std::ptr::null(), len);
            ::std::alloc::Layout::for_value_raw(p)
        }
    }
    // return the Sized variant along with the length
    pub fn fixed(&self) -> (&msg<[::std::os::raw::c_char; 0]>, usize) {
        unsafe {
            let (ptr, len) = (self as *const Self).to_raw_parts();
            (&*(ptr as *const msg<[::std::os::raw::c_char; 0]>), len)
        }
    }
    pub fn fixed_mut(
        &mut self,
    ) -> (&mut msg<[::std::os::raw::c_char; 0]>, usize) {
        unsafe {
            let (ptr, len) = (self as *mut Self).to_raw_parts();
            (&mut *(ptr as *mut msg<[::std::os::raw::c_char; 0]>), len)
        }
    }
}

Upside:

Problems/TODO:

This is a prototype for #2771

jsgf commented 4 months ago

Not sure why, but when I run cargo fmt locally it wants to change things all over the place. Oh, cargo +nightly fmt

jsgf commented 4 months ago

This doesn't handle the case of a C struct pointing to a DST object (ie thin vs fat pointers).

jsgf commented 4 months ago

I've updated the API to:

  1. generate struct MyStruct<FAM: ?Sized = [u8; 0]> - ie add a type parameter for the flexible array member, which can be dynamically sized but by default is a zero-sized array to make the structure sized. This means that all other references to this type are to the sized variant (without parameter qualification) which makes it equivalent to using __IncompleteArrayField.
  2. Added flex_ref, flex_ref_mut, flex_ptr, flex_ptr_mut to convert a sized reference/pointer to an unsized one. These are unsafe, because you must provide the size of the flexible array correctly.
  3. The DST variant, struct MyStruct<[u8]> has methods fixed, fixed_mut to return the sized structure along with the length. It also has layout which returns the size & alignment needed for a memory allocation.

I've been using this API for one of my own projects, and so far it feels pretty comfortable.

I also changed the generation of PhantomData fields for type parameters to put them at the start of the structure. This was necessary as putting them at the end interfered with the FAM field which must be last. The side-effect of this is a lot of test fixtures changed in a trivial way.

jsgf commented 3 months ago

OK updated with comments (and still on top of #2783)

bors-servo commented 3 months ago

:umbrella: The latest upstream changes (presumably 3b5ce9c5861cd2e97e9789f5b686238656abd2d6) made this pull request unmergeable. Please resolve the merge conflicts.

pvdrz commented 3 months ago

yeah we literally have the experimental flag on the CLI and a feature as well. I think other features are gated behind it already.

jsgf commented 3 months ago

Does --experimental do anything? I couldn't see anywhere that referenced it.

jsgf commented 3 months ago

@emilio @pvdrz Ping? Are there any outstanding concerns about this?

jsgf commented 3 months ago

Fixed:

jsgf commented 3 months ago

@emilio @pvdrz Ping?

bors-servo commented 3 months ago

:umbrella: The latest upstream changes (presumably fb39a30a052bbc3c4e7afe78c5c1655622094d9d) made this pull request unmergeable. Please resolve the merge conflicts.

emilio commented 3 months ago

Please rebase and ping, happy to merge. Hopefully it's just a matter of regenerating test expectations. Sorry, this didn't get into the merge queue because it got configured right after I pressed the "Merge when ready" button :/

jsgf commented 3 months ago

Do you want me to squash the commits?

emilio commented 3 months ago

Do you want me to squash the commits?

Nah, all good, thanks!