Packed struct with DST fails to compile

[isaacmorgan]

When I run bindgen with a packed struct containing a flexible array member I get a compilation error because the FAM uses drop.

I can edit the bindgen generated file to wrap FAM inside ManuallyDrop and it compiles successfully and appears to be working. Though I'm not sure if that approach is the right thing to do. Is there a way for bindgen to handle this itself, is there a better way than using ManuallyDrop?

test.h

#pragma pack(1)

typedef struct Test
{
    short int Head;
    long int Tail[];
}
Test;

build.rs

use std::env;
use std::path::PathBuf;
use bindgen::RustTarget;

fn main() {
    let bindings = bindgen::Builder::default()
        .header("test.h")
        .flexarray_dst(true)
        .rust_target(RustTarget::Nightly)
        .parse_callbacks(Box::new(bindgen::CargoCallbacks::new()))
        .generate()
        .expect("Unable to generate bindings");

    let out_path = PathBuf::from(env::var("OUT_DIR").unwrap());
    bindings
        .write_to_file(out_path.join("bindings.rs"))
        .expect("Unable to write bindings");
}

output error

error[E0277]: the size for values of type `FAM` cannot be known at compilation time
 --> out/bindings.rs:6:15
  |
4 | pub struct Test<FAM: ?Sized = [::std::os::raw::c_long; 0]> {
  |                 ----------------------------------------- this type parameter needs to be `Sized`
5 |     pub Head: ::std::os::raw::c_short,
6 |     pub Tail: FAM,
  |               ^^^ doesn't have a size known at compile-time
  |
  = note: the last field of a packed struct may only have a dynamically sized type if it does not need drop to be run

bindings.rs output with ManuallyDrop added

/* automatically generated by rust-bindgen 0.70.1 */
use std::mem::ManuallyDrop;

#[repr(C, packed)]
pub struct Test<FAM: ?Sized = [::std::os::raw::c_long; 0]> {
    pub Head: ::std::os::raw::c_short,
    pub Tail: ManuallyDrop<FAM>,
}
#[allow(clippy::unnecessary_operation, clippy::identity_op)]
const _: () = {
    ["Size of Test"][::std::mem::size_of::<Test>() - 2usize];
    ["Alignment of Test"][::std::mem::align_of::<Test>() - 1usize];
    ["Offset of field: Test::Head"][::std::mem::offset_of!(Test, Head) - 0usize];
    ["Offset of field: Test::Tail"][::std::mem::offset_of!(Test, Tail) - 2usize];
};
impl Test<[::std::os::raw::c_long]> {
    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)
        }
    }
    #[inline]
    pub fn fixed(&self) -> (&Test<[::std::os::raw::c_long; 0]>, usize) {
        unsafe {
            let (ptr, len) = (self as *const Self).to_raw_parts();
            (&*(ptr as *const Test<[::std::os::raw::c_long; 0]>), len)
        }
    }
    #[inline]
    pub fn fixed_mut(&mut self) -> (&mut Test<[::std::os::raw::c_long; 0]>, usize) {
        unsafe {
            let (ptr, len) = (self as *mut Self).to_raw_parts();
            (&mut *(ptr as *mut Test<[::std::os::raw::c_long; 0]>), len)
        }
    }
}
impl Test<[::std::os::raw::c_long; 0]> {
    #[doc = r" Convert a sized prefix to an unsized structure with the given length."]
    #[doc = r""]
    #[doc = r" SAFETY: Underlying storage is initialized up to at least `len` elements."]
    pub unsafe fn flex_ref(&self, len: usize) -> &Test<[::std::os::raw::c_long]> {
        Self::flex_ptr(self, len)
    }
    #[doc = r" Convert a mutable sized prefix to an unsized structure with the given length."]
    #[doc = r""]
    #[doc = r" SAFETY: Underlying storage is initialized up to at least `len` elements."]
    #[inline]
    pub unsafe fn flex_ref_mut(&mut self, len: usize) -> &mut Test<[::std::os::raw::c_long]> {
        Self::flex_ptr_mut(self, len).assume_init()
    }
    #[doc = r" Construct DST variant from a pointer and a size."]
    #[doc = r""]
    #[doc = r" NOTE: lifetime of returned reference is not tied to any underlying storage."]
    #[doc = r" SAFETY: `ptr` is valid. Underlying storage is fully initialized up to at least `len` elements."]
    #[inline]
    pub unsafe fn flex_ptr<'unbounded>(
        ptr: *const Self,
        len: usize,
    ) -> &'unbounded Test<[::std::os::raw::c_long]> {
        &*::std::ptr::from_raw_parts(ptr as *const (), len)
    }
    #[doc = r" Construct mutable DST variant from a pointer and a"]
    #[doc = r" size. The returned `&mut` reference is initialized"]
    #[doc = r" pointing to memory referenced by `ptr`, but there's"]
    #[doc = r" no requirement that that memory be initialized."]
    #[doc = r""]
    #[doc = r" NOTE: lifetime of returned reference is not tied to any underlying storage."]
    #[doc = r" SAFETY: `ptr` is valid. Underlying storage has space for at least `len` elements."]
    #[inline]
    pub unsafe fn flex_ptr_mut<'unbounded>(
        ptr: *mut Self,
        len: usize,
    ) -> ::std::mem::MaybeUninit<&'unbounded mut Test<[::std::os::raw::c_long]>> {
        let mut uninit = ::std::mem::MaybeUninit::<&mut Test<[::std::os::raw::c_long]>>::uninit();
        (uninit.as_mut_ptr() as *mut *mut Test<[::std::os::raw::c_long]>)
            .write(::std::ptr::from_raw_parts_mut(ptr as *mut (), len));
        uninit
    }
}

[isaacmorgan]

I think using ManuallyDrop is not a good workaround for (I assume) the same reason for the error in the first place: there's no (?) way to drop an unaligned dynamically sized type. In order to drop the value must be copied to an aligned location, but this can't be done because it's dynamically sized. Something like that.

[emilio]

cc @jsgf

[jsgf]

Yeah I think DSTs and packed are intrinsically incompatible, even though the kind of structure you're using is certainly common in C headers.

I do think ManuallyDrop is the best compromise. A packed DST field with a non-trivial drop will just need very bespoke handling. OTOH if it has a trivial drop - as in this case - then ignoring the ManuallyDrop will still be the right result.