struct_layout: Fix field offset computation for packed(n) structs.

This can cause unnecessary padding to be computed otherwise at the end
of the struct.

With repr(packed(n)), a field can have padding to adjacent fields as
long as its alignment is less than n. So reuse the code we have to align
to a field layout, aligning to the struct layout instead.

Fixes #1934

6 files changed, 84 insertions, 16 deletions

diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs
index 15aea22a..0d93c491 100644
--- a/src/codegen/mod.rs
+++ b/src/codegen/mod.rs
@@ -1650,7 +1650,7 @@ impl CodeGenerator for CompInfo {
 
         let ty = item.expect_type();
         let layout = ty.layout(ctx);
-        let mut packed = self.is_packed(ctx, &layout);
+        let mut packed = self.is_packed(ctx, layout.as_ref());
 
         let canonical_name = item.canonical_name(ctx);
         let canonical_ident = ctx.rust_ident(&canonical_name);
diff --git a/src/codegen/struct_layout.rs b/src/codegen/struct_layout.rs
index 54dfd86e..4536e889 100644
--- a/src/codegen/struct_layout.rs
+++ b/src/codegen/struct_layout.rs
@@ -18,6 +18,7 @@ pub struct StructLayoutTracker<'a> {
     ctx: &'a BindgenContext,
     comp: &'a CompInfo,
     is_packed: bool,
+    known_type_layout: Option<Layout>,
     latest_offset: usize,
     padding_count: usize,
     latest_field_layout: Option<Layout>,
@@ -86,11 +87,14 @@ impl<'a> StructLayoutTracker<'a> {
         ty: &'a Type,
         name: &'a str,
     ) -> Self {
+        let known_type_layout = ty.layout(ctx);
+        let is_packed = comp.is_packed(ctx, known_type_layout.as_ref());
         StructLayoutTracker {
             name,
             ctx,
             comp,
-            is_packed: comp.is_packed(ctx, &ty.layout(ctx)),
+            is_packed,
+            known_type_layout,
             latest_offset: 0,
             padding_count: 0,
             latest_field_layout: None,
@@ -180,23 +184,32 @@ impl<'a> StructLayoutTracker<'a> {
 
         let will_merge_with_bitfield = self.align_to_latest_field(field_layout);
 
+        let padding_bytes = match field_offset {
+            Some(offset) if offset / 8 > self.latest_offset => {
+                offset / 8 - self.latest_offset
+            }
+            _ => {
+                if will_merge_with_bitfield || field_layout.align == 0 {
+                    0
+                } else if !self.is_packed {
+                    self.padding_bytes(field_layout)
+                } else if let Some(l) = self.known_type_layout {
+                    self.padding_bytes(l)
+                } else {
+                    0
+                }
+            }
+        };
+
+        self.latest_offset += padding_bytes;
+
         let padding_layout = if self.is_packed {
             None
         } else {
-            let padding_bytes = match field_offset {
-                Some(offset) if offset / 8 > self.latest_offset => {
-                    offset / 8 - self.latest_offset
-                }
-                _ if will_merge_with_bitfield || field_layout.align == 0 => 0,
-                _ => self.padding_bytes(field_layout),
-            };
-
             // Otherwise the padding is useless.
             let need_padding = padding_bytes >= field_layout.align ||
                 field_layout.align > MAX_GUARANTEED_ALIGN;
 
-            self.latest_offset += padding_bytes;
-
             debug!(
                 "Offset: <padding>: {} -> {}",
                 self.latest_offset - padding_bytes,
diff --git a/src/ir/comp.rs b/src/ir/comp.rs
index 22c124fa..1d257d20 100644
--- a/src/ir/comp.rs
+++ b/src/ir/comp.rs
@@ -1576,7 +1576,7 @@ impl CompInfo {
     pub fn is_packed(
         &self,
         ctx: &BindgenContext,
-        layout: &Option<Layout>,
+        layout: Option<&Layout>,
     ) -> bool {
         if self.packed_attr {
             return true;
@@ -1584,7 +1584,7 @@ impl CompInfo {
 
         // Even though `libclang` doesn't expose `#pragma packed(...)`, we can
         // detect it through its effects.
-        if let Some(ref parent_layout) = *layout {
+        if let Some(parent_layout) = layout {
             if self.fields().iter().any(|f| match *f {
                 Field::Bitfields(ref unit) => {
                     unit.layout().align > parent_layout.align
@@ -1739,7 +1739,7 @@ impl IsOpaque for CompInfo {
             //
             // See https://github.com/rust-lang/rust-bindgen/issues/537 and
             // https://github.com/rust-lang/rust/issues/33158
-            if self.is_packed(ctx, layout) &&
+            if self.is_packed(ctx, layout.as_ref()) &&
                 layout.map_or(false, |l| l.align > 1)
             {
                 warn!("Found a type that is both packed and aligned to greater than \
diff --git a/tests/expectations/tests/divide-by-zero-in-struct-layout.rs b/tests/expectations/tests/divide-by-zero-in-struct-layout.rs
index 1366548e..1484719b 100644
--- a/tests/expectations/tests/divide-by-zero-in-struct-layout.rs
+++ b/tests/expectations/tests/divide-by-zero-in-struct-layout.rs
@@ -130,7 +130,6 @@ impl WithBitfieldAndAttrPacked {
 pub struct WithBitfieldAndPacked {
     pub _bitfield_1: __BindgenBitfieldUnit<[u8; 0usize], u8>,
     pub a: ::std::os::raw::c_uint,
-    pub __bindgen_padding_0: u8,
 }
 impl WithBitfieldAndPacked {
     #[inline]
diff --git a/tests/expectations/tests/packed-n-with-padding.rs b/tests/expectations/tests/packed-n-with-padding.rs
new file mode 100644
index 00000000..13cb0306
--- /dev/null
+++ b/tests/expectations/tests/packed-n-with-padding.rs
@@ -0,0 +1,48 @@
+#![allow(
+    dead_code,
+    non_snake_case,
+    non_camel_case_types,
+    non_upper_case_globals
+)]
+
+#[repr(C, packed(2))]
+#[derive(Debug, Default, Copy, Clone)]
+pub struct Packed {
+    pub a: ::std::os::raw::c_char,
+    pub b: ::std::os::raw::c_short,
+    pub c: ::std::os::raw::c_char,
+    pub d: ::std::os::raw::c_int,
+}
+#[test]
+fn bindgen_test_layout_Packed() {
+    assert_eq!(
+        ::std::mem::size_of::<Packed>(),
+        10usize,
+        concat!("Size of: ", stringify!(Packed))
+    );
+    assert_eq!(
+        ::std::mem::align_of::<Packed>(),
+        2usize,
+        concat!("Alignment of ", stringify!(Packed))
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<Packed>())).a as *const _ as usize },
+        0usize,
+        concat!("Offset of field: ", stringify!(Packed), "::", stringify!(a))
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<Packed>())).b as *const _ as usize },
+        2usize,
+        concat!("Offset of field: ", stringify!(Packed), "::", stringify!(b))
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<Packed>())).c as *const _ as usize },
+        4usize,
+        concat!("Offset of field: ", stringify!(Packed), "::", stringify!(c))
+    );
+    assert_eq!(
+        unsafe { &(*(::std::ptr::null::<Packed>())).d as *const _ as usize },
+        6usize,
+        concat!("Offset of field: ", stringify!(Packed), "::", stringify!(d))
+    );
+}
diff --git a/tests/headers/packed-n-with-padding.h b/tests/headers/packed-n-with-padding.h
new file mode 100644
index 00000000..8a6233b5
--- /dev/null
+++ b/tests/headers/packed-n-with-padding.h
@@ -0,0 +1,8 @@
+#pragma pack(push, 2)
+struct Packed {
+    char a;
+    short b;
+    char c;
+    int d;
+};
+#pragma pack(pop)