std.mem.zeroes: Zero out entire extern union, including padding

[jayschwa]

Fixes #17258

[jayschwa]

@kcbanner, I'm running into a problem with walkStackWindows after these changes. std.mem.zeroes is being called on a structure which contains non-nullable pointers, resulting in the error: "Only nullable and allowzero pointers can be set to zero."

zig/lib/std/debug.zig

Line 767 in ab3ac1e

var history_table: windows.UNWIND_HISTORY_TABLE = std.mem.zeroes(windows.UNWIND_HISTORY_TABLE);

zig/lib/std/os/windows.zig

Lines 4105 to 4108 in ab3ac1e

	pub const UNWIND_HISTORY_TABLE_ENTRY = extern struct {
	ImageBase: ULONG64,
	FunctionEntry: *Self.RUNTIME_FUNCTION,
	};

I think there are two ways this could be fixed:

1. Change the FunctionEntry field type to ?*Self.RUNTIME_FUNCTION. This is equivalent to what @cImport would do if the structure were being translated from the windows C header.
2. history_table appears to only be an output parameter of RtlLookupFunctionEntry, so it could be initialized with undefined instead of std.mem.zeroes.

Which way do you think is better?

[jayschwa]

It looks like history_table isn't even read, so I will start testing with it initialized to undefined. Still open to suggestions though.

[kcbanner]

I think option 1 (make the field an optional pointer) makes sense, that was an oversight on my part when I added it.

[jayschwa]

I think option 1 (make the field an optional pointer) makes sense, that was an oversight on my part when I added it.

This is the change that ended up working. Initializing history_table to undefined caused problems. Maybe it's actually an in-out parameter, even though the Windows docs don't say that.

[andrewrk]

For both extern structs and extern unions, it should be:

var item: T = undefined;
@memset(asBytes(&item), 0);
return item;

If I understand correctly, the motivation for making these changes here is that the compiler is handling this perfectly correct code (for structs; extern unions should be changed to this too) in a problematic way.

I suggest, instead of working around this in the standard library, to work around it in the implementation of @memset in the compiler.

[kcbanner]

I suggest, instead of working around this in the standard library, to work around it in the implementation of @memset in the compiler.

I'm working on this problem here: https://github.com/ziglang/zig/compare/master...kcbanner:zig:extern_union_comptime_memory?expand=1

I'm tracking down one last case with writing to fields of packed unions overwriting too many bits, then I'll PR it.

[jayschwa]

I chose to recursively initialize the fields for two reasons:

1. It provides additional guard rails to ensure zero values make sense for the fields being initialized. For example, it caught the non-nullable pointers in windows.UNWIND_HISTORY_TABLE.
2. It ensures pointers are initialized to "null" instead of "0", which I acknowledge is pedantic. Situations where "null" is not "0" under the hood are probably vanishingly small, but C does leave that door open. I'm not sure if Zig will ever support that scenario, but if it does, this bit of code won't need to change.

If you still think plain @memset is the way to go, I will change it.

[andrewrk]

It provides additional guard rails to ensure zero values make sense for the fields being initialized. For example, it caught the non-nullable pointers in windows.UNWIND_HISTORY_TABLE.

In Zig, extern structs have well-defined memory layout, which means that they can be used as a bag of bytes that has anything there. It would be illegal behavior if zig code dereferenced one of the non-optional pointers from windows.UNWIND_HISTORY_TABLE which had been set to zero in this manner, however, it would have been perfectly legal to send such a struct instance over the C ABI boundary, or even to another Zig function which pointer-casted the struct to some other type and began operating on it.

Therefore I think the meaning of std.mem.zeroes for extern structs and unions should be defined in terms of setting the entire memory region to 0 bytes.

It ensures pointers are initialized to "null" instead of "0", which I acknowledge is pedantic. Situations where "null" is not "0" under the hood are probably vanishingly small, but C does leave that door open.

Zig does not define optional pointers in terms of the C programming language. Zig defines the null value of *T to be address 0. https://ziglang.org/documentation/0.11.0/#Optional-Pointers

There is no door open; the Zig language stands on its own and does not depend on the C language specification.

[jayschwa]

Okay, I'll change it to plain @memset.

@kcbanner, should I keep the Windows structure change?

Update: I decided to drop that other commit since the Windows structure change will no longer be necessary.