Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Packed structs can't contain arrays #12547

Open
matrizzo opened this issue Aug 21, 2022 · 12 comments
Open

Packed structs can't contain arrays #12547

matrizzo opened this issue Aug 21, 2022 · 12 comments
Labels
error message This issue points out an error message that is unhelpful and should be improved. frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Milestone

Comments

@matrizzo
Copy link

matrizzo commented Aug 21, 2022

Zig Version

0.10.0-dev.3659+e5e6eb983

Steps to Reproduce

matteo@lab:/tmp$ zig version
0.10.0-dev.3659+e5e6eb983
matteo@lab:/tmp$ cat test.zig
const std = @import("std");

const TestStruct = packed struct {
    pad: [512]u8,
    x: u64,
};

pub fn main() void {
    const asdf = TestStruct{.pad = [_]u8{0} ** 512, .x = 1};
    std.debug.print("{d}", .{asdf.x});
}
matteo@lab:/tmp$ zig build-exe test.zig
test.zig:4:5: error: packed structs cannot contain fields of type '[512]u8'
    pad: [512]u8,
    ^~~~~~~~~~~~
test.zig:4:5: note: type has no guaranteed in-memory representation
matteo@lab:/tmp$ zig build-exe -fstage1 test.zig
matteo@lab:/tmp$

Expected Behavior

The example compiles without errors with both -fstage1 and with the default settings.

Actual Behavior

The compiler complains that [512]u8 can't be in a packed struct because "it has no guaranteed in-memory representation". This doesn't really make sense to me, isn't an u8 array supposed to be just a fixed-size sequence of bytes?

When using the -fstage1 the compiler doesn't complain.

@matrizzo matrizzo added the bug Observed behavior contradicts documented or intended behavior label Aug 21, 2022
@Vexu
Copy link
Member

Vexu commented Aug 21, 2022

Working as intended, see #10113 (comment) and the rest of the thread for reasoning.

The error should explain it better though.

@Vexu Vexu added error message This issue points out an error message that is unhelpful and should be improved. frontend Tokenization, parsing, AstGen, Sema, and Liveness. and removed bug Observed behavior contradicts documented or intended behavior labels Aug 21, 2022
@Vexu Vexu added this to the 0.11.0 milestone Aug 21, 2022
@ominitay
Copy link
Contributor

The documentation regarding packed vs extern structs is poor in my opinion. I'll get around to improving it at some point, since it does appear to mislead people a lot.

@matrizzo
Copy link
Author

Is there any way to have something that works like a packed struct (i.e., guaranteed memory layout, no padding, can be used to reinterpret memory, ...) and can contain arrays? This seems like it should be a fairly common use case when decoding some binary data.

Also, I'd argue that this is still a bug since the stage1 and stage2 compilers produce different results :)

@ominitay
Copy link
Contributor

It's a bug in stage1, which won't be fixed, at least any time soon. Extern structs have a known memory layout and should be used in most cases, but where bit-packing is required, there is only the choice of packed structs, which can't contain arrays. There are a few discussions going on about solutions to this, but not sure how far they'll get.

@matrizzo
Copy link
Author

Ok, thanks. Then isn't this section of the documentation about extern structs incorrect? It contradicts your assertion that "Extern structs should be used in most cases" https://github.com/ziglang/zig/blob/master/doc/langref.html.in#L3272

This kind of struct should only be used for compatibility with the C ABI. Every other use case should be solved with packed struct or normal struct.

@ominitay
Copy link
Contributor

Yes, I believe it is.

@matrizzo
Copy link
Author

So just to understand this correctly... At the moment there is no way at all to declare a structure with this layout in Zig

struct asdf {
    uint32_t a;
    uint64_t b[4];
};

that has no padding between a and b on x86_64. Correct?

@leecannon
Copy link
Contributor

@matrizzo It is possible

const Asdf = extern struct {
    a: u32,
    b: [4]u64 align(1), // or even align(4)
};

test {
    try std.testing.expectEqual(@sizeOf(u32) + 4 * @sizeOf(u64), @sizeOf(Asdf));
}

@ominitay
Copy link
Contributor

So just to understand this correctly... At the moment there is no way at all to declare a structure with this layout in Zig

struct asdf {
    uint32_t a;
    uint64_t b[4];
};

that has no padding between a and b on x86_64. Correct?

Use an extern struct for this. I've already clarified how you shouldn't be using packed structs when you don't want bit-packing. Lee has given a good example above.

@ominitay
Copy link
Contributor

I'd recommend that you join a Zig community if you'd like further help.

@User65-87-11
Copy link

The documentation regarding packed vs extern structs is poor in my opinion. I'll get around to improving it at some point, since it does appear to mislead people a lot.

from packed struct docs

"There is no padding between fields." Does that mean it should give back struct size as a total size of each type?
u16+u8 = 3 ? Now it returns struct size = 4 with those types in it. In C struct with packed attribute it would gives 3 bytes as expected.

@nektro
Copy link
Contributor

nektro commented Sep 24, 2023

a zig packed struct is not a c packed struct as has been explained above. a zig packed struct is for bit packing fields into an integer representation. and a u24 would be padded to a u32 which is why you get 4 instead of 3. for 3 you would need to do @bitSizeOf(T) / 8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error message This issue points out an error message that is unhelpful and should be improved. frontend Tokenization, parsing, AstGen, Sema, and Liveness.
Projects
None yet
Development

No branches or pull requests

7 participants