From 2611d97fb07e5cba5657f508e93f01d60a2379bd Mon Sep 17 00:00:00 2001 From: mlugg Date: Sat, 10 Jun 2023 01:23:17 +0100 Subject: Sema: copy pointer alignment to union field pointers This implements the semantics as discussed in today's compiler meeting, where the alignment of pointers to fields of default-layout unions cannot exceed the field's alignment. Resolves: #15878 --- test/behavior/union.zig | 109 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) (limited to 'test') diff --git a/test/behavior/union.zig b/test/behavior/union.zig index 33cf1198ad..f7a481e311 100644 --- a/test/behavior/union.zig +++ b/test/behavior/union.zig @@ -1583,3 +1583,112 @@ test "coerce enum literal to union in result loc" { try U.doTest(true); try comptime U.doTest(true); } + +test "defined-layout union field pointer has correct alignment" { + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest; // TODO + + const S = struct { + fn doTheTest(comptime U: type) !void { + var a: U = .{ .x = 123 }; + var b: U align(1) = .{ .x = 456 }; + var c: U align(64) = .{ .x = 789 }; + + const ap = &a.x; + const bp = &b.x; + const cp = &c.x; + + comptime assert(@TypeOf(ap) == *u32); + comptime assert(@TypeOf(bp) == *align(1) u32); + comptime assert(@TypeOf(cp) == *align(64) u32); + + try expectEqual(@as(u32, 123), ap.*); + try expectEqual(@as(u32, 456), bp.*); + try expectEqual(@as(u32, 789), cp.*); + } + }; + + const U1 = extern union { x: u32 }; + const U2 = packed union { x: u32 }; + + try S.doTheTest(U1); + try S.doTheTest(U2); + try comptime S.doTheTest(U1); + try comptime S.doTheTest(U2); +} + +test "undefined-layout union field pointer has correct alignment" { + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest; // TODO + + const S = struct { + fn doTheTest(comptime U: type) !void { + var a: U = .{ .x = 123 }; + var b: U align(1) = .{ .x = 456 }; + var c: U align(64) = .{ .x = 789 }; + + const ap = &a.x; + const bp = &b.x; + const cp = &c.x; + + comptime assert(@TypeOf(ap) == *u32); + comptime assert(@TypeOf(bp) == *align(1) u32); + comptime assert(@TypeOf(cp) == *u32); // undefined layout so does not inherit larger aligns + + try expectEqual(@as(u32, 123), ap.*); + try expectEqual(@as(u32, 456), bp.*); + try expectEqual(@as(u32, 789), cp.*); + } + }; + + const U1 = union { x: u32 }; + const U2 = union(enum) { x: u32 }; + + try S.doTheTest(U1); + try S.doTheTest(U2); + try comptime S.doTheTest(U1); + try comptime S.doTheTest(U2); +} + +test "packed union field pointer has correct alignment" { + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest; // TODO + + const U = packed union { x: u20 }; + const S = packed struct(u24) { a: u2, u: U, b: u2 }; + + var a: S = undefined; + var b: S align(1) = undefined; + var c: S align(64) = undefined; + + const ap = &a.u.x; + const bp = &b.u.x; + const cp = &c.u.x; + + comptime assert(@TypeOf(ap) == *align(4:2:3) u20); + comptime assert(@TypeOf(bp) == *align(1:2:3) u20); + comptime assert(@TypeOf(cp) == *align(64:2:3) u20); + + a.u = .{ .x = 123 }; + b.u = .{ .x = 456 }; + c.u = .{ .x = 789 }; + + try expectEqual(@as(u20, 123), ap.*); + try expectEqual(@as(u20, 456), bp.*); + try expectEqual(@as(u20, 789), cp.*); +} -- cgit v1.2.3 From 80e493cb3631827909da4daaff001edb8691a83f Mon Sep 17 00:00:00 2001 From: mlugg Date: Fri, 16 Jun 2023 23:08:33 +0100 Subject: Sema: copy pointer alignment to struct field pointers --- src/Module.zig | 1 + src/Sema.zig | 34 +++++++++++++++++++-- test/behavior/struct.zig | 77 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 3 deletions(-) (limited to 'test') diff --git a/src/Module.zig b/src/Module.zig index 6d04b8f894..16f1c4f641 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -991,6 +991,7 @@ pub const Struct = struct { is_comptime: bool, /// Returns the field alignment. If the struct is packed, returns 0. + /// Keep implementation in sync with `Sema.structFieldAlignment`. pub fn alignment( field: Field, mod: *Module, diff --git a/src/Sema.zig b/src/Sema.zig index 4403df671a..16b3b5c710 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -25842,6 +25842,9 @@ fn structFieldPtrByIndex( const target = mod.getTarget(); + const parent_align = struct_ptr_ty_info.flags.alignment.toByteUnitsOptional() orelse + try sema.typeAbiAlignment(struct_ptr_ty_info.child.toType()); + if (struct_obj.layout == .Packed) { comptime assert(Type.packed_struct_layout_version == 2); @@ -25863,8 +25866,6 @@ fn structFieldPtrByIndex( ptr_ty_data.packed_offset.bit_offset += struct_ptr_ty_info.packed_offset.bit_offset; } - const parent_align = struct_ptr_ty_info.flags.alignment.toByteUnitsOptional() orelse - struct_ptr_ty_info.child.toType().abiAlignment(mod); ptr_ty_data.flags.alignment = Alignment.fromByteUnits(parent_align); // If the field happens to be byte-aligned, simplify the pointer type. @@ -25888,8 +25889,13 @@ fn structFieldPtrByIndex( ptr_ty_data.packed_offset = .{ .host_size = 0, .bit_offset = 0 }; } } + } else if (struct_obj.layout == .Extern and field_index == 0) { + // This is the first field in memory, so can inherit the struct alignment + ptr_ty_data.flags.alignment = Alignment.fromByteUnits(parent_align); } else { - ptr_ty_data.flags.alignment = field.abi_align; + // Our alignment is capped at the field alignment + const field_align = try sema.structFieldAlignment(field, struct_obj.layout); + ptr_ty_data.flags.alignment = Alignment.fromByteUnits(@min(field_align, parent_align)); } const ptr_field_ty = try mod.ptrType(ptr_ty_data); @@ -35952,6 +35958,28 @@ fn unionFieldAlignment(sema: *Sema, field: Module.Union.Field) !u32 { field.abi_align.toByteUnitsOptional() orelse try sema.typeAbiAlignment(field.ty))); } +/// Keep implementation in sync with `Module.Struct.Field.alignment`. +fn structFieldAlignment(sema: *Sema, field: Module.Struct.Field, layout: std.builtin.Type.ContainerLayout) !u32 { + const mod = sema.mod; + if (field.abi_align.toByteUnitsOptional()) |a| { + assert(layout != .Packed); + return @as(u32, @intCast(a)); + } + switch (layout) { + .Packed => return 0, + .Auto => if (mod.getTarget().ofmt != .c) { + return sema.typeAbiAlignment(field.ty); + }, + .Extern => {}, + } + // extern + const ty_abi_align = try sema.typeAbiAlignment(field.ty); + if (field.ty.isAbiInt(mod) and field.ty.intInfo(mod).bits >= 128) { + return @max(ty_abi_align, 16); + } + return ty_abi_align; +} + /// Synchronize logic with `Type.isFnOrHasRuntimeBits`. pub fn fnHasRuntimeBits(sema: *Sema, ty: Type) CompileError!bool { const mod = sema.mod; diff --git a/test/behavior/struct.zig b/test/behavior/struct.zig index 95b2718efd..08c4c37eee 100644 --- a/test/behavior/struct.zig +++ b/test/behavior/struct.zig @@ -1,6 +1,7 @@ const std = @import("std"); const builtin = @import("builtin"); const native_endian = builtin.target.cpu.arch.endian(); +const assert = std.debug.assert; const expect = std.testing.expect; const expectEqual = std.testing.expectEqual; const expectEqualSlices = std.testing.expectEqualSlices; @@ -1637,3 +1638,79 @@ test "instantiate struct with comptime field" { comptime std.debug.assert(things.foo == 1); } } + +test "struct field pointer has correct alignment" { + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest; // TODO + + const S = struct { + fn doTheTest() !void { + var a: struct { x: u32 } = .{ .x = 123 }; + var b: struct { x: u32 } align(1) = .{ .x = 456 }; + var c: struct { x: u32 } align(64) = .{ .x = 789 }; + + const ap = &a.x; + const bp = &b.x; + const cp = &c.x; + + comptime assert(@TypeOf(ap) == *u32); + comptime assert(@TypeOf(bp) == *align(1) u32); + comptime assert(@TypeOf(cp) == *u32); // undefined layout, cannot inherit larger alignment + + try expectEqual(@as(u32, 123), ap.*); + try expectEqual(@as(u32, 456), bp.*); + try expectEqual(@as(u32, 789), cp.*); + } + }; + + try S.doTheTest(); + try comptime S.doTheTest(); +} + +test "extern struct field pointer has correct alignment" { + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest; // TODO + + const S = struct { + fn doTheTest() !void { + var a: extern struct { x: u32, y: u32 } = .{ .x = 1, .y = 2 }; + var b: extern struct { x: u32, y: u32 } align(1) = .{ .x = 3, .y = 4 }; + var c: extern struct { x: u32, y: u32 } align(64) = .{ .x = 5, .y = 6 }; + + const axp = &a.x; + const bxp = &b.x; + const cxp = &c.x; + const ayp = &a.y; + const byp = &b.y; + const cyp = &c.y; + + comptime assert(@TypeOf(axp) == *u32); + comptime assert(@TypeOf(bxp) == *align(1) u32); + comptime assert(@TypeOf(cxp) == *align(64) u32); // first field, inherits larger alignment + comptime assert(@TypeOf(ayp) == *u32); + comptime assert(@TypeOf(byp) == *align(1) u32); + comptime assert(@TypeOf(cyp) == *u32); + + try expectEqual(@as(u32, 1), axp.*); + try expectEqual(@as(u32, 3), bxp.*); + try expectEqual(@as(u32, 5), cxp.*); + + try expectEqual(@as(u32, 2), ayp.*); + try expectEqual(@as(u32, 4), byp.*); + try expectEqual(@as(u32, 6), cyp.*); + } + }; + + try S.doTheTest(); + try comptime S.doTheTest(); +} -- cgit v1.2.3