Skip to content

Commit e951825

Browse files
committed
ir: codegen: Handle too large bitfield units.
By not generating various code for it. In the future, we could improve on this by splitting contiguous bitfield units, if needed, so that we can implement them without dealing with rust array derive limits. Fixes #1718
1 parent 2929af6 commit e951825

File tree

5 files changed

+261
-22
lines changed

5 files changed

+261
-22
lines changed

src/codegen/mod.rs

+16-8
Original file line numberDiff line numberDiff line change
@@ -1310,23 +1310,26 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
13101310
F: Extend<proc_macro2::TokenStream>,
13111311
M: Extend<proc_macro2::TokenStream>,
13121312
{
1313+
use ir::ty::RUST_DERIVE_IN_ARRAY_LIMIT;
1314+
13131315
result.saw_bitfield_unit();
13141316

1317+
let layout = self.layout();
1318+
let unit_field_ty = helpers::bitfield_unit(ctx, layout);
13151319
let field_ty = {
1316-
let ty = helpers::bitfield_unit(ctx, self.layout());
13171320
if parent.is_union() && !parent.can_be_rust_union(ctx) {
13181321
result.saw_bindgen_union();
13191322
if ctx.options().enable_cxx_namespaces {
13201323
quote! {
1321-
root::__BindgenUnionField<#ty>
1324+
root::__BindgenUnionField<#unit_field_ty>
13221325
}
13231326
} else {
13241327
quote! {
1325-
__BindgenUnionField<#ty>
1328+
__BindgenUnionField<#unit_field_ty>
13261329
}
13271330
}
13281331
} else {
1329-
ty
1332+
unit_field_ty.clone()
13301333
}
13311334
};
13321335

@@ -1338,19 +1341,24 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
13381341
};
13391342
fields.extend(Some(field));
13401343

1341-
let unit_field_ty = helpers::bitfield_unit(ctx, self.layout());
1342-
13431344
let ctor_name = self.ctor_name();
13441345
let mut ctor_params = vec![];
13451346
let mut ctor_impl = quote! {};
1346-
let mut generate_ctor = true;
1347+
1348+
// We cannot generate any constructor if the underlying storage can't
1349+
// implement AsRef<[u8]> / AsMut<[u8]> / etc.
1350+
let mut generate_ctor = layout.size <= RUST_DERIVE_IN_ARRAY_LIMIT;
13471351

13481352
for bf in self.bitfields() {
13491353
// Codegen not allowed for anonymous bitfields
13501354
if bf.name().is_none() {
13511355
continue;
13521356
}
13531357

1358+
if layout.size > RUST_DERIVE_IN_ARRAY_LIMIT {
1359+
continue;
1360+
}
1361+
13541362
let mut bitfield_representable_as_int = true;
13551363

13561364
bf.codegen(
@@ -1395,7 +1403,7 @@ impl<'a> FieldCodegen<'a> for BitfieldUnit {
13951403
}));
13961404
}
13971405

1398-
struct_layout.saw_bitfield_unit(self.layout());
1406+
struct_layout.saw_bitfield_unit(layout);
13991407
}
14001408
}
14011409

src/ir/analysis/derive.rs

+14
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,20 @@ impl<'ctx> CannotDerive<'ctx> {
361361
return CanDerive::No;
362362
}
363363

364+
// Bitfield units are always represented as arrays of u8, but
365+
// they're not traced as arrays, so we need to check here
366+
// instead.
367+
if !self.derive_trait.can_derive_large_array() &&
368+
info.has_too_large_bitfield_unit() &&
369+
!item.is_opaque(self.ctx, &())
370+
{
371+
trace!(
372+
" cannot derive {} for comp with too large bitfield unit",
373+
self.derive_trait
374+
);
375+
return CanDerive::No;
376+
}
377+
364378
let pred = self.derive_trait.consider_edge_comp();
365379
return self.constrain_join(item, pred);
366380
}

src/ir/comp.rs

+50-14
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ use super::context::{BindgenContext, FunctionId, ItemId, TypeId, VarId};
66
use super::dot::DotAttributes;
77
use super::item::{IsOpaque, Item};
88
use super::layout::Layout;
9-
// use super::ty::RUST_DERIVE_IN_ARRAY_LIMIT;
109
use super::template::TemplateParameters;
1110
use super::traversal::{EdgeKind, Trace, Tracer};
11+
use super::ty::RUST_DERIVE_IN_ARRAY_LIMIT;
1212
use clang;
1313
use codegen::struct_layout::{align_to, bytes_from_bits_pow2};
1414
use ir::derive::CanDeriveCopy;
@@ -497,7 +497,7 @@ fn raw_fields_to_fields_and_bitfield_units<I>(
497497
ctx: &BindgenContext,
498498
raw_fields: I,
499499
packed: bool,
500-
) -> Result<Vec<Field>, ()>
500+
) -> Result<(Vec<Field>, bool), ()>
501501
where
502502
I: IntoIterator<Item = RawField>,
503503
{
@@ -543,7 +543,7 @@ where
543543
"The above loop should consume all items in `raw_fields`"
544544
);
545545

546-
Ok(fields)
546+
Ok((fields, bitfield_unit_count != 0))
547547
}
548548

549549
/// Given a set of contiguous raw bitfields, group and allocate them into
@@ -707,7 +707,10 @@ where
707707
#[derive(Debug)]
708708
enum CompFields {
709709
BeforeComputingBitfieldUnits(Vec<RawField>),
710-
AfterComputingBitfieldUnits(Vec<Field>),
710+
AfterComputingBitfieldUnits {
711+
fields: Vec<Field>,
712+
has_bitfield_units: bool,
713+
},
711714
ErrorComputingBitfieldUnits,
712715
}
713716

@@ -744,10 +747,13 @@ impl CompFields {
744747
let result = raw_fields_to_fields_and_bitfield_units(ctx, raws, packed);
745748

746749
match result {
747-
Ok(fields_and_units) => {
750+
Ok((fields, has_bitfield_units)) => {
748751
mem::replace(
749752
self,
750-
CompFields::AfterComputingBitfieldUnits(fields_and_units),
753+
CompFields::AfterComputingBitfieldUnits {
754+
fields,
755+
has_bitfield_units,
756+
},
751757
);
752758
}
753759
Err(()) => {
@@ -758,11 +764,11 @@ impl CompFields {
758764

759765
fn deanonymize_fields(&mut self, ctx: &BindgenContext, methods: &[Method]) {
760766
let fields = match *self {
761-
CompFields::AfterComputingBitfieldUnits(ref mut fields) => fields,
762-
CompFields::ErrorComputingBitfieldUnits => {
763-
// Nothing to do here.
764-
return;
765-
}
767+
CompFields::AfterComputingBitfieldUnits {
768+
ref mut fields, ..
769+
} => fields,
770+
// Nothing to do here.
771+
CompFields::ErrorComputingBitfieldUnits => return,
766772
CompFields::BeforeComputingBitfieldUnits(_) => {
767773
panic!("Not yet computed bitfield units.");
768774
}
@@ -859,7 +865,7 @@ impl Trace for CompFields {
859865
tracer.visit_kind(f.ty().into(), EdgeKind::Field);
860866
}
861867
}
862-
CompFields::AfterComputingBitfieldUnits(ref fields) => {
868+
CompFields::AfterComputingBitfieldUnits { ref fields, .. } => {
863869
for f in fields {
864870
f.trace(context, tracer, &());
865871
}
@@ -1061,7 +1067,7 @@ impl CompInfo {
10611067
/// Construct a new compound type.
10621068
pub fn new(kind: CompKind) -> Self {
10631069
CompInfo {
1064-
kind: kind,
1070+
kind,
10651071
fields: CompFields::default(),
10661072
template_params: vec![],
10671073
methods: vec![],
@@ -1124,13 +1130,43 @@ impl CompInfo {
11241130
pub fn fields(&self) -> &[Field] {
11251131
match self.fields {
11261132
CompFields::ErrorComputingBitfieldUnits => &[],
1127-
CompFields::AfterComputingBitfieldUnits(ref fields) => fields,
1133+
CompFields::AfterComputingBitfieldUnits { ref fields, .. } => {
1134+
fields
1135+
}
11281136
CompFields::BeforeComputingBitfieldUnits(_) => {
11291137
panic!("Should always have computed bitfield units first");
11301138
}
11311139
}
11321140
}
11331141

1142+
fn has_bitfields(&self) -> bool {
1143+
match self.fields {
1144+
CompFields::ErrorComputingBitfieldUnits => false,
1145+
CompFields::AfterComputingBitfieldUnits {
1146+
has_bitfield_units,
1147+
..
1148+
} => has_bitfield_units,
1149+
CompFields::BeforeComputingBitfieldUnits(_) => {
1150+
panic!("Should always have computed bitfield units first");
1151+
}
1152+
}
1153+
}
1154+
1155+
/// Returns whether we have a too large bitfield unit, in which case we may
1156+
/// not be able to derive some of the things we should be able to normally
1157+
/// derive.
1158+
pub fn has_too_large_bitfield_unit(&self) -> bool {
1159+
if !self.has_bitfields() {
1160+
return false;
1161+
}
1162+
self.fields().iter().any(|field| match *field {
1163+
Field::DataMember(..) => false,
1164+
Field::Bitfields(ref unit) => {
1165+
unit.layout.size > RUST_DERIVE_IN_ARRAY_LIMIT
1166+
}
1167+
})
1168+
}
1169+
11341170
/// Does this type have any template parameters that aren't types
11351171
/// (e.g. int)?
11361172
pub fn has_non_type_template_params(&self) -> bool {

tests/expectations/tests/timex.rs

+166
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
/* automatically generated by rust-bindgen */
2+
3+
#![allow(
4+
dead_code,
5+
non_snake_case,
6+
non_camel_case_types,
7+
non_upper_case_globals
8+
)]
9+
10+
#[repr(C)]
11+
#[derive(Copy, Clone, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)]
12+
pub struct __BindgenBitfieldUnit<Storage, Align> {
13+
storage: Storage,
14+
align: [Align; 0],
15+
}
16+
impl<Storage, Align> __BindgenBitfieldUnit<Storage, Align> {
17+
#[inline]
18+
pub const fn new(storage: Storage) -> Self {
19+
Self { storage, align: [] }
20+
}
21+
}
22+
impl<Storage, Align> __BindgenBitfieldUnit<Storage, Align>
23+
where
24+
Storage: AsRef<[u8]> + AsMut<[u8]>,
25+
{
26+
#[inline]
27+
pub fn get_bit(&self, index: usize) -> bool {
28+
debug_assert!(index / 8 < self.storage.as_ref().len());
29+
let byte_index = index / 8;
30+
let byte = self.storage.as_ref()[byte_index];
31+
let bit_index = if cfg!(target_endian = "big") {
32+
7 - (index % 8)
33+
} else {
34+
index % 8
35+
};
36+
let mask = 1 << bit_index;
37+
byte & mask == mask
38+
}
39+
#[inline]
40+
pub fn set_bit(&mut self, index: usize, val: bool) {
41+
debug_assert!(index / 8 < self.storage.as_ref().len());
42+
let byte_index = index / 8;
43+
let byte = &mut self.storage.as_mut()[byte_index];
44+
let bit_index = if cfg!(target_endian = "big") {
45+
7 - (index % 8)
46+
} else {
47+
index % 8
48+
};
49+
let mask = 1 << bit_index;
50+
if val {
51+
*byte |= mask;
52+
} else {
53+
*byte &= !mask;
54+
}
55+
}
56+
#[inline]
57+
pub fn get(&self, bit_offset: usize, bit_width: u8) -> u64 {
58+
debug_assert!(bit_width <= 64);
59+
debug_assert!(bit_offset / 8 < self.storage.as_ref().len());
60+
debug_assert!(
61+
(bit_offset + (bit_width as usize)) / 8 <=
62+
self.storage.as_ref().len()
63+
);
64+
let mut val = 0;
65+
for i in 0..(bit_width as usize) {
66+
if self.get_bit(i + bit_offset) {
67+
let index = if cfg!(target_endian = "big") {
68+
bit_width as usize - 1 - i
69+
} else {
70+
i
71+
};
72+
val |= 1 << index;
73+
}
74+
}
75+
val
76+
}
77+
#[inline]
78+
pub fn set(&mut self, bit_offset: usize, bit_width: u8, val: u64) {
79+
debug_assert!(bit_width <= 64);
80+
debug_assert!(bit_offset / 8 < self.storage.as_ref().len());
81+
debug_assert!(
82+
(bit_offset + (bit_width as usize)) / 8 <=
83+
self.storage.as_ref().len()
84+
);
85+
for i in 0..(bit_width as usize) {
86+
let mask = 1 << i;
87+
let val_bit_is_set = val & mask == mask;
88+
let index = if cfg!(target_endian = "big") {
89+
bit_width as usize - 1 - i
90+
} else {
91+
i
92+
};
93+
self.set_bit(index + bit_offset, val_bit_is_set);
94+
}
95+
}
96+
}
97+
#[repr(C)]
98+
#[derive(Copy, Clone)]
99+
pub struct timex {
100+
pub tai: ::std::os::raw::c_int,
101+
pub _bitfield_1: __BindgenBitfieldUnit<[u8; 44usize], u8>,
102+
}
103+
#[test]
104+
fn bindgen_test_layout_timex() {
105+
assert_eq!(
106+
::std::mem::size_of::<timex>(),
107+
48usize,
108+
concat!("Size of: ", stringify!(timex))
109+
);
110+
assert_eq!(
111+
::std::mem::align_of::<timex>(),
112+
4usize,
113+
concat!("Alignment of ", stringify!(timex))
114+
);
115+
assert_eq!(
116+
unsafe { &(*(::std::ptr::null::<timex>())).tai as *const _ as usize },
117+
0usize,
118+
concat!(
119+
"Offset of field: ",
120+
stringify!(timex),
121+
"::",
122+
stringify!(tai)
123+
)
124+
);
125+
}
126+
impl Default for timex {
127+
fn default() -> Self {
128+
unsafe { ::std::mem::zeroed() }
129+
}
130+
}
131+
#[repr(C)]
132+
#[derive(Copy, Clone)]
133+
pub struct timex_named {
134+
pub tai: ::std::os::raw::c_int,
135+
pub _bitfield_1: __BindgenBitfieldUnit<[u8; 44usize], u32>,
136+
}
137+
#[test]
138+
fn bindgen_test_layout_timex_named() {
139+
assert_eq!(
140+
::std::mem::size_of::<timex_named>(),
141+
48usize,
142+
concat!("Size of: ", stringify!(timex_named))
143+
);
144+
assert_eq!(
145+
::std::mem::align_of::<timex_named>(),
146+
4usize,
147+
concat!("Alignment of ", stringify!(timex_named))
148+
);
149+
assert_eq!(
150+
unsafe {
151+
&(*(::std::ptr::null::<timex_named>())).tai as *const _ as usize
152+
},
153+
0usize,
154+
concat!(
155+
"Offset of field: ",
156+
stringify!(timex_named),
157+
"::",
158+
stringify!(tai)
159+
)
160+
);
161+
}
162+
impl Default for timex_named {
163+
fn default() -> Self {
164+
unsafe { ::std::mem::zeroed() }
165+
}
166+
}

0 commit comments

Comments
 (0)