From 1b9be6cdff39b4eec0204fdb8f5644fd9324a8d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 7 Sep 2026 14:19:31 +0200 Subject: [PATCH] codegen: Pad bitfields to field offset if applicable. This fixes the bitfield cases discussed in #3453. --- .../expectations/tests/bitfield_align.rs | 162 ++++++++++++++++++ bindgen-tests/tests/headers/bitfield_align.h | 16 ++ bindgen/codegen/mod.rs | 8 +- bindgen/codegen/struct_layout.rs | 50 ++++-- bindgen/ir/comp.rs | 25 ++- 5 files changed, 232 insertions(+), 29 deletions(-) diff --git a/bindgen-tests/tests/expectations/tests/bitfield_align.rs b/bindgen-tests/tests/expectations/tests/bitfield_align.rs index 563599b94e..a49dfe6f02 100644 --- a/bindgen-tests/tests/expectations/tests/bitfield_align.rs +++ b/bindgen-tests/tests/expectations/tests/bitfield_align.rs @@ -2013,3 +2013,165 @@ impl Date3 { __bindgen_bitfield_unit } } +#[repr(C)] +#[derive(Debug, Default, Copy, Clone)] +pub struct Gap { + pub _bindgen_align: [u32; 0], + pub a: ::std::os::raw::c_char, + pub __bindgen_padding_0: [u8; 3usize], + pub _bitfield_1: __BindgenBitfieldUnit<[u8; 4usize]>, + pub c: ::std::os::raw::c_char, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of Gap"][::std::mem::size_of::() - 12usize]; + ["Alignment of Gap"][::std::mem::align_of::() - 4usize]; + ["Offset of field: Gap::a"][::std::mem::offset_of!(Gap, a) - 0usize]; + ["Offset of field: Gap::c"][::std::mem::offset_of!(Gap, c) - 8usize]; +}; +impl Gap { + #[inline] + #[allow(unnecessary_transmutes)] + pub fn b(&self) -> ::std::os::raw::c_int { + unsafe { + ::std::mem::transmute(self._bitfield_1.get_const::<0usize, 30u8>() as u32) + } + } + #[inline] + #[allow(unnecessary_transmutes)] + pub fn set_b(&mut self, val: ::std::os::raw::c_int) { + unsafe { + let val: u32 = ::std::mem::transmute(val); + self._bitfield_1.set_const::<0usize, 30u8>(val as u64) + } + } + #[inline] + #[allow(unnecessary_transmutes)] + pub unsafe fn b_raw(this: *const Self) -> ::std::os::raw::c_int { + unsafe { + ::std::mem::transmute( + <__BindgenBitfieldUnit< + [u8; 4usize], + >>::raw_get_const::< + 0usize, + 30u8, + >(::std::ptr::addr_of!((*this)._bitfield_1)) as u32, + ) + } + } + #[inline] + #[allow(unnecessary_transmutes)] + pub unsafe fn set_b_raw(this: *mut Self, val: ::std::os::raw::c_int) { + unsafe { + let val: u32 = ::std::mem::transmute(val); + <__BindgenBitfieldUnit< + [u8; 4usize], + >>::raw_set_const::< + 0usize, + 30u8, + >(::std::ptr::addr_of_mut!((*this)._bitfield_1), val as u64) + } + } + #[inline] + #[allow(unnecessary_transmutes)] + pub fn new_bitfield_1( + b: ::std::os::raw::c_int, + ) -> __BindgenBitfieldUnit<[u8; 4usize]> { + let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<[u8; 4usize]> = Default::default(); + __bindgen_bitfield_unit + .set_const::< + 0usize, + 30u8, + >({ + let b: u32 = unsafe { ::std::mem::transmute(b) }; + b as u64 + }); + __bindgen_bitfield_unit + } +} +pub type U = ::std::os::raw::c_uint; +#[repr(C, packed(2))] +#[derive(Debug, Default, Copy, Clone)] +pub struct UnderAligned { + pub before: ::std::os::raw::c_char, + pub __bindgen_padding_0: u8, + pub _bitfield_1: __BindgenBitfieldUnit<[u8; 4usize]>, + pub mid: ::std::os::raw::c_char, + pub inner: U, + pub tail: ::std::os::raw::c_char, +} +#[allow(clippy::unnecessary_operation, clippy::identity_op)] +const _: () = { + ["Size of UnderAligned"][::std::mem::size_of::() - 14usize]; + ["Alignment of UnderAligned"][::std::mem::align_of::() - 2usize]; + [ + "Offset of field: UnderAligned::before", + ][::std::mem::offset_of!(UnderAligned, before) - 0usize]; + [ + "Offset of field: UnderAligned::mid", + ][::std::mem::offset_of!(UnderAligned, mid) - 6usize]; + [ + "Offset of field: UnderAligned::inner", + ][::std::mem::offset_of!(UnderAligned, inner) - 8usize]; + [ + "Offset of field: UnderAligned::tail", + ][::std::mem::offset_of!(UnderAligned, tail) - 12usize]; +}; +impl UnderAligned { + #[inline] + #[allow(unnecessary_transmutes)] + pub fn bits(&self) -> U { + unsafe { + ::std::mem::transmute(self._bitfield_1.get_const::<0usize, 31u8>() as u32) + } + } + #[inline] + #[allow(unnecessary_transmutes)] + pub fn set_bits(&mut self, val: U) { + unsafe { + let val: u32 = ::std::mem::transmute(val); + self._bitfield_1.set_const::<0usize, 31u8>(val as u64) + } + } + #[inline] + #[allow(unnecessary_transmutes)] + pub unsafe fn bits_raw(this: *const Self) -> U { + unsafe { + ::std::mem::transmute( + <__BindgenBitfieldUnit< + [u8; 4usize], + >>::raw_get_const::< + 0usize, + 31u8, + >(::std::ptr::addr_of!((*this)._bitfield_1)) as u32, + ) + } + } + #[inline] + #[allow(unnecessary_transmutes)] + pub unsafe fn set_bits_raw(this: *mut Self, val: U) { + unsafe { + let val: u32 = ::std::mem::transmute(val); + <__BindgenBitfieldUnit< + [u8; 4usize], + >>::raw_set_const::< + 0usize, + 31u8, + >(::std::ptr::addr_of_mut!((*this)._bitfield_1), val as u64) + } + } + #[inline] + #[allow(unnecessary_transmutes)] + pub fn new_bitfield_1(bits: U) -> __BindgenBitfieldUnit<[u8; 4usize]> { + let mut __bindgen_bitfield_unit: __BindgenBitfieldUnit<[u8; 4usize]> = Default::default(); + __bindgen_bitfield_unit + .set_const::< + 0usize, + 31u8, + >({ + let bits: u32 = unsafe { ::std::mem::transmute(bits) }; + bits as u64 + }); + __bindgen_bitfield_unit + } +} diff --git a/bindgen-tests/tests/headers/bitfield_align.h b/bindgen-tests/tests/headers/bitfield_align.h index 7b9527a2ec..ce6e3d49b7 100644 --- a/bindgen-tests/tests/headers/bitfield_align.h +++ b/bindgen-tests/tests/headers/bitfield_align.h @@ -47,3 +47,19 @@ struct Date3 { unsigned short nYear : 8; // 0..100 (8 bits) unsigned char byte; }; + +struct Gap { + char a; + int b : 30; + char c; +}; + +typedef unsigned int U __attribute__((aligned(2))); + +struct UnderAligned { + char before; + U bits : 31; + char mid; + U inner; + char tail; +}; diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs index a6662cbeaf..513f26fa1a 100644 --- a/bindgen/codegen/mod.rs +++ b/bindgen/codegen/mod.rs @@ -1995,6 +1995,12 @@ impl FieldCodegen<'_> for BitfieldUnit { let access_spec = access_specifier(unit_visibility); + if let Some(padding_field) = + struct_layout.saw_bitfield_unit(layout, self.offset()) + { + fields.extend(Some(padding_field)); + } + let field = quote! { #access_spec #unit_field_ident : #field_ty , }; @@ -2011,8 +2017,6 @@ impl FieldCodegen<'_> for BitfieldUnit { } })); } - - struct_layout.saw_bitfield_unit(layout); } } diff --git a/bindgen/codegen/struct_layout.rs b/bindgen/codegen/struct_layout.rs index 0d54fbe38a..6ce2b0a60e 100644 --- a/bindgen/codegen/struct_layout.rs +++ b/bindgen/codegen/struct_layout.rs @@ -114,24 +114,20 @@ impl<'a> StructLayoutTracker<'a> { } } - pub(crate) fn saw_bitfield_unit(&mut self, layout: Layout) { - debug!("saw bitfield unit for {}: {layout:?}", self.name); - - self.latest_offset += layout.size; - + /// Returns a padding field if necessary for a given bitfield unit _before_ adding that unit. + pub(crate) fn saw_bitfield_unit( + &mut self, + layout: Layout, + offset: Option, + ) -> Option { debug!( - "Offset: : {} -> {}", - self.latest_offset - layout.size, - self.latest_offset + "saw bitfield unit for {}: {layout:?} at {offset:?}", + self.name ); - - self.latest_field_layout = Some(layout); - self.last_field_was_bitfield = true; - self.max_field_align = cmp::max(self.max_field_align, layout.align); + self.pad_to_offset(layout, offset, /* is_bitfield = */ true) } - /// Returns a padding field if necessary for a given new field _before_ - /// adding that field. + /// Returns a padding field if necessary for a given new field _before_ adding that field. pub(crate) fn saw_field( &mut self, field_name: &str, @@ -147,9 +143,23 @@ impl<'a> StructLayoutTracker<'a> { field_name: &str, field_layout: Layout, field_offset: Option, + ) -> Option { + debug!("saw_field_with_layout({field_name}, offset = {field_offset:?}, layout = {field_layout:?}"); + self.pad_to_offset( + field_layout, + field_offset, + /* is_bitfield = */ false, + ) + } + + fn pad_to_offset( + &mut self, + field_layout: Layout, + field_offset: Option, + is_bitfield: bool, ) -> Option { let will_merge_with_bitfield = - self.will_merge_with_bitfield(field_layout); + !is_bitfield && self.will_merge_with_bitfield(field_layout); let is_union = self.comp.is_union(); let padding_bytes = match field_offset { @@ -177,7 +187,9 @@ impl<'a> StructLayoutTracker<'a> { self.latest_offset += padding_bytes; - let padding_layout = if self.is_packed || is_union { + // Bitfield units are always byte-aligned, so packed(N) can't move them into place like it + // does for regular fields. + let padding_layout = if (self.is_packed && !is_bitfield) || is_union { None } else { let force_padding = self.ctx.options().force_explicit_padding; @@ -193,7 +205,7 @@ impl<'a> StructLayoutTracker<'a> { ); debug!( - "align field {field_name} to {}/{} with {padding_bytes} padding bytes {field_layout:?}", + "align field to {}/{} with {padding_bytes} padding bytes {field_layout:?}", self.latest_offset, field_offset.unwrap_or(0) / 8, ); @@ -215,10 +227,10 @@ impl<'a> StructLayoutTracker<'a> { self.latest_field_layout = Some(field_layout); self.max_field_align = cmp::max(self.max_field_align, field_layout.align); - self.last_field_was_bitfield = false; + self.last_field_was_bitfield = is_bitfield; debug!( - "Offset: {field_name}: {} -> {}", + "Offset: {} -> {}", self.latest_offset - field_layout.size, self.latest_offset ); diff --git a/bindgen/ir/comp.rs b/bindgen/ir/comp.rs index 5aae745131..cf8ecfce77 100644 --- a/bindgen/ir/comp.rs +++ b/bindgen/ir/comp.rs @@ -165,6 +165,8 @@ pub(crate) trait FieldMethods { pub(crate) struct BitfieldUnit { nth: usize, layout: Layout, + /// The offset of this unit within the struct, in bits, if known. + offset: Option, bitfields: Vec, } @@ -181,6 +183,11 @@ impl BitfieldUnit { self.layout } + /// Get the offset of this unit within the struct, in bits, if known. + pub(crate) fn offset(&self) -> Option { + self.offset + } + /// Get the bitfields within this unit. pub(crate) fn bitfields(&self) -> &[Bitfield] { &self.bitfields @@ -560,6 +567,7 @@ where fields: &mut E, bitfield_unit_count: &mut usize, unit_size_in_bits: usize, + offset: Option, bitfields: Vec, ) where E: Extend, @@ -571,13 +579,14 @@ where fields.extend(Some(Field::Bitfields(BitfieldUnit { nth: *bitfield_unit_count, layout, + offset, bitfields, }))); } // The offset we're in inside the struct, if we know it (we might not know it in presence of // templates). - let mut start_offset_in_struct = 0; + let mut known_start_offset = None; let mut max_align = 0; let mut unit_size_in_bits = 0; let mut bitfields_in_unit = vec![]; @@ -591,7 +600,7 @@ where let bitfield_size = bitfield_layout.size; if unit_size_in_bits == 0 { - start_offset_in_struct = bitfield.offset().unwrap_or(0); + known_start_offset = bitfield.offset(); } let mut offset_in_struct = @@ -599,6 +608,7 @@ where // A zero-width field serves as alignment / padding. if !packed && + bitfield.offset().is_none() && offset_in_struct != 0 && (bitfield_width == 0 || (offset_in_struct & (bitfield_align * 8 - 1)) + @@ -621,12 +631,10 @@ where // bitfields over their types size cause weird allocation size behavior from clang. // Therefore, all bitfields needed to be kept around in order to check for this // and make the struct opaque in this case - bitfields_in_unit.push(Bitfield::new( - offset_in_struct - start_offset_in_struct, - bitfield, - )); - unit_size_in_bits = - offset_in_struct - start_offset_in_struct + bitfield_width; + let bitfield_offset = + offset_in_struct - known_start_offset.unwrap_or(0); + bitfields_in_unit.push(Bitfield::new(bitfield_offset, bitfield)); + unit_size_in_bits = bitfield_offset + bitfield_width; } if unit_size_in_bits != 0 { @@ -635,6 +643,7 @@ where fields, bitfield_unit_count, unit_size_in_bits, + known_start_offset, bitfields_in_unit, ); }