Skip to content

Commit c88ddab

Browse files
author
bors-servo
authored
Auto merge of #1075 - pepyakin:fix-1034, r=fitzgen
Use `repr(packed)` If struct requires explicit alignment of 1. Fixes #1034
2 parents 8315646 + 746c743 commit c88ddab

File tree

8 files changed

+149
-112
lines changed

8 files changed

+149
-112
lines changed

‎bindgen-integration/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,7 @@ fn test_bitfields_sixth() {
183183
fn test_bitfield_constructors() {
184184
use std::mem;
185185
let mut first = bindings::bitfields::First {
186-
_bitfield_1: unsafe { mem::transmute(bindings::bitfields::First::new_bitfield_1(1, 2, 3)) },
187-
__bindgen_align: [],
186+
_bitfield_1: unsafe { mem::transmute(bindings::bitfields::First::new_bitfield_1(1, 2, 3)) }
188187
};
189188
assert!(unsafe {
190189
first.assert(1, 2, 3)

‎src/codegen/mod.rs

Lines changed: 104 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,6 +1380,8 @@ impl CodeGenerator for CompInfo {
13801380

13811381
let used_template_params = item.used_template_params(ctx);
13821382

1383+
let mut packed = self.packed();
1384+
13831385
// generate tuple struct if struct or union is a forward declaration,
13841386
// skip for now if template parameters are needed.
13851387
//
@@ -1397,97 +1399,8 @@ impl CodeGenerator for CompInfo {
13971399
return;
13981400
}
13991401

1400-
let mut attributes = vec![];
1401-
let mut needs_clone_impl = false;
1402-
let mut needs_default_impl = false;
1403-
let mut needs_debug_impl = false;
1404-
let mut needs_partialeq_impl = false;
1405-
if let Some(comment) = item.comment(ctx) {
1406-
attributes.push(attributes::doc(comment));
1407-
}
1408-
if self.packed() {
1409-
attributes.push(attributes::repr_list(&["C", "packed"]));
1410-
} else {
1411-
attributes.push(attributes::repr("C"));
1412-
}
1413-
1414-
let is_union = self.kind() == CompKind::Union;
1415-
let mut derives = vec![];
1416-
if item.can_derive_debug(ctx) {
1417-
derives.push("Debug");
1418-
} else {
1419-
needs_debug_impl = ctx.options().derive_debug &&
1420-
ctx.options().impl_debug
1421-
}
1422-
1423-
if item.can_derive_default(ctx) {
1424-
derives.push("Default");
1425-
} else {
1426-
needs_default_impl = ctx.options().derive_default;
1427-
}
1428-
1429-
if item.can_derive_copy(ctx) && !item.annotations().disallow_copy() &&
1430-
ctx.options().derive_copy
1431-
{
1432-
derives.push("Copy");
1433-
if used_template_params.is_some() {
1434-
// FIXME: This requires extra logic if you have a big array in a
1435-
// templated struct. The reason for this is that the magic:
1436-
// fn clone(&self) -> Self { *self }
1437-
// doesn't work for templates.
1438-
//
1439-
// It's not hard to fix though.
1440-
derives.push("Clone");
1441-
} else {
1442-
needs_clone_impl = true;
1443-
}
1444-
}
1445-
1446-
if item.can_derive_hash(ctx) {
1447-
derives.push("Hash");
1448-
}
1449-
1450-
if item.can_derive_partialord(ctx) {
1451-
derives.push("PartialOrd");
1452-
}
1453-
1454-
if item.can_derive_ord(ctx) {
1455-
derives.push("Ord");
1456-
}
1457-
1458-
if item.can_derive_partialeq(ctx) {
1459-
derives.push("PartialEq");
1460-
} else {
1461-
needs_partialeq_impl =
1462-
ctx.options().derive_partialeq &&
1463-
ctx.options().impl_partialeq &&
1464-
ctx.lookup_can_derive_partialeq_or_partialord(item.id())
1465-
.map_or(true, |x| {
1466-
x == CannotDeriveReason::ArrayTooLarge
1467-
});
1468-
}
1469-
1470-
if item.can_derive_eq(ctx) {
1471-
derives.push("Eq");
1472-
}
1473-
1474-
if !derives.is_empty() {
1475-
attributes.push(attributes::derives(&derives))
1476-
}
1477-
14781402
let canonical_name = item.canonical_name(ctx);
14791403
let canonical_ident = ctx.rust_ident(&canonical_name);
1480-
let mut tokens = if is_union && self.can_be_rust_union(ctx) {
1481-
quote! {
1482-
#( #attributes )*
1483-
pub union #canonical_ident
1484-
}
1485-
} else {
1486-
quote! {
1487-
#( #attributes )*
1488-
pub struct #canonical_ident
1489-
}
1490-
};
14911404

14921405
// Generate the vtable from the method list if appropriate.
14931406
//
@@ -1540,6 +1453,8 @@ impl CodeGenerator for CompInfo {
15401453
});
15411454
}
15421455
}
1456+
1457+
let is_union = self.kind() == CompKind::Union;
15431458
if is_union {
15441459
result.saw_union();
15451460
if !self.can_be_rust_union(ctx) {
@@ -1612,10 +1527,17 @@ impl CodeGenerator for CompInfo {
16121527
fields.push(padding_field);
16131528
}
16141529

1615-
if let Some(align_field) =
1616-
layout.and_then(|layout| struct_layout.align_struct(layout))
1617-
{
1618-
fields.push(align_field);
1530+
if let Some(layout) = layout {
1531+
if struct_layout.requires_explicit_align(layout) {
1532+
if layout.align == 1 {
1533+
packed = true;
1534+
} else {
1535+
let ty = helpers::blob(Layout::new(0, layout.align));
1536+
fields.push(quote! {
1537+
pub __bindgen_align: #ty ,
1538+
});
1539+
}
1540+
}
16191541
}
16201542
}
16211543

@@ -1674,6 +1596,95 @@ impl CodeGenerator for CompInfo {
16741596
quote! { }
16751597
};
16761598

1599+
let mut attributes = vec![];
1600+
let mut needs_clone_impl = false;
1601+
let mut needs_default_impl = false;
1602+
let mut needs_debug_impl = false;
1603+
let mut needs_partialeq_impl = false;
1604+
if let Some(comment) = item.comment(ctx) {
1605+
attributes.push(attributes::doc(comment));
1606+
}
1607+
if packed {
1608+
attributes.push(attributes::repr_list(&["C", "packed"]));
1609+
} else {
1610+
attributes.push(attributes::repr("C"));
1611+
}
1612+
1613+
let mut derives = vec![];
1614+
if item.can_derive_debug(ctx) {
1615+
derives.push("Debug");
1616+
} else {
1617+
needs_debug_impl = ctx.options().derive_debug &&
1618+
ctx.options().impl_debug
1619+
}
1620+
1621+
if item.can_derive_default(ctx) {
1622+
derives.push("Default");
1623+
} else {
1624+
needs_default_impl = ctx.options().derive_default;
1625+
}
1626+
1627+
if item.can_derive_copy(ctx) && !item.annotations().disallow_copy() &&
1628+
ctx.options().derive_copy
1629+
{
1630+
derives.push("Copy");
1631+
if used_template_params.is_some() {
1632+
// FIXME: This requires extra logic if you have a big array in a
1633+
// templated struct. The reason for this is that the magic:
1634+
// fn clone(&self) -> Self { *self }
1635+
// doesn't work for templates.
1636+
//
1637+
// It's not hard to fix though.
1638+
derives.push("Clone");
1639+
} else {
1640+
needs_clone_impl = true;
1641+
}
1642+
}
1643+
1644+
if item.can_derive_hash(ctx) {
1645+
derives.push("Hash");
1646+
}
1647+
1648+
if item.can_derive_partialord(ctx) {
1649+
derives.push("PartialOrd");
1650+
}
1651+
1652+
if item.can_derive_ord(ctx) {
1653+
derives.push("Ord");
1654+
}
1655+
1656+
if item.can_derive_partialeq(ctx) {
1657+
derives.push("PartialEq");
1658+
} else {
1659+
needs_partialeq_impl =
1660+
ctx.options().derive_partialeq &&
1661+
ctx.options().impl_partialeq &&
1662+
ctx.lookup_can_derive_partialeq_or_partialord(item.id())
1663+
.map_or(true, |x| {
1664+
x == CannotDeriveReason::ArrayTooLarge
1665+
});
1666+
}
1667+
1668+
if item.can_derive_eq(ctx) {
1669+
derives.push("Eq");
1670+
}
1671+
1672+
if !derives.is_empty() {
1673+
attributes.push(attributes::derives(&derives))
1674+
}
1675+
1676+
let mut tokens = if is_union && self.can_be_rust_union(ctx) {
1677+
quote! {
1678+
#( #attributes )*
1679+
pub union #canonical_ident
1680+
}
1681+
} else {
1682+
quote! {
1683+
#( #attributes )*
1684+
pub struct #canonical_ident
1685+
}
1686+
};
1687+
16771688
tokens.append(quote! {
16781689
#generics {
16791690
#( #fields )*

‎src/codegen/struct_layout.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -288,18 +288,9 @@ impl<'a> StructLayoutTracker<'a> {
288288
}
289289
}
290290

291-
pub fn align_struct(&self, layout: Layout) -> Option<quote::Tokens> {
292-
if self.max_field_align < layout.align &&
291+
pub fn requires_explicit_align(&self, layout: Layout) -> bool {
292+
self.max_field_align < layout.align &&
293293
layout.align <= mem::size_of::<*mut ()>()
294-
{
295-
let ty = helpers::blob(Layout::new(0, layout.align));
296-
297-
Some(quote! {
298-
pub __bindgen_align: #ty ,
299-
})
300-
} else {
301-
None
302-
}
303294
}
304295

305296
fn padding_bytes(&self, layout: Layout) -> usize {

‎tests/expectations/tests/bitfield-32bit-overflow.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@
44
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
55

66

7-
#[repr(C)]
7+
#[repr(C, packed)]
88
#[derive(Debug, Default, Copy)]
99
pub struct MuchBitfield {
1010
pub _bitfield_1: [u8; 5usize],
11-
pub __bindgen_align: [u8; 0usize],
1211
}
1312
#[test]
1413
fn bindgen_test_layout_MuchBitfield() {

‎tests/expectations/tests/bitfield-method-same-name.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@
44
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
55

66

7-
#[repr(C)]
7+
#[repr(C, packed)]
88
#[derive(Debug, Default, Copy)]
99
pub struct Foo {
1010
pub _bitfield_1: u8,
11-
pub __bindgen_align: [u8; 0usize],
1211
}
1312
#[test]
1413
fn bindgen_test_layout_Foo() {

‎tests/expectations/tests/issue-1034.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/* automatically generated by rust-bindgen */
2+
3+
4+
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
5+
6+
7+
#[repr(C, packed)]
8+
#[derive(Debug, Default, Copy)]
9+
pub struct S2 {
10+
pub _bitfield_1: u16,
11+
}
12+
#[test]
13+
fn bindgen_test_layout_S2() {
14+
assert_eq!(
15+
::std::mem::size_of::<S2>(),
16+
2usize,
17+
concat!("Size of: ", stringify!(S2))
18+
);
19+
assert_eq!(
20+
::std::mem::align_of::<S2>(),
21+
1usize,
22+
concat!("Alignment of ", stringify!(S2))
23+
);
24+
}
25+
impl Clone for S2 {
26+
fn clone(&self) -> Self {
27+
*self
28+
}
29+
}
30+
impl S2 {
31+
#[inline]
32+
pub fn new_bitfield_1() -> u16 {
33+
0
34+
}
35+
}

‎tests/expectations/tests/only_bitfields.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,10 @@
44
#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
55

66

7-
#[repr(C)]
7+
#[repr(C, packed)]
88
#[derive(Debug, Default, Copy)]
99
pub struct C {
1010
pub _bitfield_1: u8,
11-
pub __bindgen_align: [u8; 0usize],
1211
}
1312
#[test]
1413
fn bindgen_test_layout_C() {

‎tests/headers/issue-1034.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
struct S2 {
3+
unsigned : 11
4+
};

0 commit comments

Comments
 (0)