codegen: Guarantee opaque type layout across all architectures.

Fixes #3279

Author: emilio

bindgen/codegen/helpers.rs

@@ -80,37 +80,16 @@ pub(crate) mod attributes {
 /// just noise.
 /// TODO: Should this be `MaybeUninit`, since padding bytes are effectively
 /// uninitialized?
-pub(crate) fn blob(
-    ctx: &BindgenContext,
-    layout: Layout,
-    ffi_safe: bool,
-) -> syn::Type {
-    let opaque = layout.opaque();
-
-    // 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 = opaque.known_rust_type_for_array().unwrap_or_else(|| {
-        warn!("Found unknown alignment on code generation!");
-        syn::parse_quote! { u8 }
-    });
-
-    let data_len = opaque.array_size().unwrap_or(layout.size);
-
-    if data_len == 1 {
-        ty
-    } else if ffi_safe {
-        ctx.generated_opaque_array();
-        if ctx.options().enable_cxx_namespaces {
-            syn::parse_quote! { root::__BindgenOpaqueArray<#ty, #data_len> }
-        } else {
-            syn::parse_quote! { __BindgenOpaqueArray<#ty, #data_len> }
-        }
+pub(crate) fn blob(ctx: &BindgenContext, layout: Layout) -> syn::Type {
+    // TODO: Maybe parameterize this on the alignment.
+    ctx.generated_opaque_array();
+    let ident =
+        format_ident!("__BindgenOpaqueArray{}", std::cmp::max(1, layout.align));
+    let size = layout.size;
+    if ctx.options().enable_cxx_namespaces {
+        syn::parse_quote! { root::#ident<[u8; #size]> }
     } else {
-        // This is not FFI safe as an argument; the struct above is
-        // preferable.
-        syn::parse_quote! { [ #ty ; #data_len ] }
+        syn::parse_quote! { #ident<[u8; #size]> }
     }
 }
 

bindgen/codegen/mod.rs

@@ -581,7 +581,7 @@ impl CodeGenerator for Module {
                     utils::prepend_complex_type(&mut *result);
                 }
                 if ctx.need_opaque_array_type() {
-                    utils::prepend_opaque_array_type(&mut *result);
+                    utils::prepend_opaque_array_types(&mut *result);
                 }
                 if result.saw_objc {
                     utils::prepend_objc_header(ctx, &mut *result);
@@ -2259,14 +2259,13 @@ impl CodeGenerator for CompInfo {
 
             if has_address {
                 let layout = Layout::new(1, 1);
-                let ty = helpers::blob(ctx, Layout::new(1, 1), false);
                 struct_layout.saw_field_with_layout(
                     "_address",
                     layout,
                     /* offset = */ Some(0),
                 );
                 fields.push(quote! {
-                    pub _address: #ty,
+                    pub _address: u8,
                 });
             }
         }
@@ -2275,8 +2274,7 @@ impl CodeGenerator for CompInfo {
             match layout {
                 Some(l) => {
                     explicit_align = Some(l.align);
-
-                    let ty = helpers::blob(ctx, l, false);
+                    let ty = helpers::blob(ctx, l);
                     fields.push(quote! {
                         pub _bindgen_opaque_blob: #ty ,
                     });
@@ -2310,9 +2308,9 @@ impl CodeGenerator for CompInfo {
                     explicit_align = Some(layout.align);
                 }
                 if !struct_layout.is_rust_union() {
-                    let ty = helpers::blob(ctx, layout, false);
+                    let ty = helpers::blob(ctx, layout);
                     fields.push(quote! {
-                        pub bindgen_union_field: #ty ,
+                        pub bindgen_union_field: #ty,
                     });
                 }
             }
@@ -4069,7 +4067,7 @@ pub(crate) trait TryToOpaque {
         extra: &Self::Extra,
     ) -> error::Result<syn::Type> {
         self.try_get_layout(ctx, extra)
-            .map(|layout| helpers::blob(ctx, layout, true))
+            .map(|layout| helpers::blob(ctx, layout))
     }
 }
 
@@ -4095,7 +4093,7 @@ pub(crate) trait ToOpaque: TryToOpaque {
         extra: &Self::Extra,
     ) -> syn::Type {
         let layout = self.get_layout(ctx, extra);
-        helpers::blob(ctx, layout, true)
+        helpers::blob(ctx, layout)
     }
 }
 
@@ -4146,7 +4144,7 @@ where
     ) -> error::Result<syn::Type> {
         self.try_to_rust_ty(ctx, extra).or_else(|_| {
             if let Ok(layout) = self.try_get_layout(ctx, extra) {
-                Ok(helpers::blob(ctx, layout, true))
+                Ok(helpers::blob(ctx, layout))
             } else {
                 Err(Error::NoLayoutForOpaqueBlob)
             }
@@ -5610,23 +5608,34 @@ pub(crate) mod utils {
         result.extend(old_items);
     }
 
-    pub(crate) fn prepend_opaque_array_type(
+    pub(crate) fn prepend_opaque_array_types(
         result: &mut Vec<proc_macro2::TokenStream>,
     ) {
-        let ty = quote! {
-            /// If Bindgen could only determine the size and alignment of a
-            /// type, it is represented like this.
-            #[derive(PartialEq, Copy, Clone, Debug, Hash)]
-            #[repr(C)]
-            pub struct __BindgenOpaqueArray<T: Copy, const N: usize>(pub [T; N]);
-            impl<T: Copy + Default, const N: usize> Default for __BindgenOpaqueArray<T, N> {
-                fn default() -> Self {
-                    Self([<T as Default>::default(); N])
+        let mut tys = vec![];
+        // If Bindgen could only determine the size and alignment of a type, it is represented like
+        // this. We could use const generics or so but it wouldn't support all our rust targets. We
+        // only expect [u8; N] as the type parameter. FIXME(emilio): Probably generate only the
+        // ones we want.
+        for align in [1usize, 2, 4, 8, 16] {
+            let ident = format_ident!("__BindgenOpaqueArray{}", align);
+            let repr = if align == 1 {
+                quote! { #[repr(C)] }
+            } else {
+                let explicit = super::helpers::ast_ty::int_expr(align as i64);
+                quote! { #[repr(C, align(#explicit))] }
+            };
+            tys.push(quote! {
+                #[derive(PartialEq, Eq, Copy, Clone, Debug, Hash)]
+                #repr
+                pub struct #ident<T>(pub T);
+                impl<const N: usize> Default for #ident<[u8; N]> {
+                    fn default() -> Self {
+                        Self([0u8; N])
+                    }
                 }
-            }
-        };
-
-        result.insert(0, ty);
+            })
+        }
+        result.splice(0..0, tys);
     }
 
     pub(crate) fn build_path(

bindgen/codegen/struct_layout.rs

@@ -367,7 +367,7 @@ impl<'a> StructLayoutTracker<'a> {
     }
 
     fn padding_field(&mut self, layout: Layout) -> proc_macro2::TokenStream {
-        let ty = helpers::blob(self.ctx, layout, false);
+        let ty = helpers::blob(self.ctx, layout);
         let padding_count = self.padding_count;
 
         self.padding_count += 1;

bindgen/ir/analysis/derive.rs

@@ -179,26 +179,11 @@ impl CannotDerive<'_> {
                 return CanDerive::No;
             }
 
-            let layout_can_derive =
-                ty.layout(self.ctx).map_or(CanDerive::Yes, |l| {
-                    l.opaque().array_size_within_derive_limit()
-                });
-
-            match layout_can_derive {
-                CanDerive::Yes => {
-                    trace!(
-                        "    we can trivially derive {} for the layout",
-                        self.derive_trait
-                    );
-                }
-                _ => {
-                    trace!(
-                        "    we cannot derive {} for the layout",
-                        self.derive_trait
-                    );
-                }
-            }
-            return layout_can_derive;
+            trace!(
+                "    we can trivially derive {} for the layout",
+                self.derive_trait
+            );
+            return CanDerive::Yes;
         }
 
         match *ty.kind() {
@@ -338,25 +323,11 @@ impl CannotDerive<'_> {
                             return CanDerive::No;
                         }
 
-                        let layout_can_derive =
-                            ty.layout(self.ctx).map_or(CanDerive::Yes, |l| {
-                                l.opaque().array_size_within_derive_limit()
-                            });
-                        match layout_can_derive {
-                            CanDerive::Yes => {
-                                trace!(
-                                    "    union layout can trivially derive {}",
-                                    self.derive_trait
-                                );
-                            }
-                            _ => {
-                                trace!(
-                                    "    union layout cannot derive {}",
-                                    self.derive_trait
-                                );
-                            }
-                        }
-                        return layout_can_derive;
+                        trace!(
+                            "    union layout can trivially derive {}",
+                            self.derive_trait
+                        );
+                        return CanDerive::Yes;
                     }
                 }
 

bindgen/ir/item.rs

@@ -12,7 +12,6 @@ use super::derive::{
 use super::dot::DotAttributes;
 use super::function::{Function, FunctionKind};
 use super::item_kind::ItemKind;
-use super::layout::Opaque;
 use super::module::Module;
 use super::template::{AsTemplateParam, TemplateParameters};
 use super::traversal::{EdgeKind, Trace, Tracer};
@@ -451,7 +450,7 @@ impl Item {
         ctx: &mut BindgenContext,
     ) -> TypeId {
         let location = ty.declaration().location();
-        let ty = Opaque::from_clang_ty(ty, ctx);
+        let ty = Type::new_opaque_from_clang_ty(ty, ctx);
         let kind = ItemKind::Type(ty);
         let parent = ctx.root_module().into();
         ctx.add_item(

bindgen/ir/layout.rs

@@ -1,10 +1,6 @@
 //! Intermediate representation for the physical layout of some type.
 
-use super::derive::CanDerive;
-use super::ty::{Type, TypeKind, RUST_DERIVE_IN_ARRAY_LIMIT};
-use crate::clang;
 use crate::ir::context::BindgenContext;
-use std::cmp;
 
 /// A type that represents the struct layout of a type.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -71,56 +67,4 @@ impl Layout {
     pub(crate) fn for_size(ctx: &BindgenContext, size: usize) -> Self {
         Self::for_size_internal(ctx.target_pointer_size(), size)
     }
-
-    /// Get this layout as an opaque type.
-    pub(crate) fn opaque(&self) -> Opaque {
-        Opaque(*self)
-    }
-}
-
-/// When we are treating a type as opaque, it is just a blob with a `Layout`.
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub(crate) struct Opaque(pub(crate) Layout);
-
-impl Opaque {
-    /// Construct a new opaque type from the given clang type.
-    pub(crate) fn from_clang_ty(
-        ty: &clang::Type,
-        ctx: &BindgenContext,
-    ) -> Type {
-        let layout = Layout::new(ty.size(ctx), ty.align(ctx));
-        let ty_kind = TypeKind::Opaque;
-        let is_const = ty.is_const();
-        Type::new(None, Some(layout), ty_kind, is_const)
-    }
-
-    /// Return the known rust type we should use to create a correctly-aligned
-    /// field with this layout.
-    pub(crate) fn known_rust_type_for_array(&self) -> Option<syn::Type> {
-        Layout::known_type_for_size(self.0.align)
-    }
-
-    /// 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(crate) 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
-        }
-    }
-
-    /// Return `true` if this opaque layout's array size will fit within the
-    /// maximum number of array elements that Rust allows deriving traits
-    /// with. Return `false` otherwise.
-    pub(crate) fn array_size_within_derive_limit(&self) -> CanDerive {
-        if self
-            .array_size()
-            .is_some_and(|size| size <= RUST_DERIVE_IN_ARRAY_LIMIT)
-        {
-            CanDerive::Yes
-        } else {
-            CanDerive::Manually
-        }
-    }
 }

bindgen/ir/ty.rs

@@ -6,7 +6,7 @@ use super::dot::DotAttributes;
 use super::enum_ty::Enum;
 use super::function::FunctionSig;
 use super::item::{IsOpaque, Item};
-use super::layout::{Layout, Opaque};
+use super::layout::Layout;
 use super::objc::ObjCInterface;
 use super::template::{
     AsTemplateParam, TemplateInstantiation, TemplateParameters,
@@ -67,6 +67,17 @@ impl Type {
         }
     }
 
+    /// Construct an opaque item from a clang type.
+    pub(crate) fn new_opaque_from_clang_ty(
+        ty: &clang::Type,
+        ctx: &BindgenContext,
+    ) -> Self {
+        let layout = Layout::new(ty.size(ctx), ty.align(ctx));
+        let ty_kind = TypeKind::Opaque;
+        let is_const = ty.is_const();
+        Type::new(None, Some(layout), ty_kind, is_const)
+    }
+
     /// Which kind of type is this?
     pub(crate) fn kind(&self) -> &TypeKind {
         &self.kind
@@ -737,7 +748,7 @@ impl Type {
                  opaque type instead."
             );
             return Ok(ParseResult::New(
-                Opaque::from_clang_ty(&canonical_ty, ctx),
+                Self::new_opaque_from_clang_ty(&canonical_ty, ctx),
                 None,
             ));
         }
@@ -868,7 +879,8 @@ impl Type {
                                          from class template or base \
                                          specifier, using opaque blob"
                                     );
-                                    let opaque = Opaque::from_clang_ty(ty, ctx);
+                                    let opaque =
+                                        Self::new_opaque_from_clang_ty(ty, ctx);
                                     return Ok(ParseResult::New(opaque, None));
                                 }
                             }