Fix structs that have packed alignment

`header-translator` did not correctly catch `#pragma pack(push, ...)`
since libclang doesn't easily expose that.

We now calculate the natural alignment of the struct, and compare it to
the actual alignment, so alignment attributes should now be (at least
more) correct.

Fixes #758.

Author: madsmtm

crates/header-translator/src/stmt.rs

@@ -1,4 +1,5 @@
 use std::borrow::Cow;
+use std::cmp::Ordering;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::collections::HashMap;
@@ -551,7 +552,8 @@ pub enum Stmt {
         boxable: bool,
         fields: Vec<(String, Documentation, Ty)>,
         sendable: Option<bool>,
-        packed: bool,
+        align: usize,
+        natural_align: usize,
         documentation: Documentation,
         is_union: bool,
     },
@@ -1294,6 +1296,8 @@ impl Stmt {
                 let mut fields = Vec::new();
                 let mut sendable = None;
                 let mut packed = false;
+                let align = ty.get_alignof().expect("alignment of record type");
+                let mut natural_align = 0;
 
                 let mut res = vec![];
 
@@ -1313,6 +1317,17 @@ impl Stmt {
                         let _span = debug_span!("field", name).entered();
 
                         let ty = entity.get_type().expect("struct/union field type");
+                        let field_align = ty.get_alignof().unwrap();
+                        if align < field_align {
+                            // Similar to what bindgen does, we cannot detect
+                            // #pragma pack, but we can detect it's effect
+                            // (that the type itself has lower alignment than
+                            // its fields).
+                            packed = true;
+                        }
+                        // Compute the natural alignment of the struct.
+                        natural_align = natural_align.max(field_align);
+
                         let ty = Ty::parse_record_field(ty, context);
 
                         if entity.is_bit_field() {
@@ -1330,7 +1345,9 @@ impl Stmt {
                         // emit them at the top-level.
                         res.extend(Self::parse(&entity, context, current_library));
                     }
-                    EntityKind::PackedAttr => packed = true,
+                    EntityKind::PackedAttr => {
+                        assert_eq!(align, 1, "packed records must have an alignment of 1");
+                    }
                     EntityKind::VisibilityAttr => {}
                     _ => error!(?entity, "unknown struct/union child"),
                 });
@@ -1347,7 +1364,8 @@ impl Stmt {
                     boxable,
                     fields,
                     sendable,
-                    packed,
+                    align,
+                    natural_align,
                     documentation,
                     is_union,
                 });
@@ -2517,18 +2535,23 @@ impl Stmt {
                     boxable: _,
                     fields,
                     sendable,
-                    packed,
+                    align,
+                    natural_align,
                     documentation,
                     is_union,
                 } => {
                     write!(f, "{}", documentation.fmt(Some(id)))?;
                     write!(f, "{}", self.cfg_gate_ln(config))?;
                     write!(f, "{availability}")?;
-                    if *packed {
-                        writeln!(f, "#[repr(C, packed)]")?;
-                    } else {
-                        writeln!(f, "#[repr(C)]")?;
+
+                    // See <https://doc.rust-lang.org/reference/type-layout.html#the-alignment-modifiers>
+                    match align.cmp(natural_align) {
+                        Ordering::Less if *align == 1 => writeln!(f, "#[repr(C, packed)]")?,
+                        Ordering::Less => writeln!(f, "#[repr(C, packed({align}))]")?,
+                        Ordering::Equal => writeln!(f, "#[repr(C)]")?,
+                        Ordering::Greater => writeln!(f, "#[repr(C, align({align}))]")?,
                     }
+
                     if *is_union || fields.iter().any(|(_, _, field)| field.contains_union()) {
                         writeln!(f, "#[derive(Clone, Copy)]")?;
                     } else {

crates/objc2/src/topics/about_generated/CHANGELOG.md

@@ -27,6 +27,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   takes a nullable file name.
 * **BREAKING**: The media selection option on `AVAssetVariantQualifier` is now nullable.
 
+### Fixed
+* **BREAKING**: Fix structs with packed alignment by marking them `#[repr(packed(...))]`.
+
 ## [0.3.1] - 2025-04-19
 [0.3.1]: https://github.com/madsmtm/objc2/compare/frameworks-0.3.0...frameworks-0.3.1
 

framework-crates/objc2-core-media/tests/block_buffer_alignment.rs

@@ -0,0 +1,51 @@
+//! Regression test for <https://github.com/madsmtm/objc2/issues/758>.
+#![cfg(feature = "CMBlockBuffer")]
+use objc2_core_foundation::{kCFAllocatorDefault, kCFAllocatorNull, CFRetained};
+use objc2_core_media::{
+    kCMBlockBufferCustomBlockSourceVersion, kCMBlockBufferNoErr, CMBlockBuffer,
+    CMBlockBufferCustomBlockSource,
+};
+use std::{ffi::c_void, ptr::NonNull};
+
+unsafe extern "C-unwind" fn free_block(
+    ref_context: *mut c_void,
+    block: NonNull<c_void>,
+    size: usize,
+) {
+    let data = unsafe { Vec::from_raw_parts(block.as_ptr().cast::<u8>(), 2, 2) };
+    println!("Freeing block of size: {size}, {data:?}");
+    let ref_value = unsafe { *(ref_context.cast::<i32>()) };
+    println!("Getting RefCon: {ref_value}");
+}
+
+#[test]
+fn main() {
+    let data_size = 2;
+    let data = vec![b'A'; data_size];
+    let data = data.leak(); // we free this at `free_block`
+
+    let mut ref_data = 42;
+    let mut source: CMBlockBufferCustomBlockSource = unsafe { std::mem::zeroed() };
+    source.version = kCMBlockBufferCustomBlockSourceVersion;
+    source.FreeBlock = Some(free_block);
+    let ref_data: *mut i32 = &mut ref_data;
+    source.refCon = ref_data.cast::<c_void>();
+
+    let mut block_buffer: *mut CMBlockBuffer = std::ptr::null_mut();
+    let status = unsafe {
+        CMBlockBuffer::create_with_memory_block(
+            kCFAllocatorDefault,
+            data.as_mut_ptr().cast(),
+            data_size,
+            kCFAllocatorNull,
+            &source,
+            0,
+            data_size,
+            0,
+            NonNull::from(&mut block_buffer),
+        )
+    };
+    assert_eq!(status, kCMBlockBufferNoErr); // fail to create block buffer
+    let block_buffer = unsafe { CFRetained::from_raw(NonNull::new(block_buffer).unwrap()) };
+    assert_eq!(block_buffer.retain_count(), 1);
+}

framework-crates/objc2-core-services/src/files.rs

@@ -7,7 +7,8 @@ use crate::UniChar;
 // act as-if CoreServices is the one to define this type.
 //
 // NOTE: This is marked __attribute__((aligned(2), packed)), but that's
-// unnecessary, since the layout of the .
+// unnecessary, since the alignment of u16/UniChar, and thereby the struct
+// itself, is already 2.
 #[repr(C)]
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub struct HFSUniStr255 {
@@ -27,3 +28,13 @@ unsafe impl Encode for HFSUniStr255 {
 unsafe impl RefEncode for HFSUniStr255 {
     const ENCODING_REF: Encoding = Encoding::Pointer(&Self::ENCODING);
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn alignment() {
+        assert_eq!(core::mem::align_of::<HFSUniStr255>(), 2);
+    }
+}

framework-crates/objc2-io-kit/tests/alignment.rs

@@ -0,0 +1,7 @@
+#![cfg(all(feature = "usb", feature = "AppleUSBDefinitions"))]
+
+#[test]
+fn alignment() {
+    assert_eq!(std::mem::align_of::<objc2_io_kit::IOUSBHIDReportDesc>(), 1);
+    assert_eq!(std::mem::size_of::<objc2_io_kit::IOUSBHIDReportDesc>(), 3);
+}