Bitfields should be packed into earlier fields if alignment allows so.

[xxks-kkk]

Per the discussion with @emilio on issue #687, I create this new issue for better tracking.

Input C/C++ Header

I have the following structure definition in C

/**
 * Data used by Set Features / Get Features \ref SPDK_NVME_FEAT_ASYNC_EVENT_CONFIGURATION
 */
union spdk_nvme_feat_async_event_configuration {
	uint32_t raw;
	struct {
		union spdk_nvme_critical_warning_state crit_warn;
		uint32_t ns_attr_notice		: 1;
		uint32_t fw_activation_notice	: 1;
		uint32_t telemetry_log_notice	: 1;
		uint32_t reserved		: 21;
	} bits;
};
SPDK_STATIC_ASSERT(sizeof(union spdk_nvme_feat_async_event_configuration) == 4, "Incorrect size");

It is translated as

/// Data used by Set Features / Get Features \ref SPDK_NVME_FEAT_ASYNC_EVENT_CONFIGURATION
#[repr(C)]
#[derive(Copy, Clone)]
pub union spdk_nvme_feat_async_event_configuration { pub raw: u32, pub bits: spdk_nvme_feat_async_event_configuration__bindgen_ty_1, _bindgen_union_align: u32 }

#[repr(C)]
#[derive(Copy, Clone)]
pub struct spdk_nvme_feat_async_event_configuration__bindgen_ty_1 { pub crit_warn: spdk_nvme_critical_warning_state, pub _bitfield_1: __BindgenBitfieldUnit<[u8; 4usize], u32>, pub __bindgen_align: [u32; 0usize] }

However, during cargo test, the following test is failed

    bindgen_test_layout_spdk_nvme_feat_async_event_configuration
    bindgen_test_layout_spdk_nvme_feat_async_event_configuration__bindgen_ty_1

The corresponding test is

#[test]
fn bindgen_test_layout_spdk_nvme_feat_async_event_configuration() {
    assert_eq!(::std::mem::size_of::<spdk_nvme_feat_async_event_configuration>(), 4usize, concat!( "Size of: " , stringify ! ( spdk_nvme_feat_async_event_configuration ) ));
    assert_eq!(::std::mem::align_of::<spdk_nvme_feat_async_event_configuration>(), 4usize, concat!( "Alignment of " , stringify ! ( spdk_nvme_feat_async_event_configuration ) ));
    assert_eq!(unsafe { &(*(::std::ptr::null::<spdk_nvme_feat_async_event_configuration>())).raw as *const _ as usize }, 0usize, concat!( "Offset of field: " , stringify ! ( spdk_nvme_feat_async_event_configuration ) , "::" , stringify ! ( raw ) ));
    assert_eq!(unsafe { &(*(::std::ptr::null::<spdk_nvme_feat_async_event_configuration>())).bits as *const _ as usize }, 0usize, concat!( "Offset of field: " , stringify ! ( spdk_nvme_feat_async_event_configuration ) , "::" , stringify ! ( bits ) ));
}

and the failure happens at

thread 'bindgen_test_layout_spdk_nvme_feat_async_event_configuration' panicked at 'assertion failed: `(left == right)`
  left: `8`,
 right: `4`: Size of: spdk_nvme_feat_async_event_configuration', /home/zeyuanhu/rustfs/rust-spdk/target/debug/build/rust-spdk-3c1fe41f551c8d38/out/bindings.rs:14005:5
thread 'bindgen_test_layout_spdk_nvme_feat_async_event_configuration__bindgen_ty_1' panicked at 'assertion failed: `(left == right)`
  left: `8`,
 right: `4`: Size of: spdk_nvme_feat_async_event_configuration__bindgen_ty_1', /home/zeyuanhu/rustfs/rust-spdk/target/debug/build/rust-spdk-3c1fe41f551c8d38/out/bindings.rs:13938:5

From what I see, the union is 4 bytes on C but becomes 8 bytes in its corresponding Rust translation.

The spdk_nvme_critical_warning_state inside spdk_nvme_feat_async_event_configuration reported above is defined as

union spdk_nvme_critical_warning_state {
	uint8_t		raw;

	struct {
		uint8_t	available_spare		: 1;
		uint8_t	temperature		: 1;
		uint8_t	device_reliability	: 1;
		uint8_t	read_only		: 1;
		uint8_t	volatile_memory_backup	: 1;
		uint8_t	reserved		: 3;
	} bits;
};
SPDK_STATIC_ASSERT(sizeof(union spdk_nvme_critical_warning_state) == 1, "Incorrect size");

Bindgen Invocation

    let bindings = bindgen::Builder::default()
        .clang_arg(include_path)
        // The input header we would like to generate
        // bindings for.
        .header("src/wrapper.h")
        .blacklist_type("IPPORT_.*")   // https://github.com/rust-lang-nursery/rust-bindgen/issues/687
        .blacklist_type("max_align_t") // https://github.com/rust-lang-nursery/rust-bindgen/issues/550
        // Finish the builder and generate the bindings.
        .generate()
        // Unwrap the Result and panic on failure.
        .expect("Unable to generate bindings");

Actual Results

See above.

Expected Results

union structure expected to be 4 bytes large but got 8 bytes instead.

[emilio]

This looks actually pretty hard to fix, looks like the compiler packs the uint32_t bitfields right after the union, which is another member altogether. A bit reduced:

typedef unsigned char uint8_t;
typedef unsigned uint32_t;

struct Foo {
  uint8_t	available_spare		: 1;
  uint8_t	temperature		: 1;
  uint8_t	device_reliability	: 1;
  uint8_t	read_only		: 1;
  uint8_t	volatile_memory_backup	: 1;
  uint8_t	reserved		: 3;
};

struct Bar {
  struct Foo foo;
  uint32_t ns_attr_notice		: 1;
  uint32_t fw_activation_notice	: 1;
  uint32_t telemetry_log_notice	: 1;
  uint32_t reserved		: 21;
};

I'm not sure what code should we actually generate to make this work. Sigh, bitfields.

[emilio]

A bit more reduced, without the extra struct:

typedef unsigned char uint8_t;
typedef unsigned uint32_t;

struct Bar {
  uint8_t foo;
  uint32_t ns_attr_notice: 1;
  uint32_t fw_activation_notice: 1;
  uint32_t telemetry_log_notice: 1;
  uint32_t reserved: 21;
};

[qinghon]

I'm trying to solve this problem,
This is the closest result:

A bit more reduced, without the extra struct:

typedef unsigned char uint8_t;
typedef unsigned uint32_t;

struct Bar {
  uint8_t foo;
  uint32_t ns_attr_notice: 1;
  uint32_t fw_activation_notice: 1;
  uint32_t telemetry_log_notice: 1;
  uint32_t reserved: 21;
};

#[repr(C)]
#[derive(Debug, Copy, Clone)]
pub struct Bar {
    pub foo: u8,
    pub _bitfield_align_1: [u32; 0],
    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 3usize]>,
}

But in reality, the existing code structure is difficult to solve this problem. We have to parse everything in a large function, just like pressing bullets into the magazine one by one
for example:

struct C {
	unsigned char              x;                    /*     0     1 */
	/* Bitfield combined with previous fields */
	unsigned int               b1:1;                 /*     0: 8  4 */
	/* XXX 7 bits hole, try to pack */
	/* Bitfield combined with next fields */
	unsigned char              y;                    /*     2     1 */
	/* Bitfield combined with previous fields */
	unsigned int               b2:1;                 /*     0:24  4 */
	/* size: 4, cachelines: 1, members: 4 */
	/* sum members: 2 */
	/* sum bitfield members: 2 bits, bit holes: 1, sum bit holes: 7 bits */
	/* bit_padding: 7 bits */
	/* last cacheline: 4 bytes */
};

When parsing b1, the raw_fields_to_fields_and_bitfield_units context does not know the existence of the next field (or does not know whether it can be packaged together), which will cause the bitfield packaging to end in advance, resulting in this result:

pub struct C {
    pub x: ::std::os::raw::c_uchar,
    pub _bitfield_align_1: [u8; 0],
    pub _bitfield_1: __BindgenBitfieldUnit<[u8; 3usize]>,
    pub y: ::std::os::raw::c_uchar,
    pub _bitfield_align_2: [u8; 0],
    pub _bitfield_2: __BindgenBitfieldUnit<[u8; 1usize]>,
}

@emilio