libvlc-sys: fix cross-platform va_list layout

bindgen spells the log callback's va_list parameter the way the host
that ran it does. The bindings are generated once and compiled
everywhere, so that spelling only happens to be right on the generating
host.
This was alright for most hosts but is fragile on aarch64 where va_list
is a plain 32 bytes struct.

Given we only forward the list to vsnprintf, extracting and simplifying
the cross-platform rust implementation from the unstable std-lib is
cheap and sound. We will just have to re-export the official type once
it's stablilized.

libvlc-sys/bindings.rs

@@ -1,5 +1,7 @@
 /* automatically generated by rust-bindgen 0.72.1 */
 
+pub use crate::valist::VaList;
+
 pub const VLC_VLC_H: u32 = 1;
 pub const VLC_LIBVLC_H: u32 = 1;
 pub const VLC_LIBVLC_RENDERER_DISCOVERER_H: u32 = 1;
@@ -15,7 +17,6 @@ pub const LIBVLC_EVENTS_H: u32 = 1;
 pub const LIBVLC_DIALOG_H: u32 = 1;
 pub const LIBVLC_VLM_H: u32 = 1;
 pub const LIBVLC_DEPRECATED_H: u32 = 1;
-pub type __gnuc_va_list = __builtin_va_list;
 pub type __off_t = libc::c_long;
 pub type __off64_t = libc::c_long;
 pub type FILE = _IO_FILE;
@@ -120,13 +121,12 @@ const _: () = {
     ["Offset of field: _IO_FILE::_mode"][::core::mem::offset_of!(_IO_FILE, _mode) - 192usize];
     ["Offset of field: _IO_FILE::_unused2"][::core::mem::offset_of!(_IO_FILE, _unused2) - 196usize];
 };
-pub type va_list = __gnuc_va_list;
 unsafe extern "C" {
     pub fn vsnprintf(
         __s: *mut libc::c_char,
         __maxlen: libc::c_ulong,
         __format: *const libc::c_char,
-        __arg: *mut __va_list_tag,
+        __arg: VaList,
     ) -> libc::c_int;
 }
 #[repr(C)]
@@ -144,7 +144,7 @@ unsafe extern "C" {
 unsafe extern "C" {
     pub fn libvlc_vprinterr(
         fmt: *const libc::c_char,
-        ap: *mut __va_list_tag,
+        ap: VaList,
     ) -> *const libc::c_char;
 }
 unsafe extern "C" {
@@ -263,7 +263,7 @@ pub type libvlc_log_cb = ::core::option::Option<
         level: libc::c_int,
         ctx: *const libvlc_log_t,
         fmt: *const libc::c_char,
-        args: *mut __va_list_tag,
+        args: VaList,
     ),
 >;
 unsafe extern "C" {
@@ -3210,25 +3210,3 @@ unsafe extern "C" {
         ppsz_options: *mut *mut libc::c_char,
     );
 }
-pub type __builtin_va_list = [__va_list_tag; 1usize];
-#[repr(C)]
-#[derive(Debug, Copy, Clone)]
-pub struct __va_list_tag {
-    pub gp_offset: libc::c_uint,
-    pub fp_offset: libc::c_uint,
-    pub overflow_arg_area: *mut libc::c_void,
-    pub reg_save_area: *mut libc::c_void,
-}
-#[allow(clippy::unnecessary_operation, clippy::identity_op)]
-const _: () = {
-    ["Size of __va_list_tag"][::core::mem::size_of::<__va_list_tag>() - 24usize];
-    ["Alignment of __va_list_tag"][::core::mem::align_of::<__va_list_tag>() - 8usize];
-    ["Offset of field: __va_list_tag::gp_offset"]
-        [::core::mem::offset_of!(__va_list_tag, gp_offset) - 0usize];
-    ["Offset of field: __va_list_tag::fp_offset"]
-        [::core::mem::offset_of!(__va_list_tag, fp_offset) - 4usize];
-    ["Offset of field: __va_list_tag::overflow_arg_area"]
-        [::core::mem::offset_of!(__va_list_tag, overflow_arg_area) - 8usize];
-    ["Offset of field: __va_list_tag::reg_save_area"]
-        [::core::mem::offset_of!(__va_list_tag, reg_save_area) - 16usize];
-};

libvlc-sys/src/lib.rs

@@ -3,6 +3,8 @@
 #![allow(non_snake_case)]
 #![no_std]
 
+pub mod valist;
+
 // The bindings are a committed source file, regenerated out of band with
 // `cargo xtask bindgen`.
 include!("../bindings.rs");

libvlc-sys/src/valist.rs

@@ -0,0 +1,29 @@
+//! Platform `va_list`, extracted and simplified from `core::ffi::va_list`.
+//!
+//! libvlc-sys rust bindings are common to all libvlc's supported platforms and are bindgen
+//! generated once by the maintainers. Hence it needs a cross-platform va_list type.
+//!
+//! VaLists are platform specific, the C standard only specifies that they should be possible to
+//! pass by value. Libvlc uses them in the logger callback exclusively, until the rust stdlib C
+//! variadic API is stable, the type with the single goal of being forwarded to a C formatting
+//! function. Which simplifies greatly the implementation.
+//!
+//! Once the c_variadic API is stabilized, this file should be removed.
+
+/// [AArch64 Procedure Call Standard]:
+/// https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#id110
+#[cfg(all(target_arch = "aarch64", not(target_vendor = "apple")))]
+#[repr(C)]
+#[derive(Debug, Copy, Clone)]
+pub struct VaList {
+    stack: *mut libc::c_void,
+    gr_top: *mut libc::c_void,
+    vr_top: *mut libc::c_void,
+    gr_offs: i32,
+    vr_offs: i32,
+}
+
+/// Everywhere else: either an opaque pointer already, or a pointer to the struct the array decayed
+/// into. This is sound since VaLists are only passed around.
+#[cfg(not(all(target_arch = "aarch64", not(target_vendor = "apple"))))]
+pub type VaList = *mut libc::c_void;

src/core.rs

@@ -156,7 +156,7 @@ impl Drop for Instance {
 
 const BUF_SIZE: usize = 1024; // Write log message to the buffer by vsnprintf.
 unsafe extern "C" fn logging_cb(
-    data: *mut c_void, level: c_int, ctx: *const sys::libvlc_log_t, fmt: *const c_char, args: *mut sys::__va_list_tag) {
+    data: *mut c_void, level: c_int, ctx: *const sys::libvlc_log_t, fmt: *const c_char, args: sys::VaList) {
 
     let f: &Box<dyn Fn(LogLevel, Log, Cow<str>) + Send + 'static> = ::std::mem::transmute(data);
     let mut buf: [c_char; BUF_SIZE] = [0; BUF_SIZE];

xtask/src/main.rs

@@ -45,7 +45,11 @@ fn generate_bindings() {
         .allowlist_item("(lib|LIB)?(vlc|VLC)_.*")
         // Required by the Windows `legacy_stdio_definitions` link workaround
         // (see libvlc-sys/build.rs).
-        .allowlist_function("vsnprintf");
+        .allowlist_function("vsnprintf")
+        // Block the whole va_list family and rewrite the parameters bindings to our own `VaList`,
+        // which is ABI-correct on every target.
+        .blocklist_type(".*va_list.*")
+        .raw_line("pub use crate::valist::VaList;");
 
     for path in &library.include_paths {
         bindings = bindings.clang_arg(format!("-I{}", path.display()));
@@ -56,6 +60,14 @@ fn generate_bindings() {
         .expect("unable to generate bindings")
         .to_string();
 
+    let generated = generated
+        .replace("*mut __va_list_tag", "VaList")
+        .replace(": va_list", ": VaList");
+    assert!(
+        !generated.contains("va_list"),
+        "a va_list spelling we do not know about survived"
+    );
+
     std::fs::write(&output, generated).expect("couldn't write bindings");
     println!("wrote {}", output.display());
 }