zig
zig copied to clipboard
change uefi packed structs to new integer backed syntax
-
device_path_protocol now uses extern structs with align(1) fields because the transition to integer backed packed struct broke alignment
-
added comptime asserts that device_path_protocol structs do not violate alignment and size specifications
Thanks for the help reviewing, @ominitay.
The CI failure will be fixed by running zig fmt:
Looking for non-conforming code formatting...
../lib/std/os/uefi/protocols/device_path_protocol.zig
Whoops, looks like I forgot the newline at the end of the file :sweat:
Might be worth adding std.testing.refAllDeclsRecursive(@This()) to uefi.zig to ensure that all the packed struct stuff gets checked and stays fixed fwiw
The issue with running tests on them is that there is no testing infrastructure for uefi, there is simply no way to run them without doing something weird with the test runner. And you can't safely run the tests on another platform because of potential differences in the abi
Ah ok. I assume that would change extern struct stuff then? Because refAllDecls would just be to ensure it stays compiling, not much else afaik.
It would appear that the change making packed structs backed by integers also made them no longer align(1). So I guess I also need to go through and align(1) all the fields manually.
Tested this with my bootloader and it works again after changing those 2 alignCasts! :tada:
I've cleaned up the commit history, if people can verify that it also works for them it seems like this can be merged.
Can confirm it still works, lgtm
I finally found the important words for DevicePathProtocol.
Therefore all nodes are byte-packed data structures that may appear on any byte boundary. All code references to device path notes must assume all fields are unaligned.
Therefore, I've gone ahead and made all strings in device_path_protocol align(1) and removed all alignCasts. I also added a note so that this doesn't get confusing in the future.
Is anything else required to merge this PR?
I think everything that broke has been fixed, lgtm
Maybe rebase against master to pass the CI?
As far as I can tell, the compiler is failing on the stdlib tests. From what I can tell this is because std.os.uefi.Status is a enum(usize) and most of its fields have the high bit set.
thread 93367 panic: integer cast truncated bits
/workspace/src/value.zig:1181:58: 0x66eda97 in toSignedInt (zig)
.int_u64 => return @intCast(i64, self.castTag(.int_u64).?.data),
^
codegen/llvm.zig:1508:64: 0x6706bdf in lowerDebugTypeImpl (zig)
codegen/llvm.zig:1444:36: 0x656715e in lowerDebugType (zig)
The code in lowerDebugTypeImpl points to https://github.com/ziglang/zig/issues/645.
It would seem that zig cannot generate debug info for these types, however I think the blind intCast to i64 is the wrong idea here, since it makes this enum impossible.
I believe that this error has never come up before since std.os.uefi has never been included in the test suite.
Note that you can easily check locally whether the CI will fail by running this command:
stage3/bin/zig test ../lib/std/std.zig -target x86_64-linux