Bug: struct layout problems: Some C shorts are u32 structs, and structs are not packed correctly

[zanedp]

Which crate is this about?

windows

Crate version

0.43.0

Summary

While attempting to use the EnumDisplaySettingsExW() function to find info about my displays, I discovered that the Rust DEVMODEW.dmPelsHeight field is 8 bytes away from where it should be. It looks like there are two causes for this:

1. Some fields that are short in C++, are implemented as u32 structs in Rust ("newtype" idiom). They should be i16's.
2. u32 fields in the Rust struct are 4-byte aligned. They should not be 4-byte aligned if they are not 4-byte aligned in C++, and should be packed instead.

Below is the DEVMODEW Rust struct with comments indicating the lines where I've traced down problems.

#[repr(C)]
#[doc = "*Required features: `\"Win32_Graphics_Gdi\"`, `\"Win32_Foundation\"`*"]
#[cfg(feature = "Win32_Foundation")]
pub struct DEVMODEW {
    pub dmDeviceName: [u16; 32],
    pub dmSpecVersion: u16,
    pub dmDriverVersion: u16,
    pub dmSize: u16,
    pub dmDriverExtra: u16,
    pub dmFields: DEVMODE_FIELD_FLAGS,
    pub Anonymous1: DEVMODEW_0,
    pub dmColor: DEVMODE_COLOR, // Problem 1: dmColor is a short in wingdi.h, but DEVMODE_COLOR is `struct DEVMODE_COLOR(u32)`
    pub dmDuplex: i16,
    pub dmYResolution: i16,
    pub dmTTOption: DEVMODE_TRUETYPE_OPTION, // Problem 1: dmTTOption is a short in wingdi.h, but DEVMODE_TRUETYPE_OPTION is u32
    pub dmCollate: DEVMODE_COLLATE,  // Problem 1: dmCollate is a short in wingdi.h, but DEVMODE_COLLATE is u32
    pub dmFormName: [u16; 32],
    pub dmLogPixels: u16,
    pub dmBitsPerPel: u32, // Problem 2: Should start 2 bytes after dmLogPixels, but starts 4 bytes later because of alignment = 4. DEVMODEW is not packed.
    pub dmPelsWidth: u32,
    pub dmPelsHeight: u32,
    pub Anonymous2: DEVMODEW_1,
    pub dmDisplayFrequency: u32,
    pub dmICMMethod: u32,
    pub dmICMIntent: u32,
    pub dmMediaType: u32,
    pub dmDitherType: u32,
    pub dmReserved1: u32,
    pub dmReserved2: u32,
    pub dmPanningWidth: u32,
    pub dmPanningHeight: u32,
}

This is likely caused by two separate bugs, but I'm going to log them both into one issue for now, since I found them both in the same struct.

Toolchain version/configuration

rustup show
Default host: x86_64-pc-windows-msvc
rustup home: C:\Users\me.rustup

stable-x86_64-pc-windows-msvc (default)
rustc 1.66.0 (69f9c33d7 2022-12-12)

Reproducible example

// C++
//     #include <wingdi.h>
//     ...
//     auto offset_of_dmColor = offsetof(DEVMODEW, dmColor); // 92
//     auto offset_of_dmDuplex = offsetof(DEVMODEW, dmDuplex); // 94
//     auto offset_of_dmTTOption = offsetof(DEVMODEW, dmTTOption); // 98
//     auto offset_of_dmCollate = offsetof(DEVMODEW, dmCollate); // 100
//     auto offset_of_dmFormName = offsetof(DEVMODEW, dmFormName); // 102
//     auto offset_of_dmLogPixels = offsetof(DEVMODEW, dmLogPixels); // 166
//     auto offset_of_dmBitsPerPel = offsetof(DEVMODEW, dmBitsPerPel); // 168
//     auto offset_of_dmPelsWidth = offsetof(DEVMODEW, dmPelsWidth); // 172

use memoffset::offset_of;
use windows::Win32::Graphics::Gdi::*;

fn main() {
    // things are OK at this field
    println!("offset of dmColor: {}", offset_of!(DEVMODEW, dmColor));

    // since dmColor was a short in C++, this should be at offset_of(dmColor) + 2,
    // but it's at offset_of(dmColor) + 4, since dmColor is a struct(u32) in rust.
    println!("offset of dmDuplex: {}", offset_of!(DEVMODEW, dmDuplex));

    println!("offset of dmTTOption: {}", offset_of!(DEVMODEW, dmTTOption));

    // should be +2 from dmTTOption but is +4 because dmTTOption is struct(u32)
    println!("offset of dmCollate: {}", offset_of!(DEVMODEW, dmCollate));

    // should be +2 from dmCollate but is +4 because dmCollate is struct(u32)
    println!("offset of dmFormName: {}", offset_of!(DEVMODEW, dmFormName));

    println!(
        "offset of dmLogPixels: {}",
        offset_of!(DEVMODEW, dmLogPixels)
    );

    // This is at offset_of(dmLogPixels) + 4, rather than + 2 as it should be.
    // dmLogPixels is a u16. dmBitsPerPel should not be 4-byte aligned despite being a u32.
    println!(
        "offset of dmBitsPerPel: {}",
        offset_of!(DEVMODEW, dmBitsPerPel)
    );

    println!(
        "offset of dmPelsWidth: {}",
        offset_of!(DEVMODEW, dmPelsWidth)
    );

    // ----- let's do the same checks as above, but now with assertions -----
    // Compare C++ offsets with Rust offsets
    assert_eq!(92, offset_of!(DEVMODEW, dmColor));
    assert_eq!(94, offset_of!(DEVMODEW, dmDuplex));
    assert_eq!(98, offset_of!(DEVMODEW, dmTTOption));
    assert_eq!(100, offset_of!(DEVMODEW, dmCollate));
    assert_eq!(102, offset_of!(DEVMODEW, dmFormName));
    assert_eq!(166, offset_of!(DEVMODEW, dmLogPixels));
    assert_eq!(168, offset_of!(DEVMODEW, dmBitsPerPel));
    assert_eq!(172, offset_of!(DEVMODEW, dmPelsWidth));
}

Crate manifest

[package]
name = "struct-layout-issues"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
memoffset = "0.8.0"

[dependencies.windows]
version = "0.43.0"
features = [
    "Win32_Graphics_Gdi",
    "Win32_Foundation",
    "Win32_UI_WindowsAndMessaging"
]

Expected behavior

In my original program, I expected to be able to read DEVMODEW.dmPelsWidth and get the width of my monitor. (I instead get refresh rate.)

In the program I pasted above, I expect the offsets in Rust to be the same as the offsets seen in C++, and expect see this output:

offset of dmColor: 92
offset of dmDuplex: 94
offset of dmTTOption: 98
offset of dmCollate: 100
offset of dmFormName: 102
offset of dmLogPixels: 166
offset of dmBitsPerPel: 168
offset of dmPelsWidth: 172

Actual behavior

The field offsets beyond dmColor are incorrect:

offset of dmColor: 92
offset of dmDuplex: 96
offset of dmTTOption: 100
offset of dmCollate: 104
offset of dmFormName: 108
offset of dmLogPixels: 172
offset of dmBitsPerPel: 176
offset of dmPelsWidth: 180
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `94`,
 right: `96`', src\main.rs:52:5

Additional comments

No response

[riverar]

Related

• DEVMODE structure has fields with incorrect types win32metadata#1325
• Adjust DEVMODE structure fields win32metadata#1329

Just leaving this note here, haven't looked at this yet.

[kennykerr]

The field size issues should have been fixed - you can confirm by targeting the repo rather than the crate. The alignment issues seem to be related to microsoft/win32metadata#1044 where the packing metadata is missing, but in this case alignment seems to match MSVC so perhaps what you're seeing is just a side effect of the incorrect struct field sizes in the version you're testing.

[zanedp]

Thanks! I can confirm that by referencing the repo itself (with git = "https://github.com/microsoft/windows-rs" rather than version = "0.43.0"), my test case passes:

// Compare C++ offsets with Rust offsets
assert_eq!(92, offset_of!(DEVMODEW, dmColor));
assert_eq!(94, offset_of!(DEVMODEW, dmDuplex));
assert_eq!(98, offset_of!(DEVMODEW, dmTTOption));
assert_eq!(100, offset_of!(DEVMODEW, dmCollate));
assert_eq!(102, offset_of!(DEVMODEW, dmFormName));
assert_eq!(166, offset_of!(DEVMODEW, dmLogPixels));
assert_eq!(168, offset_of!(DEVMODEW, dmBitsPerPel));
assert_eq!(172, offset_of!(DEVMODEW, dmPelsWidth));

[kennykerr]

Sweet - thanks for checking!