Skip to content

Commit 77cbc72

Browse files
emilioemilio
committed
codegen: Pad bitfields to field offset if applicable.
This fixes the bitfield cases discussed in #3453.
1 parent dc61531 commit 77cbc72

5 files changed

Lines changed: 232 additions & 29 deletions

File tree

‎bindgen-tests/tests/expectations/tests/bitfield_align.rs‎

Lines changed: 162 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎bindgen-tests/tests/headers/bitfield_align.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,19 @@ struct Date3 {
4747
unsigned short nYear : 8; // 0..100 (8 bits)
4848
unsigned char byte;
4949
};
50+
51+
struct Gap {
52+
char a;
53+
int b : 30;
54+
char c;
55+
};
56+
57+
typedef unsigned int U __attribute__((aligned(2)));
58+
59+
struct UnderAligned {
60+
char before;
61+
U bits : 31;
62+
char mid;
63+
U inner;
64+
char tail;
65+
};

‎bindgen/codegen/mod.rs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1995,6 +1995,12 @@ impl FieldCodegen<'_> for BitfieldUnit {
19951995

19961996
let access_spec = access_specifier(unit_visibility);
19971997

1998+
if let Some(padding_field) =
1999+
struct_layout.saw_bitfield_unit(layout, self.offset())
2000+
{
2001+
fields.extend(Some(padding_field));
2002+
}
2003+
19982004
let field = quote! {
19992005
#access_spec #unit_field_ident : #field_ty ,
20002006
};
@@ -2011,8 +2017,6 @@ impl FieldCodegen<'_> for BitfieldUnit {
20112017
}
20122018
}));
20132019
}
2014-
2015-
struct_layout.saw_bitfield_unit(layout);
20162020
}
20172021
}
20182022

‎bindgen/codegen/struct_layout.rs‎

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -114,24 +114,20 @@ impl<'a> StructLayoutTracker<'a> {
114114
}
115115
}
116116

117-
pub(crate) fn saw_bitfield_unit(&mut self, layout: Layout) {
118-
debug!("saw bitfield unit for {}: {layout:?}", self.name);
119-
120-
self.latest_offset += layout.size;
121-
117+
/// Returns a padding field if necessary for a given bitfield unit _before_ adding that unit.
118+
pub(crate) fn saw_bitfield_unit(
119+
&mut self,
120+
layout: Layout,
121+
offset: Option<usize>,
122+
) -> Option<proc_macro2::TokenStream> {
122123
debug!(
123-
"Offset: <bitfield>: {} -> {}",
124-
self.latest_offset - layout.size,
125-
self.latest_offset
124+
"saw bitfield unit for {}: {layout:?} at {offset:?}",
125+
self.name
126126
);
127-
128-
self.latest_field_layout = Some(layout);
129-
self.last_field_was_bitfield = true;
130-
self.max_field_align = cmp::max(self.max_field_align, layout.align);
127+
self.pad_to_offset(layout, offset, /* is_bitfield = */ true)
131128
}
132129

133-
/// Returns a padding field if necessary for a given new field _before_
134-
/// adding that field.
130+
/// Returns a padding field if necessary for a given new field _before_ adding that field.
135131
pub(crate) fn saw_field(
136132
&mut self,
137133
field_name: &str,
@@ -147,9 +143,23 @@ impl<'a> StructLayoutTracker<'a> {
147143
field_name: &str,
148144
field_layout: Layout,
149145
field_offset: Option<usize>,
146+
) -> Option<proc_macro2::TokenStream> {
147+
debug!("saw_field_with_layout({field_name}, offset = {field_offset:?}, layout = {field_layout:?}");
148+
self.pad_to_offset(
149+
field_layout,
150+
field_offset,
151+
/* is_bitfield = */ false,
152+
)
153+
}
154+
155+
fn pad_to_offset(
156+
&mut self,
157+
field_layout: Layout,
158+
field_offset: Option<usize>,
159+
is_bitfield: bool,
150160
) -> Option<proc_macro2::TokenStream> {
151161
let will_merge_with_bitfield =
152-
self.will_merge_with_bitfield(field_layout);
162+
!is_bitfield && self.will_merge_with_bitfield(field_layout);
153163

154164
let is_union = self.comp.is_union();
155165
let padding_bytes = match field_offset {
@@ -177,7 +187,9 @@ impl<'a> StructLayoutTracker<'a> {
177187

178188
self.latest_offset += padding_bytes;
179189

180-
let padding_layout = if self.is_packed || is_union {
190+
// Bitfield units are always byte-aligned, so packed(N) can't move them into place like it
191+
// does for regular fields.
192+
let padding_layout = if (self.is_packed && !is_bitfield) || is_union {
181193
None
182194
} else {
183195
let force_padding = self.ctx.options().force_explicit_padding;
@@ -193,7 +205,7 @@ impl<'a> StructLayoutTracker<'a> {
193205
);
194206

195207
debug!(
196-
"align field {field_name} to {}/{} with {padding_bytes} padding bytes {field_layout:?}",
208+
"align field to {}/{} with {padding_bytes} padding bytes {field_layout:?}",
197209
self.latest_offset,
198210
field_offset.unwrap_or(0) / 8,
199211
);
@@ -215,10 +227,10 @@ impl<'a> StructLayoutTracker<'a> {
215227
self.latest_field_layout = Some(field_layout);
216228
self.max_field_align =
217229
cmp::max(self.max_field_align, field_layout.align);
218-
self.last_field_was_bitfield = false;
230+
self.last_field_was_bitfield = is_bitfield;
219231

220232
debug!(
221-
"Offset: {field_name}: {} -> {}",
233+
"Offset: {} -> {}",
222234
self.latest_offset - field_layout.size,
223235
self.latest_offset
224236
);

‎bindgen/ir/comp.rs‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ pub(crate) trait FieldMethods {
165165
pub(crate) struct BitfieldUnit {
166166
nth: usize,
167167
layout: Layout,
168+
/// The offset of this unit within the struct, in bits, if known.
169+
offset: Option<usize>,
168170
bitfields: Vec<Bitfield>,
169171
}
170172

@@ -181,6 +183,11 @@ impl BitfieldUnit {
181183
self.layout
182184
}
183185

186+
/// Get the offset of this unit within the struct, in bits, if known.
187+
pub(crate) fn offset(&self) -> Option<usize> {
188+
self.offset
189+
}
190+
184191
/// Get the bitfields within this unit.
185192
pub(crate) fn bitfields(&self) -> &[Bitfield] {
186193
&self.bitfields
@@ -560,6 +567,7 @@ where
560567
fields: &mut E,
561568
bitfield_unit_count: &mut usize,
562569
unit_size_in_bits: usize,
570+
offset: Option<usize>,
563571
bitfields: Vec<Bitfield>,
564572
) where
565573
E: Extend<Field>,
@@ -571,13 +579,14 @@ where
571579
fields.extend(Some(Field::Bitfields(BitfieldUnit {
572580
nth: *bitfield_unit_count,
573581
layout,
582+
offset,
574583
bitfields,
575584
})));
576585
}
577586

578587
// The offset we're in inside the struct, if we know it (we might not know it in presence of
579588
// templates).
580-
let mut start_offset_in_struct = 0;
589+
let mut known_start_offset = None;
581590
let mut max_align = 0;
582591
let mut unit_size_in_bits = 0;
583592
let mut bitfields_in_unit = vec![];
@@ -591,14 +600,15 @@ where
591600
let bitfield_size = bitfield_layout.size;
592601

593602
if unit_size_in_bits == 0 {
594-
start_offset_in_struct = bitfield.offset().unwrap_or(0);
603+
known_start_offset = bitfield.offset();
595604
}
596605

597606
let mut offset_in_struct =
598607
bitfield.offset().unwrap_or(unit_size_in_bits);
599608

600609
// A zero-width field serves as alignment / padding.
601610
if !packed &&
611+
bitfield.offset().is_none() &&
602612
offset_in_struct != 0 &&
603613
(bitfield_width == 0 ||
604614
(offset_in_struct & (bitfield_align * 8 - 1)) +
@@ -621,12 +631,10 @@ where
621631
// bitfields over their types size cause weird allocation size behavior from clang.
622632
// Therefore, all bitfields needed to be kept around in order to check for this
623633
// and make the struct opaque in this case
624-
bitfields_in_unit.push(Bitfield::new(
625-
offset_in_struct - start_offset_in_struct,
626-
bitfield,
627-
));
628-
unit_size_in_bits =
629-
offset_in_struct - start_offset_in_struct + bitfield_width;
634+
let bitfield_offset =
635+
offset_in_struct - known_start_offset.unwrap_or(0);
636+
bitfields_in_unit.push(Bitfield::new(bitfield_offset, bitfield));
637+
unit_size_in_bits = bitfield_offset + bitfield_width;
630638
}
631639

632640
if unit_size_in_bits != 0 {
@@ -635,6 +643,7 @@ where
635643
fields,
636644
bitfield_unit_count,
637645
unit_size_in_bits,
646+
known_start_offset,
638647
bitfields_in_unit,
639648
);
640649
}

0 commit comments

Comments
 (0)