codegen/llvm: fix wrong field offset in packed structs

src/arch/wasm/CodeGen.zig

@@ -3015,7 +3015,10 @@ fn lowerParentPtr(func: *CodeGen, ptr_val: Value, offset: u32) InnerError!WValue
 
             const field_offset = switch (parent_ty.zigTypeTag(mod)) {
                 .Struct => switch (parent_ty.containerLayout(mod)) {
-                    .Packed => parent_ty.packedStructFieldByteOffset(@as(usize, @intCast(field.index)), mod),
+                    .Packed => if (parent_ty.packedStructFieldByteAligned(@intCast(field.index), mod))
+                        parent_ty.packedStructFieldByteOffset(@as(usize, @intCast(field.index)), mod)
+                    else
+                        0,
                     else => parent_ty.structFieldOffset(@as(usize, @intCast(field.index)), mod),
                 },
                 .Union => switch (parent_ty.containerLayout(mod)) {

src/codegen.zig

@@ -696,13 +696,10 @@ fn lowerParentPtr(
                             mod,
                         )),
                         .Packed => if (mod.typeToStruct(base_type.toType())) |struct_obj|
-                            math.divExact(u16, struct_obj.packedFieldBitOffset(
-                                mod,
-                                @intCast(field.index),
-                            ), 8) catch |err| switch (err) {
-                                error.UnexpectedRemainder => 0,
-                                error.DivisionByZero => unreachable,
-                            }
+                            if (base_type.toType().packedStructFieldByteAligned(@intCast(field.index), mod))
+                                @divExact(struct_obj.packedFieldBitOffset(mod, @intCast(field.index)), 8)
+                            else
+                                0
                         else
                             0,
                     },

src/codegen/llvm.zig

@@ -3702,7 +3702,7 @@ pub const Object = struct {
                     .opt_payload,
                     .elem,
                     .field,
-                    => try o.lowerParentPtr(val, ty.ptrInfo(mod).packed_offset.bit_offset % 8 == 0),
+                    => try o.lowerParentPtr(val),
                     .comptime_field => unreachable,
                 };
                 switch (ptr.len) {
@@ -4137,14 +4137,14 @@ pub const Object = struct {
         return o.lowerDeclRefValue(ptr_ty, decl_index);
     }
 
-    fn lowerParentPtr(o: *Object, ptr_val: Value, byte_aligned: bool) Allocator.Error!Builder.Constant {
+    fn lowerParentPtr(o: *Object, ptr_val: Value) Allocator.Error!Builder.Constant {
         const mod = o.module;
         return switch (mod.intern_pool.indexToKey(ptr_val.toIntern()).ptr.addr) {
             .decl => |decl| o.lowerParentPtrDecl(decl),
             .mut_decl => |mut_decl| o.lowerParentPtrDecl(mut_decl.decl),
             .int => |int| try o.lowerIntAsPtr(int),
             .eu_payload => |eu_ptr| {
-                const parent_ptr = try o.lowerParentPtr(eu_ptr.toValue(), true);
+                const parent_ptr = try o.lowerParentPtr(eu_ptr.toValue());
 
                 const eu_ty = mod.intern_pool.typeOf(eu_ptr).toType().childType(mod);
                 const payload_ty = eu_ty.errorUnionPayload(mod);
@@ -4161,7 +4161,7 @@ pub const Object = struct {
                 });
             },
             .opt_payload => |opt_ptr| {
-                const parent_ptr = try o.lowerParentPtr(opt_ptr.toValue(), true);
+                const parent_ptr = try o.lowerParentPtr(opt_ptr.toValue());
 
                 const opt_ty = mod.intern_pool.typeOf(opt_ptr).toType().childType(mod);
                 const payload_ty = opt_ty.optionalChild(mod);
@@ -4179,15 +4179,15 @@ pub const Object = struct {
             },
             .comptime_field => unreachable,
             .elem => |elem_ptr| {
-                const parent_ptr = try o.lowerParentPtr(elem_ptr.base.toValue(), true);
+                const parent_ptr = try o.lowerParentPtr(elem_ptr.base.toValue());
                 const elem_ty = mod.intern_pool.typeOf(elem_ptr.base).toType().elemType2(mod);
 
                 return o.builder.gepConst(.inbounds, try o.lowerType(elem_ty), parent_ptr, null, &.{
                     try o.builder.intConst(try o.lowerType(Type.usize), elem_ptr.index),
                 });
             },
             .field => |field_ptr| {
-                const parent_ptr = try o.lowerParentPtr(field_ptr.base.toValue(), byte_aligned);
+                const parent_ptr = try o.lowerParentPtr(field_ptr.base.toValue());
                 const parent_ty = mod.intern_pool.typeOf(field_ptr.base).toType().childType(mod);
 
                 const field_index: u32 = @intCast(field_ptr.index);
@@ -4213,7 +4213,10 @@ pub const Object = struct {
                     },
                     .Struct => {
                         if (parent_ty.containerLayout(mod) == .Packed) {
+                            const ptr_ty = mod.intern_pool.typeOf(ptr_val.ip_index).toType();
+                            const byte_aligned = ptr_ty.isPackedStructFieldPtrByteAligned(mod);
                             if (!byte_aligned) return parent_ptr;
+
                             const llvm_usize = try o.lowerType(Type.usize);
                             const base_addr =
                                 try o.builder.castConst(.ptrtoint, parent_ptr, llvm_usize);

src/type.zig

@@ -3113,6 +3113,42 @@ pub const Type = struct {
         };
     }
 
+    // value can be accessed through a normal ptr (without packed_offset related masking)
+    pub fn isPackedStructFieldPtrByteAligned(ptr_ty: Type, mod: *Module) bool {
+        const ptr_info = ptr_ty.ptrInfo(mod);
+        if (ptr_info.packed_offset.host_size == 0) return true;
+        const elem_ty = ptr_ty.childType(mod);
+        const elem_abi_size = elem_ty.abiSize(mod);
+        const elem_size_bits = elem_ty.bitSize(mod);
+        const bit_offset = ptr_info.packed_offset.bit_offset;
+
+        return (bit_offset % 8 == 0 and elem_abi_size * 8 == elem_size_bits);
+    }
+
+    pub fn packedStructFieldByteAligned(ty: Type, field_index: usize, mod: *Module) bool {
+        const struct_type = mod.intern_pool.indexToKey(ty.toIntern()).struct_type;
+        const struct_obj = mod.structPtrUnwrap(struct_type.index).?;
+        assert(struct_obj.layout == .Packed);
+        comptime assert(Type.packed_struct_layout_version == 2);
+
+        var bit_offset: u16 = undefined;
+        var elem_abi_size: u64 = 0;
+        var elem_size_bits: u16 = undefined;
+        var running_bits: u16 = 0;
+        for (struct_obj.fields.values(), 0..) |f, i| {
+            if (!f.ty.hasRuntimeBits(mod)) continue;
+
+            const field_bits = @as(u16, @intCast(f.ty.bitSize(mod)));
+            if (i == field_index) {
+                bit_offset = running_bits;
+                elem_size_bits = field_bits;
+                elem_abi_size = f.ty.abiSize(mod);
+            }
+            running_bits += field_bits;
+        }
+        return (bit_offset % 8 == 0 and elem_abi_size * 8 == elem_size_bits);
+    }
+
     pub fn packedStructFieldByteOffset(ty: Type, field_index: usize, mod: *Module) u32 {
         const struct_type = mod.intern_pool.indexToKey(ty.toIntern()).struct_type;
         const struct_obj = mod.structPtrUnwrap(struct_type.index).?;

test/behavior/packed-struct.zig

@@ -354,6 +354,44 @@ test "byte-aligned field pointer offsets" {
     try comptime S.doTheTest();
 }
 
+test "nested packed struct field pointers" {
+    if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
+    if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
+    if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
+    if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // ubsan unaligned pointer access
+    if (native_endian != .Little) return error.SkipZigTest; // Byte aligned packed struct field pointers have not been implemented yet
+
+    const S2 = packed struct {
+        base: u8,
+        p0: packed struct {
+            a: u4,
+            b: u4,
+            c: u8,
+        },
+        bit: u1,
+        p1: packed struct {
+            a: u7,
+            b: u8,
+        },
+
+        var s: @This() = .{ .base = 1, .p0 = .{ .a = 2, .b = 3, .c = 4 }, .bit = 0, .p1 = .{ .a = 5, .b = 6 } };
+    };
+
+    const ptr_base = &S2.s.base;
+    const ptr_p0_a = &S2.s.p0.a;
+    const ptr_p0_b = &S2.s.p0.b;
+    const ptr_p0_c = &S2.s.p0.c;
+    const ptr_p1_a = &S2.s.p1.a;
+    //const ptr_p1_b = &S2.s.p1.b;
+    try expectEqual(@as(u8, 1), ptr_base.*);
+    try expectEqual(@as(u4, 2), ptr_p0_a.*);
+    try expectEqual(@as(u4, 3), ptr_p0_b.*);
+    try expectEqual(@as(u8, 4), ptr_p0_c.*);
+    try expectEqual(@as(u7, 5), ptr_p1_a.*);
+    // try expectEqual(@as(u8,6), ptr_p1_b.*);    https://github.com/ziglang/zig/issues/16748
+}
+
 test "load pointer from packed struct" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
@@ -382,18 +420,59 @@ test "@intFromPtr on a packed struct field" {
     if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
 
     const S = struct {
-        const P = packed struct {
-            x: u8,
-            y: u8,
-            z: u32,
-        };
+        const P = packed struct { x: u8, y: u8, z: u32 };
+
         var p0: P = P{
             .x = 1,
             .y = 2,
             .z = 0,
         };
     };
     try expect(@intFromPtr(&S.p0.z) - @intFromPtr(&S.p0.x) == 2);
+
+    const S2 = packed struct {
+        base: u8,
+        p0: packed struct {
+            a: u4,
+            b: u4,
+            c: u8,
+        },
+        bit: u1,
+        p1: packed struct {
+            a: u7,
+            b: u8,
+        },
+
+        var s: @This() = .{ .base = 1, .p0 = .{ .a = 2, .b = 3, .c = 4 }, .bit = 0, .p1 = .{ .a = 5, .b = 6 } };
+    };
+    try expect(@intFromPtr(&S2.s.base) - @intFromPtr(&S2.s) == 0);
+    try expect(@intFromPtr(&S2.s.p0.a) - @intFromPtr(&S2.s) == 1);
+    try expect(@intFromPtr(&S2.s.p0.b) - @intFromPtr(&S2.s) == 1);
+    try expect(@intFromPtr(&S2.s.p0.c) - @intFromPtr(&S2.s) == 2);
+    try expect(@intFromPtr(&S2.s.bit) - @intFromPtr(&S2.s) == 0);
+    try expect(@intFromPtr(&S2.s.p1.a) - @intFromPtr(&S2.s) == 0);
+    // try expect(@intFromPtr(&S2.s.p1.b) - @intFromPtr(&S2.s) == 4);  https://github.com/ziglang/zig/issues/16748
+}
+
+test "packed struct fields modification" {
+    const Small = packed struct {
+        val: u8 = 0,
+        lo: u4 = 0,
+        hi: u4 = 0,
+
+        var p: @This() = undefined;
+    };
+    Small.p = .{
+        .val = 0x12,
+        .lo = 3,
+        .hi = 4,
+    };
+    try expect(@as(u16, @bitCast(Small.p)) == 0x4312);
+
+    Small.p.val -= Small.p.lo;
+    Small.p.val += Small.p.hi;
+    Small.p.hi -= Small.p.lo;
+    try expect(@as(u16, @bitCast(Small.p)) == 0x1313);
 }
 
 test "optional pointer in packed struct" {

test/behavior/packed-union.zig

@@ -38,3 +38,50 @@ test "flags in packed union" {
     try expectEqual(true, test_bits.enable_1);
     try expectEqual(false, test_bits.other_flags.flags.enable_1);
 }
+
+test "flags in packed union at offset" {
+    if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
+    if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
+    if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest;
+    if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
+
+    const FlagBits = packed union {
+        base_flags: packed union {
+            flags: packed struct(u4) {
+                enable_1: bool = true,
+                enable_2: bool = false,
+                enable_3: bool = false,
+                enable_4: bool = false,
+            },
+            bits: u4,
+        },
+        adv_flags: packed struct(u12) {
+            pad: u8 = 0,
+            adv: packed union {
+                flags: packed struct(u4) {
+                    enable_1: bool = true,
+                    enable_2: bool = false,
+                    enable_3: bool = false,
+                    enable_4: bool = false,
+                },
+                bits: u4,
+            },
+        },
+    };
+    var test_bits: FlagBits = .{ .adv_flags = .{ .adv = .{ .flags = .{} } } };
+
+    try expectEqual(@as(u8, 0), test_bits.adv_flags.pad);
+    try expectEqual(true, test_bits.adv_flags.adv.flags.enable_1);
+    try expectEqual(false, test_bits.adv_flags.adv.flags.enable_2);
+
+    test_bits.adv_flags.adv.flags.enable_1 = false;
+    test_bits.adv_flags.adv.flags.enable_2 = true;
+    try expectEqual(@as(u8, 0), test_bits.adv_flags.pad);
+    try expectEqual(false, test_bits.adv_flags.adv.flags.enable_1);
+    try expectEqual(true, test_bits.adv_flags.adv.flags.enable_2);
+
+    test_bits.adv_flags.adv.bits = 12;
+    try expectEqual(@as(u8, 0), test_bits.adv_flags.pad);
+    try expectEqual(false, test_bits.adv_flags.adv.flags.enable_1);
+    try expectEqual(false, test_bits.adv_flags.adv.flags.enable_2);
+}