Zig 0.16.0 require @ptrCast on pointer to packed struct

Explain
1

I just updated one of my repo to 0.16.0. Previously, I had a variable with type *MyType. Previously, I assign this variable as my_pointer: *MyType = &my_type_variable. However, on 0.16.0, I had to do it with my_pointer: *MyType = @ptrCast(&my_type_variable) where my_type_variable: Mytype.

As additional information, MyType = packed struct(u64) {}; if that helps.

Can someone enlighten me why do I have to do @ptrCast here? Maybe it is part of the changelog, but I still quite confused here.

1 Like
2

what is the definition of my_type_variable

3

It has type MyType. I added this information in the original post.

4

I cannot reproduce this, can you provide a snippet of the actual code, including the type definition.

And also need to check that you are actually using 0.16 release and not an older nightly build.

2 Likes
5

I used zig 0.16.0 release.

It turns out that the exact scenario is the packed struct MyType is part of outer struct that is also a packed struct. Here take a look into this snippet

const std = @import("std");

const MyType = packed struct(u64) {
    a: u16,
    b: u16,
    c: u16,
    d: u16,
};

const MyOuterType = packed struct(u128) {
    first: MyType,
    second: MyType,
};
pub fn main() !void {
    const first_my_type_variable: MyType = .{ .a = 1, .b = 2, .c = 3, .d = 4 };
    const second_my_type_variable: MyType = .{ .a = 1, .b = 2, .c = 3, .d = 4 };
    var my_outer_variable: MyOuterType = .{
        .first = first_my_type_variable,
        .second = second_my_type_variable,
    };
    const my_pointer: *MyType = &my_outer_variable.first;
    std.log.debug("{}", .{my_pointer.*});
}

This will result the following error

test.zig:21:33: error: expected type '*test.MyType', found '*align(16:0:16) test.MyType'
    const my_pointer: *MyType = &my_outer_variable.first;
                                ^~~~~~~~~~~~~~~~~~~~~~~~
test.zig:21:33: note: pointer host size '16' cannot cast into pointer host size '0'
6

Ah, that will do it.

Pointers to members of packed structs have extra comp time information to know where exactly the data is, your @ptrCast is getting rid of that information, you’re (un)lucky if happens to still read the correct data.

How you get around this depends on your exact use case:

  • you could change your pointer type to match it,
  • you could copy the data to avoid the pointer issues
  • you could change the outer type to not be packed
  • probably more I cant think of atm.
1 Like
7

Hmmm I wonder what I can do here. My use case is that both my_pointer and MyOuterPart is part of a bigger struct. I think I will not have any dangling pointer as it is impossible for my_pointer to access invalid address.

The main reason why I started this question is because why does this only happen in 0.16.0 and never been catched on previous release

8

This change happened quite some time ago, the relevant issue is byte-aligned pointer to `packed struct` field is no longer a normal pointer · Issue #25715 · ziglang/zig · GitHub

Previously byte-aligned pointers inside bitpacked structs would coerce to normal ones but that doesn’t happen since frontend: packed struct field ptr no longer finds byte borders · ziglang/zig@0681bf0 · GitHub

4 Likes
9

Dangling pointers are not relevant? Did you mean an invalid pointer from your cast?

some quick testing shows that it coerces, and it does it so correctly to my surprise! It even recognises when the bit offsets cause it to be non default aligned!

If you’ve never encountered this error before then you must have, by chance, always done it with a pointer that was compatible with a non bit pointer.

Unfortunately, @ptrCast does not retain the correct casting behaviour, so you’d have to do some ptr arithmatic to get a correct pointer.

@GasInfinity answered while I was messing around with a function to try to cast correctly, which kinda works, but I’m too tired to remember how to do alignment properly

1 Like
10

From this snippet, I don’t believe you should be using a packed struct here. packed struct types are for bit-packing: if you’re only using them with byte-aligned types like u16, there’s a good chace you’re not doing the right thing. The bit-packing behavior is why pointers to packed struct fields have strange types which are not compatible with normal pointers. In general, it’s very rare to actually explicitly take pointers to packed struct fields.

I suspect you instead want extern struct, which is not bit-packed but guarantees C-like struct layout rules, where fields are placed in order as tightly packed as possible while still respecting their alignment requirements. If you don’t think that’s right, please explain your real use case and I can try to help you out!

6 Likes
11

A non-volatile pointer is allowed to be cast to a volatile pointer.

In the past, if the bit offset was exactly byte-aligned, the bit pointer would be converted into a regular pointer. In this process, the information about the host size was directly erased.

However, the size of the host is not as irrelevant as it appears. If we convert a pointer to a volatile version, we will find that their semantics are different.

In this example, my_volatile_pointer, which contains host size information, must be read using two instructions:

        mov     rax, qword ptr [rbp - 64]
        mov     rax, qword ptr [rbp - 56]

, that is, a full 128-bit read.
The my_volatile_pointer_legeacy that erases host size information reads 64 bits directly when accessed:

        mov     rax, qword ptr [rbp - 64]

In addition, bit pointers cannot use atomic instructions, while regular pointers can. In the past, the practice of erasing the information of bit pointers to regular pointers may have inadvertently allowed certain unexpected atomic behaviors.

Edit: Currently, the documentation for 0.16 still retains the following description for pointers to packed struct fields:
“the pointer to a non-byte-aligned field has special properties and cannot be passed when a normal pointer is expected”

This sentence implies that a pointer to a packed struct field aligned by bytes can be passed to a normal pointer, which is somewhat misleading at present.

Nevertheless, I believe the current implementation is better; it just needs improved documentation.

12

The inner one I believe should be packed struct because it is for bit packing (but maybe, my understanding may be wrong). At first, I thought the outer one can be extern struct, however there is one field that is u48 which is I also believe not byte-aligned types. Then, I concluded that both the inner and outer struct should be packed struct. That is my thought process, if something is wrong, I will be glad to be corrected.

To give you the full use case, here is the actual struct that I use. This struct is being used to send and receive information from PC to microcontroller (written in C). As a note, currently this struct is a legacy code so I currently do not have the abiility to change the struct.

const std = @import("std");

const MyType = packed struct(u64) {
    a: f32,
    b: u10,
    c: u6,
    d: bool,
    e: bool,
    f: bool,
    g: bool,
    h: packed struct {
        h1: bool,
        h2: bool,
    },
    i: bool,
    j: u1,
    k: enum(u4) { dummy },
    l: u4,
};

const MyOuterType = packed struct(u256) {
    a: enum(u16) { dummy },
    b: u48,
    c: packed struct(u192) {
        c1: MyType,
        c2: MyType,
        c3: MyType,
    },
};

pub fn main() !void {
    const my_type_variable = std.mem.zeroInit(MyType, .{
        .d = true,
        .e = true,
        .f = true,
    });
    var my_outer_variable = std.mem.zeroInit(MyOuterType, .{});
    my_outer_variable.c.c1 = my_type_variable;
    const my_pointer: *MyType = @ptrCast(&my_outer_variable.c.c1);
    std.log.debug("pointer value: {}", .{my_pointer.*});
    std.log.debug("actual value: {}", .{my_outer_variable.c.c1});
}

Currently, even this snippet already give the wrong output

debug: pointer value: .{ .a = 0, .b = 0, .c = 0, .d = false, .e = false, .f = false, .g = false, .h = .{ .h1 = false, .h2 = false }, .i = false, .j = 0, .k = .dummy, .l = 0 }
debug: actual value: .{ .a = 0, .b = 0, .c = 0, .d = true, .e = true, .f = true, .g = false, .h = .{ .h1 = false, .h2 = false }, .i = false, .j = 0, .k = .dummy, .l = 0 }
13

You can easily hack this to work with a helper function:

pub fn fieldPackedPtr(
	container: anytype,
	comptime field_name: []const u8,
) *@FieldType(@TypeOf(container), field_name) {
	// Discard all type and alignment information
	const addr: usize = @intFromPtr(&container);
	// Assert the field's bit-offset is an exact multiple of 8
	return @ptrFromInt(addr + comptime @divExact(@bitOffsetOf(@TypeOf(container), field_name), 8));
}

test MyType {
	const my_type_variable = std.mem.zeroInit(MyType, .{
		.d = true,
		.e = true,
		.f = true,
	});
	var my_outer_variable = std.mem.zeroInit(MyOuterType, .{});
	my_outer_variable.c.c1 = my_type_variable;
	const my_pointer: *MyType = fieldPackedPtr(my_outer_variable.c, "c1");
	try std.testing.expectEqualDeep(my_outer_variable.c.c1, my_pointer.*);
}
1 Like
14

What if you refactored OuterType like this? Would that make sense?

const MyOuterType = extern struct {
    ab: packed struct(u64) {
        a: enum(u16) { dummy },
        b: u48,
    },
    c: [3]MyType,
};
1 Like
15

That only works if the field is aligned naturally, which it may not be, addressing that is annoying to say the least.

16

Hm, the u48 is a bit odd, you don’t usually see non-power-of-two types in this kind of thing. Out of interest, could I ask for the C type definitions this is based on? Is there any relationship between a and b, or are they just two separate values which just happen to look a bit like they could be parts of a 64-bit integer?

But yeah, it sounds to me like MyType should be a packed struct while MyOuterType should not. To be clear, the only use case for packed struct is bit-packing: using it for anything else is at best a code smell and at worst a bug. (For instance, because packed structs put their first field at the least-significant bit of their backing integer, the field order effectively gets flipped on big-endian targets if you use a packed struct just to try and pack bytes together.) There is an accepted proposal to change the packed keyword to bitpack to make this more clear.

My suggestion is likely one of these two:

const MyOuterType = extern struct {
    a: enum(u16) { dummy },
    b_raw: [6]u8, // to be `@bitCast` to and from `u48`
    c: [3]MyType,
};
// or
const MyOuterType = extern struct {
    /// This is probably a bad field name, I can't suggest a good one
    /// without knowing the full context I'm afraid!
    header: packed struct(u64) {
        a: enum(u16) { dummy },
        b: u48,
    },
    c: [3]MyType,
};

It kind of looks to me like MyOuterType might be thought of as a sequence of 4x 64-bit values, with a and b being packed into the first one. In that case, the second suggestion probably makes sense.

2 Likes
17

I am afraid I cannot share the C type definition. However, your suggestion makes sense to me. Fortunately, the u48 is actually only a reserved bit so both first solution and second solution works well to me, though I prefer the second approach for readability.

Thank you for everyone who is participating this topic.