Skip to content

Commit

Permalink
Unnamed bit-fields should not affect alignment
Browse files Browse the repository at this point in the history
According to the x86[-64] ABI spec: "Unnamed bit-fields’ types do not affect the
alignment of a structure or union". This makes sense: such bit-fields are only
used for padding, and we can't perform an un-aligned read of something we can't
read because we can't even name it.

Fixes #1076
  • Loading branch information
fitzgen committed Oct 31, 2017
1 parent 7bc4f34 commit e17bd8d
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 10 deletions.
22 changes: 15 additions & 7 deletions src/ir/comp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,20 +608,28 @@ fn bitfields_to_allocation_units<E, I>(
}
}

// According to the x86[-64] ABI spec: "Unnamed bit-fields’ types do not
// affect the alignment of a structure or union". This makes sense: such
// bit-fields are only used for padding, and we can't perform an
// un-aligned read of something we can't read because we can't even name
// it.
if bitfield.name().is_some() {
max_align = cmp::max(max_align, bitfield_align);

// NB: The `bitfield_width` here is completely, absolutely
// intentional. Alignment of the allocation unit is based on the
// maximum bitfield width, not (directly) on the bitfields' types'
// alignment.
unit_align = cmp::max(unit_align, bitfield_width);
}

// Always keep all bitfields around. While unnamed bitifields are used
// for padding (and usually not needed hereafter), large unnamed
// 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, bitfield));

max_align = cmp::max(max_align, bitfield_align);

// NB: The `bitfield_width` here is completely, absolutely intentional.
// Alignment of the allocation unit is based on the maximum bitfield
// width, not (directly) on the bitfields' types' alignment.
unit_align = cmp::max(unit_align, bitfield_width);

unit_size_in_bits = offset + bitfield_width;

// Compute what the physical unit's final size would be given what we
Expand Down
7 changes: 4 additions & 3 deletions tests/expectations/tests/issue-1034.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]


#[repr(C, packed)]
#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct S2 {
pub _bitfield_1: u16,
pub _bitfield_1: u8,
pub __bindgen_padding_0: u8,
}
#[test]
fn bindgen_test_layout_S2() {
Expand All @@ -24,7 +25,7 @@ fn bindgen_test_layout_S2() {
}
impl S2 {
#[inline]
pub fn new_bitfield_1() -> u16 {
pub fn new_bitfield_1() -> u8 {
0
}
}
31 changes: 31 additions & 0 deletions tests/expectations/tests/issue-1076-unnamed-bitfield-alignment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/* automatically generated by rust-bindgen */


#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]


#[repr(C)]
#[derive(Debug, Default, Copy, Clone)]
pub struct S1 {
pub _bitfield_1: [u8; 2usize],
pub __bindgen_padding_0: u8,
}
#[test]
fn bindgen_test_layout_S1() {
assert_eq!(
::std::mem::size_of::<S1>(),
3usize,
concat!("Size of: ", stringify!(S1))
);
assert_eq!(
::std::mem::align_of::<S1>(),
1usize,
concat!("Alignment of ", stringify!(S1))
);
}
impl S1 {
#[inline]
pub fn new_bitfield_1() -> u16 {
0
}
}
4 changes: 4 additions & 0 deletions tests/headers/issue-1076-unnamed-bitfield-alignment.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
struct S1 {
signed : 15;
unsigned : 6
};

0 comments on commit e17bd8d

Please sign in to comment.