std::sys::windows::process::Command::spawn: mem::size_of::<c::STARTUPINFOW> as c::DWORD compiles to a garbage value

[Fulgen301]

Code

use std::process::Command;

fn main() {
    Command::new("cmd")
        .args(["/C", "echo hello"])
        .spawn()
        .unwrap();
}

The Windows implementation of spawning a process creates a STARTUPINFOW structure and sets its cb member to the size of the structure:

rust/library/std/src/sys/windows/process.rs

Line 355 in 53a0397

si.cb = mem::size_of::<c::STARTUPINFOW> as c::DWORD;

On stable and beta, this works correctly and the size is set to 0x68. However, on nightly, the compiler compiles the size_of to a garbage value:

lea     rax, [rust_startupinfow_cb!core::mem::size_of<std::sys::windows::c::windows_sys::STARTUPINFOW> (7ff6d0436740)]
mov     dword ptr [rbp+580h], eax

Note that that value (0xd0436740 in the example above) changes with each rebuild of the program in question.
This can be verified by setting a breakpoint on the call to CreateProcessW or one of the functions that it calls and examining the STARTUPINFOW structure on the stack (e.g. setting a breakpoint on KERNELBASE!CreateProcessInternalW and reinterpreting r11 + 0x48 as STARTUPINFOW **).

Version it worked on

It most recently worked on: rustc 1.73.0-beta.4 (9f37cd4 2023-09-01)

Version with regression

rustc --version --verbose:

rustc 1.74.0-nightly (9f5fc1bd4 2023-09-02)
binary: rustc
commit-hash: 9f5fc1bd443f59583e7af0d94d289f95fe1e20c4
commit-date: 2023-09-02
host: x86_64-pc-windows-msvc
release: 1.74.0-nightly
LLVM version: 17.0.0

@rustbot modify labels: +regression-from-beta-to-nightly -regression-untriaged

[ChrisDenton]

rust/library/std/src/sys/windows/process.rs

Line 355 in 53a0397

si.cb = mem::size_of::<c::STARTUPINFOW> as c::DWORD;

Ah, sorry I missed the lack of () in review. Though it is a bit concerning that our tests didn't catch it either. If someone wants to make a quick PR to fix it, I'd appreciate it.

[ChrisDenton]

Also cc @michaelvanstraten

[Fulgen301]

Heh, missed that too, explains the lea... I'll make a PR.

(How is casting a function pointer, which is 64bit, to a DWORD, which is 32bit, safe Rust though?)

[ChrisDenton]

Yeah as is bad. I mean, casting is completely safe in Rust... but using the result of the cast isn't necessarily.

[michaelvanstraten]

Can't believe this compiled, didn't know that you could cast a function to an i32 in safe rust.

Really sorry for this <3

[ChrisDenton]

It's ok. It's subtle and we both missed it. An as cast is way too powerful for its own good.

[michaelvanstraten]

Maybe we should make a clippy lint for this.

[michaelvanstraten]

@Fulgen301 Just out of interest, how did you discover this?

[saethlin]

Maybe we should make a clippy lint for this.

At least one clippy lint already exists, and it warns by default https://rust-lang.github.io/rust-clippy/master/index.html#/fn_to_numeric_cast_with_truncation

Though the lint I would prefer here is warning against ever taking the address of a generic function.

[Fulgen301]

@michaelvanstraten I was actually trying to get some handle information about the handles cargo test passes to the process it spawns since I wanted to figure out why it didn't print any output when the process being tested is a GUI process (I was assuming it was trying to pass console driver handles to the process, which wouldn't work since the child process wouldn't have a console attached), and my Rust knowledge was still weak enough that looking at the handles in a debugger was easier than to dig through three files of defaults and fallbacks, so I set a breakpoint, looked at the STARTUPINFOW and discovered that the cb value was way too big.

[Fulgen301]

Yeah as is bad. I mean, casting is completely safe in Rust... but using the result of the cast isn't necessarily.

It's a cast that doesn't make any sense though - on a 64bit platform, a Rust function pointer can never fit into 32 bits, so requiring it to go through an intermediary, e.g. usize, could help prevent such accidents.

[saethlin]

as supports numerous truncating conversions, which is indeed one of the problems with it.