Stop using `mem::zeroed` for FFI

[joboet]

Quite a lot of stds FFI code uses mem::zeroed to create empty structures that are to be filled by FFI. E.g.:

rust/library/std/src/sys/pal/windows/time.rs

Lines 68 to 72 in d2f335d

	unsafe {
	let mut t: SystemTime = mem::zeroed();
	c::GetSystemTimePreciseAsFileTime(&mut t.t);
	t
	}

This is unnecessary, since the C code does not require the structures to be initialized (you wouldn't zero out structures in C either). Thus, this pattern just reduces performance, as it results in the initialization of potentially very large structures such as sockaddr_storage. We should get rid of this pattern and replace it with proper handling of uninitialized data through MaybeUninit.

Edit (after discussion below): In some instances one might decide to keep the zero-initialization behaviour, but I think this should still go through MaybeUninit::zeroed instead of mem::zeroed to make the point of initialization explicit (by introducing .assume_init() calls in the right places.

I'll probably do the network code myself (I want to clean some things up there anyway), but I'm happy to mentor you if you'd like to help with other instances such as the filesystem code (library/std/src/sys/pal/*/fs.rs). Just contact me here or on Zulip. The best way to find the pattern is probably by searching for mem::zeroed in library/std/src/sys.

[ChrisDenton]

I don't think GetSystemTimePreciseAsFileTime is the best example. FILETIME is a couple of u32s (aka a 4-byte aligned u64). You wouldn't usually bother with MaybeUninit for a u64.

But for bigger structs I do agree.

[the8472]

Afaik in the linux kernel zero-initialization for structs is the default as a security measure. And some APIs explicitly require it, e.g. cmsghdr.

[joboet]

I don't think GetSystemTimePreciseAsFileTime is the best example. FILETIME is a couple of u32s (aka a 4-byte aligned u64). You wouldn't usually bother with MaybeUninit for a u64.

But for bigger structs I do agree.

Fair point. I just wanted a very clear example of the pattern.

Edit: Also, I think using MaybeUninit even here is very much justified as it explicitly annotates when the structure has been filled with useful data.

[joboet]

Afaik in the linux kernel zero-initialization for structs is the default as a security measure.

I don't think this is necessary in our case. For one, there is an FFI and linkage unit boundary in our case, meaning the optimizer can't rely introduce any optimizations that rely on the initialization state of the structure. Also, I guess Linux probably does this to make sure that sensitive data is not accidentally leaked when copying out kernel memory to userspace. This isn't relevant for us as we assume the environment to be trusted.

And some APIs explicitly require it, e.g. cmsghdr.

Obviously we should evaluate the correctness individually for each instance of the pattern. But for e.g. pthread_t, I highly doubt that anyone in C-land would zero-initialize it.

[Noratrieb]

I think it makes sense to do for potentially big structs, but I think it should be carefully evaluated on a case-by-case basis, as using MaybeUninit makes the code more complicated.

[hanna-kruppe]

For one, there is an FFI and linkage unit boundary in our case, meaning the optimizer can't rely introduce any optimizations that rely on the initialization state of the structure.

This kind of argument worries me. C programmers have been proven wrong about it many times as LTO has become more commonplace. It’s generally true today with libcs written in C and compiled to machine code before Rust even enters the picture, but this can and quite possibly should change. There’s cross-language LTO, LLVM libc has the explicit goal of supporting LTO of libc code + application code, and there are even (highly experimental) projects implementing libc interfaces in Rust.

[the8472]

And the wobblyness of an uninit struct backed by MADV_FREE'd memory would leak through an FFI barrier. I.e. *foo == *foo wouldn't hold. Of course the C code shouldn't do that without initializing the data in the first place and the UB would be clearly C's fault, but it's additional evidence that the optimization barrier argument is insufficient.

[joboet]

I think it makes sense to do for potentially big structs, but I think it should be carefully evaluated on a case-by-case basis, as using MaybeUninit makes the code more complicated.

I'd argue that the additional complexity is a good thing: Having to write .assume_init() makes you think twice about whether you've actually filled in the data. So even if we want to keep zero-initialization in some cases, I'd argue it still should be done with MaybeUninit::zeroed instead of mem::zeroed.

Of course the C code shouldn't do that without initializing the data in the first place and the UB would be clearly C's fault

Exactly. The question is how much we trust the C code to be correct. I'd argue that in nearly all of the cases inside std, misbehaving C code is both highly unlikely (because these are APIs used by everyone, e.g. GetSystemInfo on Windows) and would be unsound anyway (e.g. in the case of GetSystemInfo because the result is used with NonZero::new_unchecked, so if the structure is not filled, even the zero-initialization would lead to undefined behaviour).