Zig: Incorrect byte offset and struct size for packed structs

Created on 5 Jun 2019  路  10Comments  路  Source: ziglang/zig

The byte offsets of fields within a packed struct are sometimes incorrect, see the below example.

const std = @import("std");

const PackedStruct = packed struct {
    /// bitoffset = 0, byteoffset = 0
    bool_a: bool,
    /// bitoffset = 1, byteoffset = 0
    bool_b: bool,
    /// bitoffset = 2, byteoffset = 0
    bool_c: bool,
    /// bitoffset = 3, byteoffset = 0
    bool_d: bool,
    /// bitoffset = 4, byteoffset = 0
    bool_e: bool,
    /// bitoffset = 5, byteoffset = 0
    bool_f: bool,
    /// bitoffset = 6, byteoffset = 0
    u1_a: u1,
    /// bitoffset = 7, byteoffset = 0
    bool_g: bool,
    /// bitoffset = 8, byteoffset = 1
    u1_b: u1,
    /// bitoffset = 9, byteoffset = 1
    u3_a: u3,
    /// bitoffset = 12, byteoffset = 1
    u10_a: u10,
    /// bitoffset = 22, byteoffset = 2
    u10_b: u10,
};

pub fn main() void {
    inline for (@typeInfo(PackedStruct).Struct.fields) |field| {
        std.debug.warn("field={}, byteoffset={} bitoffset={}\n",
            field.name,
            usize(@byteOffsetOf(PackedStruct, field.name)),
            usize(@bitOffsetOf(PackedStruct, field.name))
        );
    }
    std.debug.warn("totalsize {} bytes\n", usize(@sizeOf(PackedStruct)));
}

The comments show what the bit offsets and byte offsets should be judging by the documentation[1], but below is the result:

field=bool_a, byteoffset=0 bitoffset=0
field=bool_b, byteoffset=0 bitoffset=1
field=bool_c, byteoffset=0 bitoffset=2
field=bool_d, byteoffset=0 bitoffset=3
field=bool_e, byteoffset=0 bitoffset=4
field=bool_f, byteoffset=0 bitoffset=5
field=u1_a, byteoffset=0 bitoffset=6
field=bool_g, byteoffset=0 bitoffset=7
field=u1_b, byteoffset=1 bitoffset=8
field=u3_a, byteoffset=1 bitoffset=9
field=u10_a, byteoffset=1 bitoffset=12
field=u10_b, byteoffset=1 bitoffset=22
totalsize 5 bytes

The size is also incorrect, since it should be 4 bytes (the sum of the fields) but is instead 5.

1: "bool fields use exactly 1 bit" and "Zig supports arbitrary width Integers and although normally, integers with fewer than 8 bits will still use 1 byte of memory, in packed structs, they use exactly their bit width" at https://ziglang.org/documentation/0.4.0/#packed-struct

bug stage1

All 10 comments

I believe the problem could be in analyze.cpp:resolve_struct_type.cpp as that seems to be where the packed struct offsets are calculated.
https://github.com/ziglang/zig/blob/fdddd131068775f8c811a51fdc08939165ee9e58/src/analyze.cpp#L1657

Most of ir code is beyond my understanding but I see in resolve_struct_type.cpp() the idea of byte offset in a packed struct seems to be linked to the "preceding gen_field_index" and that's probably a larger concept than I've grok'd. My assumption is there is not currently an exact correlation between @byteOffsetOf and field.offset and if that's true then the builtin can resolve by considering field.bit_offset_in_host:

diff --git a/src/ir.cpp b/src/ir.cpp
index fb9e7b51..0dcd1d44 100644
--- a/src/ir.cpp
+++ b/src/ir.cpp
@@ -18331,11 +18331,11 @@ static IrInstruction *ir_analyze_instruction_byte_offset_of(IrAnalyze *ira,

     IrInstruction *field_name_value = instruction->field_name->child;
     size_t byte_offset = 0;
-    if (!validate_byte_offset(ira, type_value, field_name_value, &byte_offset))
+    TypeStructField *field = nullptr;
+    if (!(field = validate_byte_offset(ira, type_value, field_name_value, &byte_offset)))
         return ira->codegen->invalid_instruction;

-
-    return ir_const_unsigned(ira, &instruction->base, byte_offset);
+    return ir_const_unsigned(ira, &instruction->base, byte_offset + field->bit_offset_in_host / 8);
 }

 static IrInstruction *ir_analyze_instruction_bit_offset_of(IrAnalyze *ira,

note: this doesn't address @sizeOf returning a larger size of the struct than expected

I see no checks for whether or not the struct is packed in analyze.cpp:get_struct_type, could that be the root of the issue? I see some field offset calculations in there.

I don't believe that this is just an issue with @byteOffsetOf, @sizeOf and @bitOffsetOff as the struct doesn't seem to be laid out properly in memory from within GDB (fields are empty when they should show some numerical value, the equivalent C code shows them as having some value).

Another possibly simpler case:

error: destination type 'y' has size 5 but source type 'u32' has size 4
    const y = @bitCast(packed struct {_1: u1, x: u7, _: u24}, u32(0x1ff4)).x;
                       ^

It seems this is a problem even on structs which don't contain non-byte-divisble sized types

pub const Test = packed struct {
    a: [3]u8,
    b: [8]u8,
};

@byteOffsetOf(Test, "b"); returns 0

They are arrays of u8, so the first member is 24 bits.

It seems like i ran into this problem as well. Here is some additional findings wheather when this happens or not:

pub const Flags1 = packed struct {
    // byte 0
    b0_0: u1,
    b0_1: u1,
    b0_2: u1,
    b0_3: u1,
    b0_4: u1,
    b0_5: u1,
    b0_6: u1,
    b0_7: u1,

    // partial byte 1 (but not 8 bits)
    b1_0: u1,
    b1_1: u1,
    b1_2: u1,
    b1_3: u1,
    b1_4: u1,
    b1_5: u1,
    b1_6: u1,

    // some padding to fill to size 3
    _: u9,
};

pub const Flags2 = packed struct {
    // byte 0
    b0_0: u1,
    b0_1: u1,
    b0_2: u1,
    b0_3: u1,
    b0_4: u1,
    b0_5: u1,
    b0_6: u1,
    b0_7: u1,

    // partial byte 1 (but not 8 bits)
    b1_0: u1,
    b1_1: u1,
    b1_2: u1,
    b1_3: u1,
    b1_4: u1,
    b1_5: u1,
    b1_6: u1,

    // some padding that should yield @sizeOf(Flags2) == 4
    _: u10, // this *was* originally 17, but the error happens with 10 as well
};

pub const Flags3 = packed struct {
    // byte 0
    b0_0: u1,
    b0_1: u1,
    b0_2: u1,
    b0_3: u1,
    b0_4: u1,
    b0_5: u1,
    b0_6: u1,
    b0_7: u1,

    // byte 1
    b1_0: u1,
    b1_1: u1,
    b1_2: u1,
    b1_3: u1,
    b1_4: u1,
    b1_5: u1,
    b1_6: u1,
    b1_7: u1,

    // some padding that should yield @sizeOf(Flags2) == 4
    _: u16, // it works, if the padding is 8-based
};

comptime {
    @compileLog("Flags1", @sizeOf(Flags1)); // => 3
    @compileLog("Flags2", @sizeOf(Flags2)); // => 5
    @compileLog("Flags3", @sizeOf(Flags3)); // => 4
}

I could reduce this problem to a struct with a single field:

const std = @import("std");

const Broken = packed struct {
    element: u24,
};

test "sizeOf == 4" { // succeeds
    std.debug.assert(@sizeOf(Broken) == 4);
}

test "sizeOf == 3" { // fails
    std.debug.assert(@sizeOf(Broken) == 3);
}

The assertion fails, my current zig version is 0.5.0+ad0871ea4

Another failure case is packed structs:

const std = @import("std");

const s1 = packed struct {
  a: u8,
  b: u8,
  c: u8,
};

const s2 = packed struct {
  d: u8,
  e: u8,
  f: u8,
};

const s3 = packed struct {
  x: s1,
  y: s2,
};

pub fn main() u8 {
  std.debug.warn("@sizeOf(s1)={}\n", .{@sizeOf(s1)});
  std.debug.warn("@sizeOf(s2)={}\n", .{@sizeOf(s2)});
  std.debug.warn("@sizeOf(s3)={}\n", .{@sizeOf(s3)});
  return 5;
}
@sizeOf(s1)=3
@sizeOf(s2)=3
@sizeOf(s3)=8

I ran into this today, minified my example to this code which fails its comptime check:

pub const S = packed struct {
    _: [3]u8,
};
comptime { std.debug.assert(@sizeOf(S) == 3); }
Was this page helpful?
0 / 5 - 0 ratings