Auto merge of #413 - emilio:debug-array-unaligned, r=fitzgen

codegen: Avoid generating invalid Rust code when a struct is not properly aligned.

r? @fitzgen

cc #412

3 files changed, 42 insertions, 15 deletions

diff --git a/libbindgen/src/codegen/helpers.rs b/libbindgen/src/codegen/helpers.rs
index b4cc75f5..06dadab0 100644
--- a/libbindgen/src/codegen/helpers.rs
+++ b/libbindgen/src/codegen/helpers.rs
@@ -49,20 +49,22 @@ impl BlobTyBuilder {
     }
 
     pub fn build(self) -> P<ast::Ty> {
-        use std::cmp;
+        let opaque = self.layout.opaque();
 
-        let ty_name = match self.layout.align {
-            8 => "u64",
-            4 => "u32",
-            2 => "u16",
-            1 | _ => "u8",
-        };
-        let data_len = if ty_name == "u8" {
-            self.layout.size
-        } else {
-            self.layout.size / cmp::max(self.layout.align, 1)
+        // FIXME(emilio, #412): We fall back to byte alignment, but there are
+        // some things that legitimately are more than 8-byte aligned.
+        //
+        // Eventually we should be able to `unwrap` here, but...
+        let ty_name = match opaque.known_rust_type_for_array() {
+            Some(ty) => ty,
+            None => {
+                warn!("Found unknown alignment on code generation!");
+                "u8"
+            }
         };
 
+        let data_len = opaque.array_size().unwrap_or(self.layout.size);
+
         let inner_ty = aster::AstBuilder::new().ty().path().id(ty_name).build();
         if data_len == 1 {
             inner_ty
diff --git a/libbindgen/src/ir/comp.rs b/libbindgen/src/ir/comp.rs
index 0d1c653e..968bf879 100644
--- a/libbindgen/src/ir/comp.rs
+++ b/libbindgen/src/ir/comp.rs
@@ -893,6 +893,9 @@ impl<'a> CanDeriveCopy<'a> for CompInfo {
 
         if self.kind == CompKind::Union {
             if !ctx.options().unstable_rust {
+                // NOTE: If there's no template parameters we can derive copy
+                // unconditionally, since arrays are magical for rustc, and
+                // __BindgenUnionField always implements copy.
                 return true;
             }
 
diff --git a/libbindgen/src/ir/layout.rs b/libbindgen/src/ir/layout.rs
index 3a07f7e8..033fff62 100644
--- a/libbindgen/src/ir/layout.rs
+++ b/libbindgen/src/ir/layout.rs
@@ -46,12 +46,35 @@ impl Layout {
 /// When we are treating a type as opaque, it is just a blob with a `Layout`.
 pub struct Opaque(pub Layout);
 
+impl Opaque {
+    /// Return the known rust type we should use to create a correctly-aligned
+    /// field with this layout.
+    pub fn known_rust_type_for_array(&self) -> Option<&'static str> {
+        Some(match self.0.align {
+            8 => "u64",
+            4 => "u32",
+            2 => "u16",
+            1 => "u8",
+            _ => return None,
+        })
+    }
+
+    /// Return the array size that an opaque type for this layout should have if
+    /// we know the correct type for it, or `None` otherwise.
+    pub fn array_size(&self) -> Option<usize> {
+        if self.known_rust_type_for_array().is_some() {
+            Some(self.0.size / cmp::max(self.0.align, 1))
+        } else {
+            None
+        }
+    }
+}
+
 impl CanDeriveDebug for Opaque {
     type Extra = ();
 
     fn can_derive_debug(&self, _: &BindgenContext, _: ()) -> bool {
-        let size_divisor = cmp::max(1, self.0.align);
-        self.0.size / size_divisor <= RUST_DERIVE_IN_ARRAY_LIMIT
+        self.array_size().map_or(false, |size| size <= RUST_DERIVE_IN_ARRAY_LIMIT)
     }
 }
 
@@ -59,8 +82,7 @@ impl<'a> CanDeriveCopy<'a> for Opaque {
     type Extra = ();
 
     fn can_derive_copy(&self, _: &BindgenContext, _: ()) -> bool {
-        let size_divisor = cmp::max(1, self.0.align);
-        self.0.size / size_divisor <= RUST_DERIVE_IN_ARRAY_LIMIT
+        self.array_size().map_or(false, |size| size <= RUST_DERIVE_IN_ARRAY_LIMIT)
     }
 
     fn can_derive_copy_in_array(&self, ctx: &BindgenContext, _: ()) -> bool {