Detect nested structs with differing layout or packing requirements (#…

…3522)
microsoft · Feb 27, 2025 · 4c707bc · 4c707bc
1 parent 1d0fbe0
commit 4c707bc
Show file tree

Hide file tree

Showing 10 changed files with 260 additions and 38 deletions.
diff --git a/crates/libs/bindgen/src/types/cpp_struct.rs b/crates/libs/bindgen/src/types/cpp_struct.rs
@@ -279,17 +279,45 @@ impl CppStruct {
     pub fn has_explicit_layout(&self) -> bool {
         self.def.flags().contains(TypeAttributes::ExplicitLayout)
             || self
-                .def
-                .fields()
-                .any(|field| field.ty(Some(self)).has_explicit_layout())
+                .multi_struct_fields()
+                .any(|ty| ty.has_explicit_layout())
     }
 
     pub fn has_packing(&self) -> bool {
-        self.def.class_layout().is_some()
-            || self
-                .def
-                .fields()
-                .any(|field| field.ty(Some(self)).has_packing())
+        self.def.class_layout().is_some() || self.multi_struct_fields().any(|ty| ty.has_packing())
+    }
+
+    // Returns all possible struct field types including arch-specific overloads.
+    // This avoids skipping arch-specific definitions of structs that may have
+    // different layout or packing requirements.
+    fn multi_struct_fields(&self) -> impl Iterator<Item = CppStruct> + '_ {
+        self.def
+            .fields()
+            .map(|field| field.ty(Some(self)))
+            .filter_map(|ty| match ty {
+                Type::CppStruct(ty) => Some(ty),
+                Type::ArrayFixed(ty, _) => {
+                    if let Type::CppStruct(ty) = *ty {
+                        Some(ty)
+                    } else {
+                        None
+                    }
+                }
+                _ => None,
+            })
+            .flat_map(|ty| {
+                ty.def
+                    .reader()
+                    .with_full_name(ty.def.namespace(), ty.def.name())
+            })
+            .filter_map(|ty| {
+                if let Type::CppStruct(ty) = ty {
+                    Some(ty)
+                } else {
+                    None
+                }
+            })
+            .chain(self.nested.values().cloned())
     }
 
     pub fn size(&self) -> usize {

diff --git a/crates/libs/bindgen/src/types/mod.rs b/crates/libs/bindgen/src/types/mod.rs
@@ -717,22 +717,6 @@ impl Type {
         matches!(self, Self::PtrConst(_, _) | Self::PtrMut(_, _))
     }
 
-    pub fn has_explicit_layout(&self) -> bool {
-        match self {
-            Self::CppStruct(ty) => ty.has_explicit_layout(),
-            Self::ArrayFixed(ty, _) => ty.has_explicit_layout(),
-            _ => false,
-        }
-    }
-
-    pub fn has_packing(&self) -> bool {
-        match self {
-            Self::CppStruct(ty) => ty.has_packing(),
-            Self::ArrayFixed(ty, _) => ty.has_packing(),
-            _ => false,
-        }
-    }
-
     pub fn is_byte_size(&self) -> bool {
         match self {
             Self::PtrConst(ty, _) | Self::PtrMut(ty, _) => ty.is_byte_size(),

diff --git a/crates/libs/windows/src/Windows/Win32/Devices/DeviceAndDriverInstallation/mod.rs b/crates/libs/windows/src/Windows/Win32/Devices/DeviceAndDriverInstallation/mod.rs
@@ -7492,7 +7492,7 @@ impl Default for SP_DETECTDEVICE_PARAMS {
 }
 #[repr(C)]
 #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec", target_arch = "x86_64"))]
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Copy)]
 pub struct SP_DETECTDEVICE_PARAMS {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub DetectProgressNotify: PDETECT_PROGRESS_NOTIFY,
@@ -7990,7 +7990,7 @@ pub struct SP_ENABLECLASS_PARAMS {
 }
 #[repr(C)]
 #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec", target_arch = "x86_64"))]
-#[derive(Clone, Copy, Debug, Default, PartialEq)]
+#[derive(Clone, Copy, Default)]
 pub struct SP_ENABLECLASS_PARAMS {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub ClassGuid: windows_core::GUID,
@@ -8265,7 +8265,7 @@ impl Default for SP_INSTALLWIZARD_DATA {
 #[repr(C)]
 #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec", target_arch = "x86_64"))]
 #[cfg(feature = "Win32_UI_Controls")]
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Copy)]
 pub struct SP_INSTALLWIZARD_DATA {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub Flags: u32,
@@ -8305,7 +8305,7 @@ impl Default for SP_NEWDEVICEWIZARD_DATA {
 #[repr(C)]
 #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec", target_arch = "x86_64"))]
 #[cfg(feature = "Win32_UI_Controls")]
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Copy)]
 pub struct SP_NEWDEVICEWIZARD_DATA {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub Flags: u32,
@@ -8377,7 +8377,7 @@ impl Default for SP_ORIGINAL_FILE_INFO_W {
     }
 }
 #[repr(C)]
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Copy)]
 pub struct SP_POWERMESSAGEWAKE_PARAMS_A {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub PowerMessageWake: [i8; 512],
@@ -8402,7 +8402,7 @@ impl Default for SP_POWERMESSAGEWAKE_PARAMS_W {
 }
 #[repr(C)]
 #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec", target_arch = "x86_64"))]
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Copy)]
 pub struct SP_POWERMESSAGEWAKE_PARAMS_W {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub PowerMessageWake: [u16; 512],
@@ -8424,7 +8424,7 @@ pub struct SP_PROPCHANGE_PARAMS {
 }
 #[repr(C)]
 #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec", target_arch = "x86_64"))]
-#[derive(Clone, Copy, Debug, Default, PartialEq)]
+#[derive(Clone, Copy, Default)]
 pub struct SP_PROPCHANGE_PARAMS {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub StateChange: SETUP_DI_STATE_CHANGE,
@@ -8507,14 +8507,14 @@ pub struct SP_REMOVEDEVICE_PARAMS {
 }
 #[repr(C)]
 #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec", target_arch = "x86_64"))]
-#[derive(Clone, Copy, Debug, Default, PartialEq)]
+#[derive(Clone, Copy, Default)]
 pub struct SP_REMOVEDEVICE_PARAMS {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub Scope: SETUP_DI_REMOVE_DEVICE_SCOPE,
     pub HwProfile: u32,
 }
 #[repr(C)]
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Copy)]
 pub struct SP_SELECTDEVICE_PARAMS_A {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub Title: [i8; 60],
@@ -8546,7 +8546,7 @@ impl Default for SP_SELECTDEVICE_PARAMS_W {
 }
 #[repr(C)]
 #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec", target_arch = "x86_64"))]
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Copy)]
 pub struct SP_SELECTDEVICE_PARAMS_W {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub Title: [u16; 60],
@@ -8561,7 +8561,7 @@ impl Default for SP_SELECTDEVICE_PARAMS_W {
     }
 }
 #[repr(C)]
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Copy)]
 pub struct SP_TROUBLESHOOTER_PARAMS_A {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub ChmFile: [i8; 260],
@@ -8588,7 +8588,7 @@ impl Default for SP_TROUBLESHOOTER_PARAMS_W {
 }
 #[repr(C)]
 #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec", target_arch = "x86_64"))]
-#[derive(Clone, Copy, Debug, PartialEq)]
+#[derive(Clone, Copy)]
 pub struct SP_TROUBLESHOOTER_PARAMS_W {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub ChmFile: [u16; 260],
@@ -8610,7 +8610,7 @@ pub struct SP_UNREMOVEDEVICE_PARAMS {
 }
 #[repr(C)]
 #[cfg(any(target_arch = "aarch64", target_arch = "arm64ec", target_arch = "x86_64"))]
-#[derive(Clone, Copy, Debug, Default, PartialEq)]
+#[derive(Clone, Copy, Default)]
 pub struct SP_UNREMOVEDEVICE_PARAMS {
     pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
     pub Scope: u32,

diff --git a/crates/libs/windows/src/Windows/Win32/System/Search/mod.rs b/crates/libs/windows/src/Windows/Win32/System/Search/mod.rs
@@ -21618,7 +21618,7 @@ impl Default for SSVARIANT_0 {
 }
 #[repr(C)]
 #[cfg(feature = "Win32_System_Com")]
-#[derive(Clone, Debug, Default, PartialEq)]
+#[derive(Default)]
 pub struct SSVARIANT_0_4 {
     pub dbobj: DBOBJECT,
     pub pUnk: core::mem::ManuallyDrop<Option<windows_core::IUnknown>>,

diff --git a/crates/tests/libs/bindgen/src/lib.rs b/crates/tests/libs/bindgen/src/lib.rs
@@ -83,6 +83,10 @@ pub mod reference_struct_sys_filter;
 pub mod reference_struct_sys_reference_namespace;
 pub mod reference_struct_sys_reference_type;
 pub mod sort;
+pub mod struct_arch_a;
+pub mod struct_arch_a_sys;
+pub mod struct_arch_w;
+pub mod struct_arch_w_sys;
 pub mod struct_cpp_sys;
 pub mod struct_cpp_win;
 pub mod struct_disambiguate;

diff --git a/crates/tests/libs/bindgen/src/struct_arch_a.rs b/crates/tests/libs/bindgen/src/struct_arch_a.rs
@@ -0,0 +1,40 @@
+#![allow(
+    non_snake_case,
+    non_upper_case_globals,
+    non_camel_case_types,
+    dead_code,
+    clippy::all
+)]
+
+#[repr(transparent)]
+#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
+pub struct DI_FUNCTION(pub u32);
+#[repr(C, packed(1))]
+#[cfg(target_arch = "x86")]
+#[derive(Clone, Copy, Default)]
+pub struct SP_CLASSINSTALL_HEADER {
+    pub cbSize: u32,
+    pub InstallFunction: DI_FUNCTION,
+}
+#[repr(C)]
+#[cfg(any(
+    target_arch = "aarch64",
+    target_arch = "arm64ec",
+    target_arch = "x86_64"
+))]
+#[derive(Clone, Copy, Debug, Default, PartialEq)]
+pub struct SP_CLASSINSTALL_HEADER {
+    pub cbSize: u32,
+    pub InstallFunction: DI_FUNCTION,
+}
+#[repr(C)]
+#[derive(Clone, Copy)]
+pub struct SP_POWERMESSAGEWAKE_PARAMS_A {
+    pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
+    pub PowerMessageWake: [i8; 512],
+}
+impl Default for SP_POWERMESSAGEWAKE_PARAMS_A {
+    fn default() -> Self {
+        unsafe { core::mem::zeroed() }
+    }
+}
diff --git a/crates/tests/libs/bindgen/src/struct_arch_a_sys.rs b/crates/tests/libs/bindgen/src/struct_arch_a_sys.rs
@@ -0,0 +1,38 @@
+#![allow(
+    non_snake_case,
+    non_upper_case_globals,
+    non_camel_case_types,
+    dead_code,
+    clippy::all
+)]
+
+pub type DI_FUNCTION = u32;
+#[repr(C, packed(1))]
+#[cfg(target_arch = "x86")]
+#[derive(Clone, Copy, Default)]
+pub struct SP_CLASSINSTALL_HEADER {
+    pub cbSize: u32,
+    pub InstallFunction: DI_FUNCTION,
+}
+#[repr(C)]
+#[cfg(any(
+    target_arch = "aarch64",
+    target_arch = "arm64ec",
+    target_arch = "x86_64"
+))]
+#[derive(Clone, Copy, Default)]
+pub struct SP_CLASSINSTALL_HEADER {
+    pub cbSize: u32,
+    pub InstallFunction: DI_FUNCTION,
+}
+#[repr(C)]
+#[derive(Clone, Copy)]
+pub struct SP_POWERMESSAGEWAKE_PARAMS_A {
+    pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
+    pub PowerMessageWake: [i8; 512],
+}
+impl Default for SP_POWERMESSAGEWAKE_PARAMS_A {
+    fn default() -> Self {
+        unsafe { core::mem::zeroed() }
+    }
+}
diff --git a/crates/tests/libs/bindgen/src/struct_arch_w.rs b/crates/tests/libs/bindgen/src/struct_arch_w.rs
@@ -0,0 +1,63 @@
+#![allow(
+    non_snake_case,
+    non_upper_case_globals,
+    non_camel_case_types,
+    dead_code,
+    clippy::all
+)]
+
+#[repr(transparent)]
+#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
+pub struct DI_FUNCTION(pub u32);
+#[repr(C, packed(1))]
+#[cfg(target_arch = "x86")]
+#[derive(Clone, Copy, Default)]
+pub struct SP_CLASSINSTALL_HEADER {
+    pub cbSize: u32,
+    pub InstallFunction: DI_FUNCTION,
+}
+#[repr(C)]
+#[cfg(any(
+    target_arch = "aarch64",
+    target_arch = "arm64ec",
+    target_arch = "x86_64"
+))]
+#[derive(Clone, Copy, Debug, Default, PartialEq)]
+pub struct SP_CLASSINSTALL_HEADER {
+    pub cbSize: u32,
+    pub InstallFunction: DI_FUNCTION,
+}
+#[repr(C, packed(1))]
+#[cfg(target_arch = "x86")]
+#[derive(Clone, Copy)]
+pub struct SP_POWERMESSAGEWAKE_PARAMS_W {
+    pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
+    pub PowerMessageWake: [u16; 512],
+}
+#[cfg(target_arch = "x86")]
+impl Default for SP_POWERMESSAGEWAKE_PARAMS_W {
+    fn default() -> Self {
+        unsafe { core::mem::zeroed() }
+    }
+}
+#[repr(C)]
+#[cfg(any(
+    target_arch = "aarch64",
+    target_arch = "arm64ec",
+    target_arch = "x86_64"
+))]
+#[derive(Clone, Copy)]
+pub struct SP_POWERMESSAGEWAKE_PARAMS_W {
+    pub ClassInstallHeader: SP_CLASSINSTALL_HEADER,
+    pub PowerMessageWake: [u16; 512],
+}
+#[cfg(any(
+    target_arch = "aarch64",
+    target_arch = "arm64ec",
+    target_arch = "x86_64"
+))]
+impl Default for SP_POWERMESSAGEWAKE_PARAMS_W {
+    fn default() -> Self {
+        unsafe { core::mem::zeroed() }
+    }
+}