commit 40cf3f7ae5fbfb84b7af6b27e6296ee858b209ef (tree)
parent d98147414d084bc41b00ba9c0be8c7b82ad4e76c
Author: Andrew Kelley <andrew@ziglang.org>
Date: Sun, 25 Jun 2023 23:06:53 -0700
Merge pull request #15995 from mlugg/fix/union-field-ptr-align
Sema: copy pointer alignment to union field pointers
Diffstat:
5 files changed, 231 insertions(+), 9 deletions(-)
diff --git a/lib/std/pdb.zig b/lib/std/pdb.zig
@@ -776,8 +776,8 @@ pub const Pdb = struct {
} else 0;
const found_line_index = start_line_index + line_entry_idx * @sizeOf(LineNumberEntry);
- const line_num_entry = @as(*align(1) LineNumberEntry, @ptrCast(&subsect_info[found_line_index]));
- const flags = @as(*LineNumberEntry.Flags, @ptrCast(&line_num_entry.Flags));
+ const line_num_entry: *align(1) LineNumberEntry = @ptrCast(&subsect_info[found_line_index]);
+ const flags: *align(1) LineNumberEntry.Flags = @ptrCast(&line_num_entry.Flags);
return debug.LineInfo{
.file_name = source_file_name,
diff --git a/src/Module.zig b/src/Module.zig
@@ -992,6 +992,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
@@ -25871,6 +25871,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);
@@ -25892,8 +25895,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.
@@ -25917,8 +25918,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);
@@ -26089,6 +26095,7 @@ fn unionFieldPtr(
assert(unresolved_union_ty.zigTypeTag(mod) == .Union);
const union_ptr_ty = sema.typeOf(union_ptr);
+ const union_ptr_info = union_ptr_ty.ptrInfo(mod);
const union_ty = try sema.resolveTypeFields(unresolved_union_ty);
const union_obj = mod.typeToUnion(union_ty).?;
const field_index = try sema.unionFieldIndex(block, union_ty, field_name, field_name_src);
@@ -26096,10 +26103,16 @@ fn unionFieldPtr(
const ptr_field_ty = try mod.ptrType(.{
.child = field.ty.toIntern(),
.flags = .{
- .is_const = !union_ptr_ty.ptrIsMutable(mod),
- .is_volatile = union_ptr_ty.isVolatilePtr(mod),
- .address_space = union_ptr_ty.ptrAddressSpace(mod),
- },
+ .is_const = union_ptr_info.flags.is_const,
+ .is_volatile = union_ptr_info.flags.is_volatile,
+ .address_space = union_ptr_info.flags.address_space,
+ .alignment = if (union_obj.layout == .Auto) blk: {
+ const union_align = union_ptr_info.flags.alignment.toByteUnitsOptional() orelse try sema.typeAbiAlignment(union_ty);
+ const field_align = try sema.unionFieldAlignment(field);
+ break :blk InternPool.Alignment.fromByteUnits(@min(union_align, field_align));
+ } else union_ptr_info.flags.alignment,
+ },
+ .packed_offset = union_ptr_info.packed_offset,
});
const enum_field_index = @as(u32, @intCast(union_obj.tag_ty.enumFieldIndex(field_name, mod).?));
@@ -35974,6 +35987,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
@@ -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();
+}
diff --git 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.*);
+}
