Repository: rust-lang/rust-bindgen (rust)

Issue:
AArch64 Linux va_list not FFI-safe
<!-- Thanks for filing a bindgen issue! We appreciate it :-) -->

Hello folks,
We noticed that for AArch64 Linux, va_list was being translated to a type that was not FFI-safe.
Whilst we aren't aware of any cases where va_list would be used by Rust, this does result in warnings appearing for some projects when built for Arm and not other architectures. Rather than try to block these functions from being translated in various crates, I thought it best to raise something here as a general solution may be better.

I suspect there is some subtle interaction with clang and the builtin va_list type for AArch64 Linux.

### Input C/C++ Header

```C++
// typedefs below come from <stdarg.h>
typedef __builtin_va_list __gnuc_va_list;
typedef __builtin_va_list va_list;

int my_func(char *format, va_list args);
```

### Bindgen Invocation

<!-- Place either the `bindgen::Builder` or the command line flags used here. -->

```
$ bindgen input.h
```

### Actual Results

```rust
/* automatically generated by rust-bindgen 0.69.4 */

pub type __gnuc_va_list = [u64; 4usize];
pub type va_list = [u64; 4usize];
extern "C" {
    pub fn my_func(
        format: *mut ::std::os::raw::c_char,
        args: va_list,
    ) -> ::std::os::raw::c_int;
}
```

and/or

```
warning: `extern` block uses type `[u64; 4]`, which is not FFI-safe
 --> test.rs:8:15
  |
8 |         args: va_list,
  |               ^^^^^^^ not FFI-safe
  |
  = help: consider passing a pointer to the array
  = note: passing raw arrays by value is not FFI-safe
  = note: `#[warn(improper_ctypes)]` on by default
```

### Expected Results

We don't expect the warning to appear. Maybe something like this would be better?
```rust
#[repr(C)]
#[repr(align(8))]
#[derive(Debug, Copy, Clone)]
pub struct va_list {
    pub _bindgen_opaque_blob: [u64; 4usize],
}
```


Pull request description:
Wrap the array representation of opaque types in a #[repr(C)] struct
Fixes #2845. Wrapping the generated array in a struct ensures that it is passed in an FFI-safe way. This does assume that the calling convention for structs is only dependent on size and alignment, but that seems like an improvement on using a type whose calling convention is not considered stable at all. Padding bytes (etc.) have been excluded from this treatment.

Patch:
diff --git a/bindgen-integration/src/lib.rs b/bindgen-integration/src/lib.rs
index b939d25b..13f5bd88 100755
--- a/bindgen-integration/src/lib.rs
+++ b/bindgen-integration/src/lib.rs
@@ -254,7 +254,7 @@ fn test_item_rename() {
 #[test]
 fn test_matching_with_rename() {
     assert_eq!(bindings::enum_to_be_constified_THREE, 3);
-    assert_eq!(unsafe { bindings::TEMPLATED_CONST_VALUE.len() }, 30);
+    assert_eq!(unsafe { bindings::TEMPLATED_CONST_VALUE.0.len() }, 30);
 }
 
 #[test]
diff --git a/bindgen/codegen/helpers.rs b/bindgen/codegen/helpers.rs
index aac04a12..c572b888 100644
--- a/bindgen/codegen/helpers.rs
+++ b/bindgen/codegen/helpers.rs
@@ -75,9 +75,16 @@ pub(crate) mod attributes {
     }
 }
 
-/// Generates a proper type for a field or type with a given `Layout`, that is,
-/// a type with the correct size and alignment restrictions.
-pub(crate) fn blob(layout: Layout) -> syn::Type {
+/// The `ffi_safe` argument should be true if this is a type that the user might
+/// reasonably use, e.g. not struct padding, where the __BindgenOpaqueArray is
+/// 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
@@ -93,7 +100,12 @@ pub(crate) fn blob(layout: Layout) -> syn::Type {
 
     if data_len == 1 {
         ty
+    } else if ffi_safe && ctx.options().rust_features().min_const_generics {
+        ctx.generated_opaque_array();
+        syn::parse_quote! { __BindgenOpaqueArray<#ty, #data_len> }
     } else {
+        // This is not FFI safe as an argument; the struct above is
+        // preferable.
         syn::parse_quote! { [ #ty ; #data_len ] }
     }
 }
diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs
index 1561d4d8..ab3e0ce8 100644
--- a/bindgen/codegen/mod.rs
+++ b/bindgen/codegen/mod.rs
@@ -581,6 +581,9 @@ impl CodeGenerator for Module {
                 if ctx.need_bindgen_complex_type() {
                     utils::prepend_complex_type(&mut *result);
                 }
+                if ctx.need_opaque_array_type() {
+                    utils::prepend_opaque_array_type(&mut *result);
+                }
                 if result.saw_objc {
                     utils::prepend_objc_header(ctx, &mut *result);
                 }
@@ -2275,7 +2278,7 @@ impl CodeGenerator for CompInfo {
 
             if has_address {
                 let layout = Layout::new(1, 1);
-                let ty = helpers::blob(Layout::new(1, 1));
+                let ty = helpers::blob(ctx, Layout::new(1, 1), false);
                 struct_layout.saw_field_with_layout(
                     "_address",
                     layout,
@@ -2292,7 +2295,7 @@ impl CodeGenerator for CompInfo {
                 Some(l) => {
                     explicit_align = Some(l.align);
 
-                    let ty = helpers::blob(l);
+                    let ty = helpers::blob(ctx, l, false);
                     fields.push(quote! {
                         pub _bindgen_opaque_blob: #ty ,
                     });
@@ -2326,7 +2329,7 @@ impl CodeGenerator for CompInfo {
             }
 
             if !struct_layout.is_rust_union() {
-                let ty = helpers::blob(layout);
+                let ty = helpers::blob(ctx, layout, false);
                 fields.push(quote! {
                     pub bindgen_union_field: #ty ,
                 });
@@ -4048,7 +4051,8 @@ pub(crate) trait TryToOpaque {
         ctx: &BindgenContext,
         extra: &Self::Extra,
     ) -> error::Result<syn::Type> {
-        self.try_get_layout(ctx, extra).map(helpers::blob)
+        self.try_get_layout(ctx, extra)
+            .map(|layout| helpers::blob(ctx, layout, true))
     }
 }
 
@@ -4074,7 +4078,7 @@ pub(crate) trait ToOpaque: TryToOpaque {
         extra: &Self::Extra,
     ) -> syn::Type {
         let layout = self.get_layout(ctx, extra);
-        helpers::blob(layout)
+        helpers::blob(ctx, layout, true)
     }
 }
 
@@ -4125,7 +4129,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(layout))
+                Ok(helpers::blob(ctx, layout, true))
             } else {
                 Err(Error::NoLayoutForOpaqueBlob)
             }
@@ -5573,6 +5577,25 @@ pub(crate) mod utils {
         result.extend(old_items);
     }
 
+    pub(crate) fn prepend_opaque_array_type(
+        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])
+                }
+            }
+        };
+
+        result.insert(0, ty);
+    }
+
     pub(crate) fn build_path(
         item: &Item,
         ctx: &BindgenContext,
diff --git a/bindgen/codegen/struct_layout.rs b/bindgen/codegen/struct_layout.rs
index 0645d8a8..88b250cf 100644
--- a/bindgen/codegen/struct_layout.rs
+++ b/bindgen/codegen/struct_layout.rs
@@ -393,7 +393,7 @@ impl<'a> StructLayoutTracker<'a> {
     }
 
     fn padding_field(&mut self, layout: Layout) -> proc_macro2::TokenStream {
-        let ty = helpers::blob(layout);
+        let ty = helpers::blob(self.ctx, layout, false);
         let padding_count = self.padding_count;
 
         self.padding_count += 1;
diff --git a/bindgen/features.rs b/bindgen/features.rs
index c278d10f..e434c701 100644
--- a/bindgen/features.rs
+++ b/bindgen/features.rs
@@ -263,7 +263,10 @@ define_rust_targets! {
     Stable_1_71(71) => { c_unwind_abi: #106075 },
     Stable_1_68(68) => { abi_efiapi: #105795 },
     Stable_1_64(64) => { core_ffi_c: #94503 },
-    Stable_1_51(51) => { raw_ref_macros: #80886 },
+    Stable_1_51(51) => {
+        raw_ref_macros: #80886,
+        min_const_generics: #74878,
+    },
     Stable_1_59(59) => { const_cstr: #54745 },
     Stable_1_47(47) => { larger_arrays: #74060 },
     Stable_1_43(43) => { associated_constants: #68952 },
diff --git a/bindgen/ir/context.rs b/bindgen/ir/context.rs
index df7db899..fe97a8c5 100644
--- a/bindgen/ir/context.rs
+++ b/bindgen/ir/context.rs
@@ -388,6 +388,9 @@ pub(crate) struct BindgenContext {
     /// The options given by the user via cli or other medium.
     options: BindgenOptions,
 
+    /// Whether an opaque array was generated
+    generated_opaque_array: Cell<bool>,
+
     /// Whether a bindgen complex was generated
     generated_bindgen_complex: Cell<bool>,
 
@@ -595,6 +598,7 @@ If you encounter an error missing from this list, please file an issue or a PR!"
             options,
             generated_bindgen_complex: Cell::new(false),
             generated_bindgen_float16: Cell::new(false),
+            generated_opaque_array: Cell::new(false),
             allowlisted: None,
             blocklisted_types_implement_traits: Default::default(),
             codegen_items: None,
@@ -2607,6 +2611,16 @@ If you encounter an error missing from this list, please file an issue or a PR!"
         }
     }
 
+    /// Call if an opaque array is generated
+    pub(crate) fn generated_opaque_array(&self) {
+        self.generated_opaque_array.set(true)
+    }
+
+    /// Whether we need to generate the opaque array type
+    pub(crate) fn need_opaque_array_type(&self) -> bool {
+        self.generated_opaque_array.get()
+    }
+
     /// Call if a bindgen complex is generated
     pub(crate) fn generated_bindgen_complex(&self) {
         self.generated_bindgen_complex.set(true);
diff --git a/bindgen/ir/ty.rs b/bindgen/ir/ty.rs
index 31606465..98df40d2 100644
--- a/bindgen/ir/ty.rs
+++ b/bindgen/ir/ty.rs
@@ -582,7 +582,7 @@ pub(crate) enum TypeKind {
     /// A compound type, that is, a class, struct, or union.
     Comp(CompInfo),
 
-    /// An opaque type that we just don't understand. All usage of this shoulf
+    /// An opaque type that we just don't understand. All usage of this should
     /// result in an opaque blob of bytes generated from the containing type's
     /// layout.
     Opaque,


Test patch:
diff --git a/bindgen-tests/tests/expectations/tests/issue-544-stylo-creduce-2.rs b/bindgen-tests/tests/expectations/tests/issue-544-stylo-creduce-2.rs
index c81b6729..8a752f69 100644
--- a/bindgen-tests/tests/expectations/tests/issue-544-stylo-creduce-2.rs
+++ b/bindgen-tests/tests/expectations/tests/issue-544-stylo-creduce-2.rs
@@ -1,10 +1,20 @@
 #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+/// 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])
+    }
+}
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
 pub struct Foo {
     pub member: *mut Foo_SecondAlias,
 }
-pub type Foo_FirstAlias = [u8; 0usize];
+pub type Foo_FirstAlias = __BindgenOpaqueArray<u8, 0usize>;
 pub type Foo_SecondAlias = Foo;
 impl Default for Foo {
     fn default() -> Self {
diff --git a/bindgen-tests/tests/expectations/tests/libclang-9/issue-544-stylo-creduce-2.rs b/bindgen-tests/tests/expectations/tests/libclang-9/issue-544-stylo-creduce-2.rs
index 5c5ad156..7f0471bd 100644
--- a/bindgen-tests/tests/expectations/tests/libclang-9/issue-544-stylo-creduce-2.rs
+++ b/bindgen-tests/tests/expectations/tests/libclang-9/issue-544-stylo-creduce-2.rs
@@ -1,11 +1,21 @@
 #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+/// 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])
+    }
+}
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
 pub struct Foo {
-    pub member: *mut [u8; 0usize],
+    pub member: *mut __BindgenOpaqueArray<u8, 0usize>,
 }
-pub type Foo_FirstAlias = [u8; 0usize];
-pub type Foo_SecondAlias = [u8; 0usize];
+pub type Foo_FirstAlias = __BindgenOpaqueArray<u8, 0usize>;
+pub type Foo_SecondAlias = __BindgenOpaqueArray<u8, 0usize>;
 impl Default for Foo {
     fn default() -> Self {
         let mut s = ::std::mem::MaybeUninit::<Self>::uninit();
diff --git a/bindgen-tests/tests/expectations/tests/non-type-params.rs b/bindgen-tests/tests/expectations/tests/non-type-params.rs
index 64b293cb..afd21fb7 100644
--- a/bindgen-tests/tests/expectations/tests/non-type-params.rs
+++ b/bindgen-tests/tests/expectations/tests/non-type-params.rs
@@ -1,11 +1,21 @@
 #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+/// 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])
+    }
+}
 pub type Array16 = u8;
-pub type ArrayInt4 = [u32; 4usize];
+pub type ArrayInt4 = __BindgenOpaqueArray<u32, 4usize>;
 #[repr(C)]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct UsesArray {
-    pub array_char_16: [u8; 16usize],
-    pub array_bool_8: [u8; 8usize],
+    pub array_char_16: __BindgenOpaqueArray<u8, 16usize>,
+    pub array_bool_8: __BindgenOpaqueArray<u8, 8usize>,
     pub array_int_4: ArrayInt4,
 }
 #[allow(clippy::unnecessary_operation, clippy::identity_op)]
diff --git a/bindgen-tests/tests/expectations/tests/nsBaseHashtable.rs b/bindgen-tests/tests/expectations/tests/nsBaseHashtable.rs
index f2f7eefb..32fcc37a 100644
--- a/bindgen-tests/tests/expectations/tests/nsBaseHashtable.rs
+++ b/bindgen-tests/tests/expectations/tests/nsBaseHashtable.rs
@@ -1,4 +1,14 @@
 #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+/// 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])
+    }
+}
 #[repr(C)]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct nsBaseHashtableET {
@@ -14,7 +24,7 @@ pub struct nsTHashtable {
 pub struct nsBaseHashtable {
     pub _address: u8,
 }
-pub type nsBaseHashtable_KeyType = [u8; 0usize];
+pub type nsBaseHashtable_KeyType = __BindgenOpaqueArray<u8, 0usize>;
 pub type nsBaseHashtable_EntryType = nsBaseHashtableET;
 #[repr(C)]
 #[derive(Debug, Copy, Clone)]
diff --git a/bindgen-tests/tests/expectations/tests/partial-specialization-and-inheritance.rs b/bindgen-tests/tests/expectations/tests/partial-specialization-and-inheritance.rs
index e7c9a38d..cd22cce4 100644
--- a/bindgen-tests/tests/expectations/tests/partial-specialization-and-inheritance.rs
+++ b/bindgen-tests/tests/expectations/tests/partial-specialization-and-inheritance.rs
@@ -1,4 +1,14 @@
 #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+/// 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])
+    }
+}
 #[repr(C)]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct Base {
@@ -16,7 +26,7 @@ pub struct Usage {
 }
 extern "C" {
     #[link_name = "\u{1}_ZN5Usage13static_memberE"]
-    pub static mut Usage_static_member: [u32; 2usize];
+    pub static mut Usage_static_member: __BindgenOpaqueArray<u32, 2usize>;
 }
 #[allow(clippy::unnecessary_operation, clippy::identity_op)]
 const _: () = {
diff --git a/bindgen-tests/tests/expectations/tests/size_t_template.rs b/bindgen-tests/tests/expectations/tests/size_t_template.rs
index e422131c..9126c507 100644
--- a/bindgen-tests/tests/expectations/tests/size_t_template.rs
+++ b/bindgen-tests/tests/expectations/tests/size_t_template.rs
@@ -1,8 +1,18 @@
 #![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+/// 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])
+    }
+}
 #[repr(C)]
 #[derive(Debug, Default, Copy, Clone)]
 pub struct C {
-    pub arr: [u32; 3usize],
+    pub arr: __BindgenOpaqueArray<u32, 3usize>,
 }
 #[allow(clippy::unnecessary_operation, clippy::identity_op)]
 const _: () = {
diff --git a/bindgen-tests/tests/expectations/tests/va_list_aarch64_linux.rs b/bindgen-tests/tests/expectations/tests/va_list_aarch64_linux.rs
new file mode 100644
index 00000000..c4cc9446
--- /dev/null
+++ b/bindgen-tests/tests/expectations/tests/va_list_aarch64_linux.rs
@@ -0,0 +1,18 @@
+#![allow(dead_code, non_snake_case, non_camel_case_types, non_upper_case_globals)]
+/// 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])
+    }
+}
+pub type va_list = __BindgenOpaqueArray<u64, 4usize>;
+extern "C" {
+    pub fn vprintf(
+        format: *const ::std::os::raw::c_char,
+        vlist: __BindgenOpaqueArray<u64, 4usize>,
+    ) -> ::std::os::raw::c_int;
+}
diff --git a/bindgen-tests/tests/headers/va_list_aarch64_linux.h b/bindgen-tests/tests/headers/va_list_aarch64_linux.h
new file mode 100644
index 00000000..7d2206a7
--- /dev/null
+++ b/bindgen-tests/tests/headers/va_list_aarch64_linux.h
@@ -0,0 +1,4 @@
+// bindgen-flags: -- --target=aarch64-unknown-linux-gnu
+
+typedef __builtin_va_list va_list;
+int vprintf(const char* format, va_list vlist);